Make DbConnectionFactory.getConnection() try-with-resources safe
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 970
- Forks
- 486
- Avg merge
- 3d 33m
- Merged PRs (30d)
- 170
Description
Description
DbConnectionFactory.getConnection() returns a ThreadLocal-managed connection. Using it in try-with-resources causes close() to run on that connection when the block exits, which corrupts ThreadLocal state and can lead to connection leaks or pool exhaustion.
Problem: Callers (e.g. DatabaseHealthCheck, DatabaseHealthEventManager) use:
try (Connection conn = DbConnectionFactory.getConnection()) { ... }
When close() runs, it returns the connection to the pool but leaves a stale reference in the ThreadLocal, or worse, removes the wrong connection from the holder.
Proposed solution: Add a wrapper (e.g. TryWithResourcesSafeConnection) that only actually closes the connection when it was opened by the current call — matching the semantics of @CloseDBIfOpened. When ownsConnection is true, close() removes from ThreadLocal and closes. When false, close() is a no-op.
Acceptance Criteria
-
getConnection()returns a wrapper that is safe for try-with-resources - Closing the wrapper only closes the connection when this call opened it (no prior connection existed)
- Existing callers using
closeSilently()/closeAndCommit()continue to work unchanged - Identity checks (e.g.
DbConnectionFactory.getConnection() != connection) still work (same wrapper instance returned for existing connection) -
closeConnection()handles the wrapper correctly (no ConcurrentModificationException)
Additional Context
- Related to PR #34942 (DatabaseHealthCheck robustness) — this fix was reverted from that PR to keep scope focused
DotConnectionWrapperandDotJobConnectionWrapperexist; the new wrapper would extendDotConnectionWrapperand overrideclose()
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at DbConnectionFactory.getConnection(), then inspect DotConnectionWrapper, DotJobConnectionWrapper, and callers such as DatabaseHealthCheck and DatabaseHealthEventManager. Trace ownership, ThreadLocal handling, closeConnection(), closeSilently(), and closeAndCommit(). Done means try-with-resources is safe while identity checks and existing close behavior remain correct.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100