Skip to content

Fix resource leaks in JunitReportReader and Bdd2Octane#51

Merged
micheleh merged 2 commits intomasterfrom
fix/resource-leaks-reader-streams
Apr 9, 2026
Merged

Fix resource leaks in JunitReportReader and Bdd2Octane#51
micheleh merged 2 commits intomasterfrom
fix/resource-leaks-reader-streams

Conversation

@micheleh
Copy link
Copy Markdown
Collaborator

@micheleh micheleh commented Apr 4, 2026

Summary

  • JunitReportReader now implements Closeable, allowing callers to properly release its internal XMLEventReader and the underlying InputStream
  • Bdd2Octane.run() uses try-with-resources for both the FileOutputStream (output) and JunitReportReader (per report file), ensuring streams are closed even on exceptions

Context

This is the same class of bug that caused a "Too many open files" error previously fixed in FeatureFileLocator (commit 2d31c79, PR #18). The Bdd2Octane/JunitReportReader code paths were not addressed at that time.

Previously, if processing was interrupted by an exception mid-iteration, neither the report file input streams nor the output stream were closed. With many report files, this would exhaust file descriptors.

Verification

  • Confirmed streams are properly closed after both full and partial iteration
  • Full test suite passes (42 tests, 0 failures)

JunitReportReader now implements Closeable, closing both its
XMLEventReader and the underlying InputStream. Bdd2Octane uses
try-with-resources for the report reader and the output stream,
ensuring resources are released even when processing fails.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@micheleh micheleh requested a review from nissimshitrit April 4, 2026 18:56
try {
reader.close();
} catch (XMLStreamException e) {
// best effort
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be good to add log here so user will be aware what happen, I don't think it need to throw exception since closing file failure should not effect result

@micheleh micheleh merged commit 968f078 into master Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants