dotCMS / dotCMS/core

fix: remediate try-with-resources DbConnectionFactory.getConnection() anti-pattern causing secondary connection leaks

Open
#34,489 2 comments 0 reactions 1 assignee View on GitHub

@spbolton is already working on this.

Since Feb 24, 2026.

dotCMS : Technical Debt Priority : 2 High Team : Enablement Type : Task
Dominant language
Java
Stars
970
Forks
486
Avg merge
3d 33m
Merged PRs (30d)
170

Description

Description

`try (Connection conn = DbConnectionFactory.getConnection())` is unsafe when called in a context where a connection already exists on the current thread. `getConnection()` returns the existing ThreadLocal connection; the try-with-resources block then calls `conn.close()` on exit, which invokes `DbConnectionFactory.closeSilently()` — returning the connection to HikariCP and clearing the ThreadLocal.

Any code that runs afterwards in the same call stack — or any connection acquired in an outer scope — now holds an invalidated ThreadLocal reference. Subsequent calls to `getConnection()` obtain a new connection from HikariCP. If the outer scope's connection lifecycle tracking used `isNewConnection=false`, it will not close this new connection. The result is a connection permanently checked out of HikariCP that is never returned.

This pattern contributes to background connection accumulation, particularly in error paths and concurrent operations. It is distinct from the primary experiment/variant leak (#34489 epic: #34837) but adds to the same RDS pressure.

Confirmed affected files
File Line Context
`DatabaseHealthEventManager.java` 235 Health check event handling
`DatabaseHealthCheck.java` 69 Database health probe
`DropOldContentletRunner.java` Old contentlet cleanup task
`UniqueFieldsValidationInitializer.java` Field validation on startup
`FolderFactoryImpl.java` `findSystemFolder()` — original scope of this issue
Fix

Replace `try (Connection conn = DbConnectionFactory.getConnection())` with `DbConnectionFactory.wrapConnection(() -> { ... })`, or ensure connection lifecycle is managed via `@CloseDBIfOpened` / `@WrapInTransaction` on the caller so ByteBuddy's advice handles the ThreadLocal correctly.

Do not close a connection obtained from `DbConnectionFactory.getConnection()` via try-with-resources unless the code guarantees no connection exists on the current thread at the point of acquisition.

Acceptance Criteria

  • `DatabaseHealthEventManager.java:235` — anti-pattern identified and remediated
  • `DatabaseHealthCheck.java:69` — anti-pattern identified and remediated
  • `DropOldContentletRunner.java` — anti-pattern identified and remediated
  • `UniqueFieldsValidationInitializer.java` — anti-pattern identified and remediated
  • `FolderFactoryImpl.findSystemFolder()` — original scope confirmed and remediated
  • Codebase-wide search for `try.*DbConnectionFactory.getConnection` run to identify any additional occurrences not listed above
  • All changes verified not to alter behaviour on the happy path (unit or integration test coverage)

Additional Context

Identified during production diagnosis of connection accumulation in `k8s-frankfurt-prod-1`. Connections matching this leak pattern appear in `pg_stat_activity` as idle for longer than `maxLifetime` — which is only possible if HikariCP has them marked `STATE_IN_USE` by a thread that has since moved on or terminated.

Epic: #34837
Analysis: `main/analysis/db-connection-pool-diagnosis-2026-03-02.md`

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.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.