Skip to content

Fix Resource Leaks with Try-With-Resources Pattern#586

Merged
cyrille-artho merged 1 commit into
javapathfinder:masterfrom
PritamP20:fix-resource-leaks
Mar 25, 2026
Merged

Fix Resource Leaks with Try-With-Resources Pattern#586
cyrille-artho merged 1 commit into
javapathfinder:masterfrom
PritamP20:fix-resource-leaks

Conversation

@PritamP20
Copy link
Copy Markdown
Contributor

@PritamP20 PritamP20 commented Jan 7, 2026


Description

This PR fixes multiple resource leaks across the JPF core codebase.
Several FileInputStream, FileReader, and BufferedReader instances were not being closed properly, which could lead to file handle exhaustion and memory leaks during long JPF runs.

fixes - #585

Changes Made

File Leaks Fixed Description
JPF_java_lang_System.java 1 Fixed unclosed FileInputStream in getSysPropsFromFile()
LogConsole.java 1 Fixed unclosed BufferedReader in run()
Config.java 2 Fixed FileInputStream leaks in loadProperties()
RepositoryEntry.java 6 Fixed unclosed readers in SVN/Hg/Git factory classes
JPFSiteUtils.java 4 Fixed FileReader/BufferedReader leaks in utility methods

Total: 14 resource leaks fixed.


Example

Before

FileInputStream fis = new FileInputStream(cf);
p.load(fis);
// never closed
return getProperties(env, p);

After

try (FileInputStream fis = new FileInputStream(cf)) {
    p.load(fis);
}
return getProperties(env, p);

Testing

✅ All 1,030 tests passed
✅ Verified with ./gradlew buildJars test
✅ Manually tested with java -jar build/RunJPF.jar src/examples/Racer.jpf
✅ No functional or behavioral changes

Test Summary:

  • Parallel tests: 1014 passed, 0 failed
  • Single-threaded tests: 16 passed, 0 failed

Impact

  • Prevents memory leaks and file handle exhaustion
  • Simplifies cleanup logic (removed multiple finally blocks)
  • Improves code readability and safety
  • No behavior change — purely a cleanup/refactor

Checklist

  • Code compiles and runs successfully
  • All tests pass
  • No new dependencies added
  • Follows existing code style
  • Backward compatible

@PritamP20
Copy link
Copy Markdown
Contributor Author

PritamP20 commented Jan 7, 2026

@cyrille-artho or @Mahmoud-Khawaja can you please review this

@Mahmoud-Khawaja
Copy link
Copy Markdown
Contributor

Hi Pritam,
thank you for your contribution. sorry I’m not quite active during this time as im going through exams. but great catch! I see this is a good fix and it will help prevent memory leaks. However, you’ll need to wait for @cyrille-artho approval for the merge

@PritamP20
Copy link
Copy Markdown
Contributor Author

@Mahmoud-Khawaja Sure, thank's for the feedback, if there are any improvement with this PR if you feel, please let me konw

@PritamP20
Copy link
Copy Markdown
Contributor Author

@cyrille-artho can you please review it

@PritamP20
Copy link
Copy Markdown
Contributor Author

@cyrille-artho can you please review this

Copy link
Copy Markdown
Member

@cyrille-artho cyrille-artho left a comment

Choose a reason for hiding this comment

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

Thank you for your effort with various code involving try.
Can you please elaborate on the new one-line try blocks that have neither a catch nor finally block associated to them?
Furthermore, why remove the lines br.close() in several places?
It may be a good idea to split your PR into smaller ones so we can review the changes one by one.

}
}
}
br.close();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please reinstate the lines that close br as to ensure efficient resource handling.

@PritamP20
Copy link
Copy Markdown
Contributor Author

@cyrille-artho
Thanks for the review!

  1. The one-line try statements are try-with-resources blocks (Java 7+). They automatically close resources that implement AutoCloseable, so explicit catch/finally blocks aren’t required.
  2. The explicit br.close() calls were removed because the compiler already generates them when using try-with-resources. Re-adding them wouldn’t change behavior but would be redundant.

If you prefer keeping the explicit close() calls for clarity, I can restore them please let me know your preference.

@cyrille-artho
Copy link
Copy Markdown
Member

Sorry for the late response; various factors coincided that prevented me from getting back to this earlier.
Thank you for the explanations. We can merge this PR.

@cyrille-artho cyrille-artho merged commit c450e0b into javapathfinder:master Mar 25, 2026
2 checks passed
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.

3 participants