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 #4121: test: stabilise StatementTest.fastCloses on Windows
Date: Sun, 31 May 2026 16:51:29 +0000
Message-ID: <[email protected]> (raw)
### What
Make `StatementTest.fastCloses()` bound its work by wall-clock time instead of a fixed 1000 iterations, and clean up dead setup the test never used. Also apply an overflow-safe `nanoTime()` comparison to the neighbouring `sideStatementFinalizers()`.
### Why
`fastCloses()` intermittently hit its `@Timeout(40)` on Windows CI (for example [run 26582831581](https://github.com/pgjdbc/pgjdbc/actions/runs/26582831581/job/78320413863)). The test creates a fresh `Statement` per iteration and races `st.close()` (on another thread) against `st.executeQuery("select 1")`. When a close lands while the query is still in flight it cancels the query, and a cancel opens a **fresh socket** to the backend — the wire protocol mandates a separate connection for cancel requests (`QueryExecutorBase.sendQueryCancel`). Many short-lived sockets are expensive on Windows (slower connection setup, `TIME_WAIT` accumulation), so a fixed 1000-iteration loop can run past the 40s timeout. The main DB connection is already reused; the churn comes from the cancel path, which cannot reuse a connection.
### How
- Bound the loop by a 10s wall-clock budget (cap still 1000) so the close/execute race runs on every iteration but stops well inside the timeout on slow runners.
- Use the overflow-safe `System.nanoTime() - start < budget` form rather than `nanoTime() < deadline`, which can misbehave across a `nanoTime()` wrap-around.
- Shut down and await the executor in a `finally` block so background close tasks never leak into other tests.
- Drop copy-paste setup `fastCloses` never exercised: the `notify_then_sleep` function, the `client_min_messages` session setting, and the unused `cnt` histogram. These came from the adjacent `closeInProgressStatement` test, whose query actually calls the function; `fastCloses` has always run `select 1` (dead since the original 2017 commit that fixed #1022). `notify_then_sleep` remains in use by `warningsAreAvailableAsap` and `closeInProgressStatement`.
- Apply the same overflow-safe `nanoTime` comparison to `sideStatementFinalizers`.
### How to verify
```
./gradlew :postgresql:test --tests 'org.postgresql.test.jdbc2.StatementTest'
```
Local run (PostgreSQL 16): full `StatementTest` 43/43 green; `fastCloses` ~1s, `sideStatementFinalizers` ~2s (its budget). `compileTestJava` and `checkstyleTest` pass.
### All Submissions:
* [x] Have you followed the guidelines in our [Contributing](https://github.com/pgjdbc/pgjdbc/blob/master/CONTRIBUTING.md) document?
* [x] Have you checked to ensure there aren't other open [Pull Requests](../../pulls) for the same update/change?
### Changes to Existing Features:
* [x] Does this break existing behaviour? No — test-only change, no production code touched.
* [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? N/A — this stabilises existing tests.
* [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/test/jdbc2/StatementTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/StatementTest.java
index ba10ef78b2..3e9ba6e484 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/StatementTest.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/StatementTest.java
@@ -41,9 +41,7 @@
import java.sql.Statement;
import java.time.Duration;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.List;
-import java.util.Map;
import java.util.Properties;
import java.util.Random;
import java.util.concurrent.Callable;
@@ -1007,59 +1005,48 @@ void concurrentIsValid() throws Throwable {
@Test
@Timeout(40)
- void fastCloses() throws SQLException {
+ void fastCloses() throws Exception {
+ // Closing a Statement from another thread while executeQuery() runs must never deadlock or
+ // hang. A close that lands while the query is still in flight cancels it, and every cancel
+ // opens a fresh socket to the backend (the protocol mandates a separate connection for cancel
+ // requests). Short-lived sockets are expensive on Windows, so the loop is bounded by wall-clock
+ // time rather than a fixed iteration count: the race runs on every iteration and stops once the
+ // time budget is spent, which keeps the test well inside the @Timeout on slow runners.
ExecutorService executor = Executors.newSingleThreadExecutor();
- con.createStatement().execute("SET SESSION client_min_messages = 'NOTICE'");
- con.createStatement()
- .execute("CREATE OR REPLACE FUNCTION notify_then_sleep() RETURNS VOID AS "
- + "$BODY$ "
- + "BEGIN "
- + "RAISE NOTICE 'start';"
- + "EXECUTE pg_sleep(1);" // Note: timeout value does not matter here, we just test if test crashes or locks somehow
- + "END "
- + "$BODY$ "
- + "LANGUAGE plpgsql;");
- Map<String, Integer> cnt = new HashMap<>();
- final Random rnd = new Random();
- for (int i = 0; i < 1000; i++) {
- final Statement st = con.createStatement();
- executor.submit((Callable<Void>) () -> {
- int s = rnd.nextInt(10);
- if (s > 8) {
- try {
- Thread.sleep(0);
- } catch (InterruptedException ex) {
- // don't execute the close here as this thread was cancelled below in shutdownNow
+ try {
+ final Random rnd = new Random();
+ final long start = System.nanoTime();
+ final long budgetNanos = TimeUnit.SECONDS.toNanos(10);
+ // Subtract and compare to stay correct across a nanoTime() overflow; see its Javadoc.
+ int iterations = 0;
+ while (iterations < 1000 && System.nanoTime() - start < budgetNanos) {
+ iterations++;
+ try (Statement st = con.createStatement()) {
+ executor.submit((Callable<Void>) () -> {
+ if (rnd.nextInt(10) > 8) {
+ Thread.yield();
+ }
+ st.close();
return null;
+ });
+ st.executeQuery("select 1").close();
+ // Acceptable: the close landed before the query started or after it finished.
+ } catch (SQLException e) {
+ // The close may cancel the in-flight query (QUERY_CANCELED) or close the statement
+ // before execution begins (OBJECT_NOT_IN_STATE); any other state is a failure.
+ String sqlState = e.getSQLState();
+ if (!PSQLState.OBJECT_NOT_IN_STATE.getState().equals(sqlState)
+ && !PSQLState.QUERY_CANCELED.getState().equals(sqlState)) {
+ fail("Query is expected to succeed or be cancelled via st.close(), got SQLState "
+ + sqlState + ": " + e.getMessage());
}
}
- st.close();
- return null;
- });
- ResultSet rs = null;
- String sqlState = "0";
- try {
- rs = st.executeQuery("select 1");
- // Acceptable
- } catch (SQLException e) {
- sqlState = e.getSQLState();
- if (!PSQLState.OBJECT_NOT_IN_STATE.getState().equals(sqlState)
- && !PSQLState.QUERY_CANCELED.getState().equals(sqlState)) {
- assertEquals(
- PSQLState.QUERY_CANCELED.getState(),
- e.getSQLState(),
- "Query is expected to be cancelled via st.close(), got " + e.getMessage()
- );
- }
- } finally {
- TestUtil.closeQuietly(rs);
- TestUtil.closeQuietly(st);
}
- Integer val = cnt.get(sqlState);
- val = (val == null ? 0 : val) + 1;
- cnt.put(sqlState, val);
+ assertTrue(iterations > 0, "fastCloses ran no iterations");
+ } finally {
+ executor.shutdown();
+ executor.awaitTermination(10, TimeUnit.SECONDS);
}
- executor.shutdown();
}
/**
@@ -1068,7 +1055,8 @@ void fastCloses() throws SQLException {
*/
@Test
void sideStatementFinalizers() throws SQLException {
- long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(2);
+ final long start = System.nanoTime();
+ final long budgetNanos = TimeUnit.SECONDS.toNanos(2);
final AtomicInteger leaks = new AtomicInteger();
final AtomicReference<Throwable> cleanupFailure = new AtomicReference<>();
@@ -1078,7 +1066,8 @@ void sideStatementFinalizers() throws SQLException {
cleaners.add(new LazyCleanerImpl("pgjdbc-test-cleaner-" + i, Duration.ofSeconds(2)));
}
- for (int q = 0; System.nanoTime() < deadline || leaks.get() < 10000; q++) {
+ // Subtract and compare to stay correct across a nanoTime() overflow; see its Javadoc.
+ for (int q = 0; System.nanoTime() - start < budgetNanos || leaks.get() < 10000; q++) {
for (int i = 0; i < 100; i++) {
PreparedStatement ps = con.prepareStatement("select " + (i + q));
ps.close();
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 #4121: test: stabilise StatementTest.fastCloses on Windows
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