pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feedFrom: vlsi (@vlsi) <[email protected]>
To: pgjdbc/pgjdbc <[email protected]>
Subject: [pgjdbc/pgjdbc] PR #4156: fix: avoid nulling contextClassLoader on shared commonPool workers
Date: Thu, 04 Jun 2026 19:35:43 +0000
Message-ID: <[email protected]> (raw)
## Why
The cleanup task in `LazyCleanerImpl` (the Java 8 variant) submits a task to `ForkJoinPool.commonPool()` and set the worker's `contextClassLoader` to `null` without ever restoring it. Common-pool workers are shared across the whole JVM and reused, so an unrelated task scheduled later on the same worker ran with a `null` `contextClassLoader`. Libraries that rely on the thread context classloader then break — the reporter hit this with reflections8. See [Issue #4155](https://github.com/pgjdbc/pgjdbc/issues/4155).
The `null` was added for [Issue #3953](https://github.com/pgjdbc/pgjdbc/issues/3953): a guard against a long-lived worker pinning the submitter's classloader (for example a web application's).
## What
- `LazyCleanerImpl` (Java 8 variant) now clears the `contextClassLoader` on the **submitting** thread only, around the `execute()` call, and restores it immediately afterwards. If the common pool spawns a fresh worker for the cleanup task, the worker is not born holding the caller's classloader, so the #3953 leak is still prevented — but the shared worker's `contextClassLoader` is never mutated, so #4155 is fixed.
- The in-task `setContextClassLoader` on the worker is gone, which also removes the `InnocuousForkJoinWorkerThread` `SecurityException` path from #3953.
- No change to the Java 11+ variant: it uses the native `Cleaner` and never touches the context classloader.
- Changelog entry under 42.7.12.
## How to verify
The cleanup `ForkJoinPool` path runs only under the Java 8 variant, so verification uses `-PjdkTestVersion=8`:
- `./gradlew -PjdkTestVersion=8 :pgjdbc-junit4-test:test --tests org.postgresql.test.jdbc4.jdbc41.DriverSupportsClassUnloadingTest` — green. This classloader-unloading test guards the #3953 leak; it fails if the worker is left holding the submitter's classloader.
- `./gradlew -PjdkTestVersion=8 :postgresql:test --tests org.postgresql.util.LazyCleanerTest` — green.
- `./gradlew -PenableCheckerframework :postgresql:classes :postgresql:checkstyleMain` — passes (nullness + style).
## Note
An earlier revision instead restored the **worker's** `contextClassLoader` in a `finally` block. That reintroduced the #3953 classloader leak and failed `DriverSupportsClassUnloadingTest` on Java 8, because restoring the value re-pinned the submitter's classloader on the long-lived worker. Moving the scrub to the submitting thread avoids both problems. The #4155 direction (a `null` loader leaking to unrelated tasks) has no direct regression test, since it would need a custom common-pool thread factory installed at JVM start.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4137b06225..2594a79d8b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
* chore: `commitPrepared` / `rollback`-of-prepared now return `XAER_RMFAIL` instead of `XAER_RMERR` when the underlying connection is left in a non-idle `TransactionState`. Transaction managers (Geronimo, Narayana, Atomikos) treat `XAER_RMFAIL` as retryable on a fresh `XAResource`; the prepared transaction is no longer abandoned.
### Fixed
+* fix: the driver no longer nulls the `contextClassLoader` of shared `ForkJoinPool.commonPool()` worker threads, which previously left unrelated tasks on those threads running with a `null` classloader [Issue #4155](https://github.com/pgjdbc/pgjdbc/issues/4155)
* fix: getCharacterStream wraps String in StringReader [PR #4063](https://github.com/pgjdbc/pgjdbc/pull/4063)
* fix: `PGXAConnection` no longer saves and restores the underlying connection's JDBC `autoCommit` flag. All XA-protocol SQL (`BEGIN`, `PREPARE TRANSACTION`, `COMMIT`, `ROLLBACK`, `COMMIT PREPARED`, `ROLLBACK PREPARED`, the `recover()` SELECT) is sent through `QUERY_SUPPRESS_BEGIN`, so the caller's `autoCommit` value is invariant across every `XAResource` call. Fixes the "2nd phase commit must be issued using an idle connection" failure during recovery on managed datasources that pool connections with `autoCommit=false` (TomEE, WildFly, WebSphere Liberty).
* fix: `PGXAConnection.prepare()` now mutates XA state only after `PREPARE TRANSACTION` succeeds. A failed `PREPARE` previously left the driver thinking the branch was already prepared, so the follow-up `rollback(xid)` tried `ROLLBACK PREPARED` against a non-existent gid and returned `XAER_RMERR`. Transaction managers (Narayana) escalated this to `HeuristicMixedException`. With the fix, `rollback(xid)` takes the active-branch path and issues a plain `ROLLBACK`, which the server accepts cleanly. Fixes [Issue #3153](https://github.com/pgjdbc/pgjdbc/issues/3153), [Issue #3123](https://github.com/pgjdbc/pgjdbc/issues/3123).
diff --git a/pgjdbc/src/main/java/org/postgresql/util/LazyCleanerImpl.java b/pgjdbc/src/main/java/org/postgresql/util/LazyCleanerImpl.java
index 6e6fbfa677..1b20c7ddf1 100644
--- a/pgjdbc/src/main/java/org/postgresql/util/LazyCleanerImpl.java
+++ b/pgjdbc/src/main/java/org/postgresql/util/LazyCleanerImpl.java
@@ -176,45 +176,61 @@ private boolean startThread() {
// We use ForkJoinPool to work around Thread.inheritedAccessControlContext memory leak
// Java creates FJP threads without caller's access control context, thus we reduce
// the surface for the leak.
- ForkJoinPool.commonPool().execute(
- () -> {
- // ForkJoinPool does not inherit contextClassLoader from the submitting thread,
- // however custom thread factories might, so we try nullifying it to avoid
- // classloader leaks. InnocuousForkJoinWorkerThread forbids setContextClassLoader,
- // so we catch SecurityException to avoid crashing the cleanup thread.
- // See https://github.com/pgjdbc/pgjdbc/issues/3953
- try {
- Thread.currentThread().setContextClassLoader(null);
- } catch (SecurityException ignore) {
- // InnocuousForkJoinWorkerThread or a SecurityManager forbids setContextClassLoader
- }
- RefQueueBlocker<Object> blocker =
- new RefQueueBlocker<>(queue, threadName, threadTtl);
- while (true) {
- try {
- ForkJoinPool.managedBlock(blocker);
- Node<?> ref = (Node<?>) blocker.drainOne();
- if (ref != null) {
- ref.onClean(true);
- } else if (checkEmpty()) {
- // The blocker waited the full threadTtl without receiving a ref, and there are
- // no pending registrations, so the cleanup task can terminate. Keeping the task
- // alive across transient empties amortizes ForkJoinPool submit/compensation
- // overhead under bursty workloads.
- break;
- }
- } catch (InterruptedException e) {
- if (checkEmpty()) {
- LOGGER.log(Level.FINE, "Got interrupt and the cleanup queue is empty, will terminate the cleanup thread");
- break;
+ //
+ // Null this (submitting) thread's contextClassLoader around execute(): if the common pool
+ // spawns a fresh worker to run the cleanup task, the worker inherits the submitter's
+ // contextClassLoader at birth. Leaving that classloader (which may be a web application's)
+ // on a worker that lives for the JVM's lifetime would pin it and leak it
+ // (https://github.com/pgjdbc/pgjdbc/issues/3953). We restore the value right after, so the
+ // change is scoped to our own call: we never mutate a shared common-pool worker, whose
+ // contextClassLoader unrelated tasks rely on (https://github.com/pgjdbc/pgjdbc/issues/4155).
+ Thread current = Thread.currentThread();
+ ClassLoader prevContextCL = current.getContextClassLoader();
+ try {
+ current.setContextClassLoader(null);
+ } catch (SecurityException ignore) {
+ // setContextClassLoader can be denied even without a SecurityManager: when this is an
+ // InnocuousForkJoinWorkerThread (a registration made from a common-pool task), it throws
+ // unconditionally. This is best-effort, so submit anyway with the original loader.
+ }
+ try {
+ ForkJoinPool.commonPool().execute(
+ () -> {
+ RefQueueBlocker<Object> blocker =
+ new RefQueueBlocker<>(queue, threadName, threadTtl);
+ while (true) {
+ try {
+ ForkJoinPool.managedBlock(blocker);
+ Node<?> ref = (Node<?>) blocker.drainOne();
+ if (ref != null) {
+ ref.onClean(true);
+ } else if (checkEmpty()) {
+ // The blocker waited the full threadTtl without receiving a ref, and there are
+ // no pending registrations, so the cleanup task can terminate. Keeping the task
+ // alive across transient empties amortizes ForkJoinPool submit/compensation
+ // overhead under bursty workloads.
+ break;
+ }
+ } catch (InterruptedException e) {
+ if (checkEmpty()) {
+ LOGGER.log(Level.FINE, "Got interrupt and the cleanup queue is empty, will terminate the cleanup thread");
+ break;
+ }
+ LOGGER.log(Level.FINE, "Got interrupt and the cleanup queue is NOT empty. Will ignore the interrupt");
+ } catch (Throwable e) {
+ LOGGER.log(Level.WARNING, "Unexpected exception while executing onClean", e);
}
- LOGGER.log(Level.FINE, "Got interrupt and the cleanup queue is NOT empty. Will ignore the interrupt");
- } catch (Throwable e) {
- LOGGER.log(Level.WARNING, "Unexpected exception while executing onClean", e);
}
}
- }
- );
+ );
+ } finally {
+ try {
+ current.setContextClassLoader(prevContextCL);
+ } catch (SecurityException ignore) {
+ // Denied by an InnocuousForkJoinWorkerThread or a SecurityManager, as above; the loader
+ // was never changed, so there is nothing to restore.
+ }
+ }
return true;
}
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: github://pgjdbc/pgjdbc
Cc: [email protected], [email protected]
Subject: Re: [pgjdbc/pgjdbc] PR #4156: fix: avoid nulling contextClassLoader on shared commonPool workers
In-Reply-To: <<[email protected]>>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox