dotCMS / dotCMS/core

Make DbConnectionFactory.getConnection() try-with-resources safe

Open
#34,948 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

dotCMS : Technical Debt stale Team : Enablement Triage Type : Refactoring
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
  • DotConnectionWrapper and DotJobConnectionWrapper exist; the new wrapper would extend DotConnectionWrapper and override close()

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.