Dhairya-007 commented on PR #29702:
URL: https://github.com/apache/camel/pull/29702#issuecomment-6825263196
All three points are in, thanks — the leak is the important one and I had
not thought it through: before this change those paths practically never threw,
so opening-then-skipping was harmless; with a stale offset it throws on every
poll.
**1. Close on failure.** All three places:
```java
InputStream inputStream = IOHelper.toInputStream(f, charset);
try {
inputStream.skipNBytes(file.getLastOffsetValue());
} catch (IOException e) {
// the stream is not returned, so nothing else can close it
IOHelper.close(inputStream);
throw e;
}
return inputStream;
```
the same for the reader in `genericFileToReader`, and in
`IOHelper.toString(BufferedReader, long)` the `skipFully` call moved inside the
existing `try` whose `finally` already closes the reader.
**2. Upgrade guide** — added, and it says what you pointed out about the
repeat: the failing exchange leaves the file in place, so the next poll reads
the same offset from the resume store and fails again, and the recorded offset
has to be reset for the file to be consumed from the start.
**3. The test you suggested** — added as
`GenericFileConverterResumeOffsetTest`, four cases: both converters skipping to
a valid offset, and an offset past the end failing with `EOFException` for
each. Each failure case then deletes the file and asserts it is gone, which is
the Windows-relevant check that the handle was actually released — the
assertion that would have failed before this commit on that platform.
Also **rebased on main**, since the branch was 58 commits behind and the
upgrade-guide entry would otherwise have landed in a conflicting spot (same
situation as #29709).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
