ExecutionDataReader ignores all EOFException?

[bjk...@gmail.com]

ExecutionDataReader.read has a try/catch for EOFException. Looking at commit history, the catch block used to have a comment "// expected at the end of the stream" (comment removed in commit 20823ddbd), so it seems the intention was to catch EOFException thrown from the readByte() call, but the placement of the try/catch means that EOFException from readBlock is also ignored. This causes a few quirks that we have observed in actual files:
1. Completely empty files are accepted.
2. Non-empty but truncated execution data files are accepted. e.g., a BLOCK_EXECUTIONDATA claims to have 200 bytes, but the file only contains an addition 10 bytes.

(We have not diagnosed the root cause of these truncated files in our environments. Perhaps there are file transfer issues, or perhaps JVM processes are being killed before fully writing the data.)

Do I misunderstand the intent of this try/catch, or would the readByte call ideally be replaced with in.read() and an explicit check against -1 and the try/catch removed altogether? Can that change even be made at this point, or would it be considered an incompatible change due to inability to read existing files? If so, perhaps a comment should be added to mention this subtlety in the current behavior?

[Marc R. Hoffmann]

Hi,

thanks for sharing this detailled analysis.

1. Empty exec files are considered valid by desing (the JaCoCo agent
writes an empty file at startup to ensure the target location is writable).
2. Truncated (e.g. incomplete blocks) should not be ignored.

I agree catching EOFException is a bad practice here and the method
should be how you proposed. I created a pull request here:
https://github.com/jacoco/jacoco/pull/397

Regards,
-marc