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 #4122: test: fix LazyCleanerTest timeouts for the lingering Java 8 cleanup thread
Date: Sun, 31 May 2026 18:22:40 +0000
Message-ID: <[email protected]> (raw)
### What
Adjust the `LazyCleanerTest` await budgets and make its thread-lifecycle assertions adapt to the actually loaded cleaner implementation. No production code changes.
- Derive the thread-stop budget from `threadTtl` (`2 × ttl + 5s`) via a small `threadStopBudget` helper, instead of a hard-coded `10s`.
- Raise the cleanup-completion budget in `exceptionsDuringCleanupAreHandled` from `10s` to `30s`.
- Branch the thread-lifecycle assertions on the observed behaviour (`isThreadRunning()` captured after `register()`) rather than on the runtime Java version: the dedicated-thread variant must stop once the queue empties; the native-`Cleaner` variant never starts a thread.
- Apply the same `threadStopBudget` formula to `phantomCleaner` to remove the same latent risk.
### Why
`LazyCleanerImpl` now keeps its Java 8 cleanup thread alive for up to one `threadTtl` after the reference queue empties (05166d0fd, #4037). The worst case to observe `!isThreadRunning()` is therefore about `2 × threadTtl`: up to one window to pick up the final GC'd reference, plus a full window that must time out before the loop breaks.
The tests used `threadTtl = 5s` with an `Await` budget of exactly `10s = 2 × ttl` and zero slack. Once the CI matrix started occasionally running under `-XX:ActiveProcessorCount=1` (e06c64995), GC, the reference handler, and the ForkJoinPool worker share a single core, and the extra latency pushed two tests past the deadline:
- `cleanupCompletesAfterManualClean` — timed out waiting for `!isThreadRunning()`.
- `exceptionsDuringCleanupAreHandled` — timed out waiting for both cleanups to run (this one is bound by GC/enqueue latency, not `threadTtl`, hence the separate `30s` budget).
Reported failures: [run 26633780304](https://github.com/pgjdbc/pgjdbc/actions/runs/26633780304/job/78488849120) and [run 26715502613](https://github.com/pgjdbc/pgjdbc/actions/runs/26715502613/job/78733270691).
The active `LazyCleanerImpl` is selected by multi-release packaging, not by the runtime JDK. The Maven source-distribution build runs the PhantomReference-based Java 8 variant on any JDK, so the assertions cannot branch on `JavaVersion` — they detect the variant from its behaviour instead. The production thread TTL (30s default) is unchanged; the tests keep their `5s` value on purpose, since a lifecycle test using the production TTL would need a >60s budget.
### How to verify
Gradle, under a single CPU, on each JDK:
```
./gradlew :postgresql:test --tests org.postgresql.util.LazyCleanerTest \
-PjdkTestVersion=8 -PtestExtraJvmArgs=-XX:ActiveProcessorCount=1
./gradlew :postgresql:test --tests org.postgresql.util.LazyCleanerTest \
-PjdkTestVersion=11 -PtestExtraJvmArgs=-XX:ActiveProcessorCount=1
```
Source distribution (the Java 8 variant on a modern JDK, the path that failed in CI):
```
./gradlew sourceDistribution -Ppgjdbc.version=1.0 -Prelease -Psigning.pgp.enabled=OFF
cd pgjdbc/build/distributions && tar xzf postgresql-1.0-jdbc-src.tar.gz
cd postgresql-1.0-jdbc-src && mvn --batch-mode test -Dtest=LazyCleanerTest
```
Local results, all 4 tests pass:
- Gradle Java 8 and source-distribution Maven (dedicated thread): ~15s (one `threadTtl` window per test, within the 15s budget).
- Gradle Java 11 and Java 21 (native `Cleaner`): ~1.1s.
### Changes to Existing Features:
* [ ] Does this break existing behaviour? No — test-only change; production `LazyCleanerImpl` is untouched.
* [x] Have you added an explanation of what your changes do and why you'd like us to include them?
* [x] Have you written new tests for your core changes, as applicable? Adjusted the existing tests; assertions now adapt to the loaded cleaner variant.
* [x] Have you successfully run tests with your changes locally?
🤖 Generated with [Claude Code](https://claude.com/claude-code)
diff --git a/pgjdbc/src/test/java/org/postgresql/util/LazyCleanerTest.java b/pgjdbc/src/test/java/org/postgresql/util/LazyCleanerTest.java
index 9ba1850633..c1f1a7a778 100644
--- a/pgjdbc/src/test/java/org/postgresql/util/LazyCleanerTest.java
+++ b/pgjdbc/src/test/java/org/postgresql/util/LazyCleanerTest.java
@@ -24,6 +24,7 @@
import static java.time.Duration.ofSeconds;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -33,6 +34,7 @@
import org.junit.jupiter.api.Test;
+import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -46,7 +48,8 @@ void phantomCleaner() throws InterruptedException {
List<Object> list = new ArrayList<>(Arrays.asList(
new Object(), new Object(), new Object()));
- LazyCleanerImpl t = new LazyCleanerImpl("Cleaner", ofSeconds(5));
+ Duration ttl = ofSeconds(5);
+ LazyCleanerImpl t = new LazyCleanerImpl("Cleaner", ttl);
String[] collected = new String[list.size()];
List<LazyCleaner.Cleanable<RuntimeException>> cleaners = new ArrayList<>();
@@ -67,9 +70,13 @@ void phantomCleaner() throws InterruptedException {
);
}
+ // The active LazyCleanerImpl is selected by multi-release packaging, not by the runtime JDK
+ // (the Maven source-distribution build runs the Java 8 variant on any JDK), so branch on the
+ // observed behaviour rather than on JavaVersion.
+ boolean dedicatedThread = t.isThreadRunning();
if (JavaVersion.getRuntimeVersion() == JavaVersion.v1_8) {
- assertTrue(t.isThreadRunning(),
- "cleanup thread should be running, and it should wait for the leaks");
+ assertTrue(dedicatedThread,
+ "Java 8 always loads the PhantomReference-based cleaner, which runs a dedicated thread");
}
cleaners.get(1).clean();
@@ -82,14 +89,14 @@ void phantomCleaner() throws InterruptedException {
System.gc();
System.gc();
- if (JavaVersion.getRuntimeVersion() == JavaVersion.v1_8) {
+ if (dedicatedThread) {
Await.until(
- "The cleanup thread should detect leaks and terminate within 5-10 seconds after GC",
- ofSeconds(10),
+ "The cleanup thread should detect leaks and terminate after GC",
+ threadStopBudget(ttl),
() -> !t.isThreadRunning()
);
} else {
- // Allow some room for Java's Cleaner to clean the refs
+ // The native Cleaner has no dedicated thread to stop; give it room to process the refs
Thread.sleep(1000);
}
@@ -103,7 +110,8 @@ void phantomCleaner() throws InterruptedException {
@Test
void cleanupCompletesAfterManualClean() throws InterruptedException {
String threadName = UUID.randomUUID().toString();
- LazyCleanerImpl t = new LazyCleanerImpl(threadName, ofSeconds(5));
+ Duration ttl = ofSeconds(5);
+ LazyCleanerImpl t = new LazyCleanerImpl(threadName, ttl);
AtomicBoolean cleaned = new AtomicBoolean();
List<Object> list = new ArrayList<>();
@@ -115,9 +123,13 @@ void cleanupCompletesAfterManualClean() throws InterruptedException {
leak -> cleaned.set(true)
);
+ // The active LazyCleanerImpl is selected by multi-release packaging, not by the runtime JDK
+ // (the Maven source-distribution build runs the Java 8 variant on any JDK), so branch on the
+ // observed behaviour rather than on JavaVersion.
+ boolean dedicatedThread = t.isThreadRunning();
if (JavaVersion.getRuntimeVersion() == JavaVersion.v1_8) {
- assertTrue(t.isThreadRunning(),
- "cleanup thread should be running when there are objects to monitor");
+ assertTrue(dedicatedThread,
+ "Java 8 always loads the PhantomReference-based cleaner, which runs a dedicated thread");
}
// Manually clean the object
@@ -131,19 +143,23 @@ void cleanupCompletesAfterManualClean() throws InterruptedException {
System.gc();
System.gc();
- if (JavaVersion.getRuntimeVersion() == JavaVersion.v1_8) {
+ if (dedicatedThread) {
Await.until(
"Cleanup thread should stop when no objects remain",
- ofSeconds(10),
+ threadStopBudget(ttl),
() -> !t.isThreadRunning()
);
+ } else {
+ assertFalse(t.isThreadRunning(),
+ "Native Cleaner variant: there is no dedicated thread to stop");
}
}
@Test
@DisableLogger(LazyCleanerImpl.class)
void exceptionsDuringCleanupAreHandled() throws InterruptedException {
- LazyCleanerImpl t = new LazyCleanerImpl("test-cleaner", ofSeconds(5));
+ Duration ttl = ofSeconds(5);
+ LazyCleanerImpl t = new LazyCleanerImpl("test-cleaner", ttl);
java.util.concurrent.atomic.AtomicInteger cleanupCount = new java.util.concurrent.atomic.AtomicInteger(0);
List<Object> list = new ArrayList<>();
@@ -166,9 +182,13 @@ void exceptionsDuringCleanupAreHandled() throws InterruptedException {
leak -> secondCleaned.set(true)
);
+ // The active LazyCleanerImpl is selected by multi-release packaging, not by the runtime JDK
+ // (the Maven source-distribution build runs the Java 8 variant on any JDK), so branch on the
+ // observed behaviour rather than on JavaVersion.
+ boolean dedicatedThread = t.isThreadRunning();
if (JavaVersion.getRuntimeVersion() == JavaVersion.v1_8) {
- assertTrue(t.isThreadRunning(),
- "cleanup thread should be running when there are objects to monitor");
+ assertTrue(dedicatedThread,
+ "Java 8 always loads the PhantomReference-based cleaner, which runs a dedicated thread");
}
// Trigger cleanup
@@ -176,18 +196,25 @@ void exceptionsDuringCleanupAreHandled() throws InterruptedException {
System.gc();
System.gc();
+ // Cleanup completion is bound by GC and reference-enqueue latency, not by threadTtl. Allow a
+ // generous budget so the assertion holds even when the JVM runs on a single CPU
+ // (-XX:ActiveProcessorCount=1), where GC, the reference handler, and the ForkJoinPool worker
+ // all share one core.
Await.until(
"Both cleanups should complete despite exception",
- ofSeconds(10),
+ ofSeconds(30),
() -> cleanupCount.get() == 1 && secondCleaned.get()
);
- if (JavaVersion.getRuntimeVersion() == JavaVersion.v1_8) {
+ if (dedicatedThread) {
Await.until(
"Cleanup thread should stop after all objects are cleaned",
- ofSeconds(10),
+ threadStopBudget(ttl),
() -> !t.isThreadRunning()
);
+ } else {
+ assertFalse(t.isThreadRunning(),
+ "Native Cleaner variant: there is no dedicated thread to stop");
}
}
@@ -204,4 +231,18 @@ void exceptionOnCleanRethrowsToCaller() {
RuntimeException thrownException = assertThrows(RuntimeException.class, cleanable::clean);
assertSame(expectedException, thrownException, "Expected same exception instance to be thrown");
}
+
+ /**
+ * Worst-case time for the Java 8 cleanup thread to stop once its registry empties. The thread
+ * waits up to one {@code threadTtl} window to observe the final GC'd reference, then must time
+ * out a second full {@code threadTtl} window before the loop breaks (see {@code LazyCleanerImpl}).
+ * The extra seconds absorb scheduling slack when the JVM runs on a single CPU
+ * (-XX:ActiveProcessorCount=1).
+ *
+ * @param threadTtl the time-to-live configured for the cleaner
+ * @return the await budget to use for {@code !isThreadRunning()}
+ */
+ private static Duration threadStopBudget(Duration threadTtl) {
+ return threadTtl.multipliedBy(2).plus(ofSeconds(5));
+ }
}
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 #4122: test: fix LazyCleanerTest timeouts for the lingering Java 8 cleanup thread
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