fix: remediate try-with-resources DbConnectionFactory.getConnection() anti-pattern causing secondary connection leaks
@spbolton is already working on this.
Since Feb 24, 2026.
- 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
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.
Assessment
This issue has not been assessed yet.