Message-ID: From: "sehrope (@sehrope)" To: "pgjdbc/pgjdbc" Date: Sun, 31 May 2026 15:39:33 +0000 Subject: [pgjdbc/pgjdbc] PR #4120: Add connectThreadFactory and refactor Driver to use FutureTask for loginTimeout connection attempts List-Id: X-GitHub-Additions: 408 X-GitHub-Author-Id: 1690926 X-GitHub-Author-Login: sehrope X-GitHub-Base: master X-GitHub-Changed-Files: 7 X-GitHub-Commits: 4 X-GitHub-Deletions: 85 X-GitHub-Head-Branch: feat-conn-thread-factory-and-refactor-conn-task X-GitHub-Head-SHA: f1d168c9e87075439df1448ddea1580bf572d1de X-GitHub-Issue: 4120 X-GitHub-Merge-SHA: f1d168c9e87075439df1448ddea1580bf572d1de X-GitHub-Merged-By: sehrope X-GitHub-Repo: pgjdbc/pgjdbc X-GitHub-State: merged X-GitHub-Type: pull_request X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/pull/4120 Content-Type: text/plain; charset=utf-8 First commit adds a new `connectThreadFactoryFactory` connection property (with companion connectThreadFactoryFactoryArg) that lets callers supply a PGThreadFactoryFactory for the worker thread, following the existing xmlFactoryFactory pattern. When unset, DefaultPGThreadFactoryFactory reproduces the prior behavior (a daemon thread named "PostgreSQL JDBC driver connection thread"). This was originally done to facilitate testing of the interruptible scram patch (i.e., to allow the test to inject a custom thread factory). However it's genuinely useful on its own as users can now customize and name the background threads. For an app that is dynamically creating connections on the fly (e.g., a reporting SaaS) this would allow thread dumps with hanging connection threads to reflect things like the original user request. The second commit refactors the code path in Driver's connection creation with a login timeout. Previously we created a new thread, tried it in the background, and abandoned it if it hung past the login timeout. That was all done with some custom resource lock handling. The refactor uses a `FutureTask` for most of the work and allows the connection work to be interruptible. On abandonment it now calls `cancel(true)`, interrupting the worker thread rather than leaving it running until it finishes on its own. The existing race where the worker connects after the caller has given up is preserved (it closes any connection it establishes post-abandonment so nothing leaks). As part of the refactor we've also renamed `ConnectThread` to `ConnectTask` (it was never a thread...). This will not change anything immediately. But once we bump our scram library version to be interruptible it will allow scram PBKDF2 derivation work to respect a non-zero `loginTimeout`. Ditto for any other long / slow work in the connection thread (e.g., imagine an out of band OAuth flow that has an I/O wait, this would allow for proper interruption). Tests and checks pass. Would eventually need some changelog information, but will save that to be done in tandem with the scram update. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4137b06225..950f2922cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Security ### Added * feat: invalidate the prepared-statement cache after CREATE/DROP/ALTER so callers no longer trip on "cached plan must not change result type" without opting into `autosave=ALWAYS`. Controlled by the new `flushCacheOnDdl` connection property (default `true`); set to `false` for the prior behaviour. +* feat: add `connectThreadFactory` connection property to customize the `ThreadFactory` used to spawn the worker thread that runs the connection attempt when `loginTimeout` is in effect. The value is the fully qualified name of a class implementing `java.util.concurrent.ThreadFactory`. With a null value, the default, the driver retains the prior behavior of using a daemon thread named `"PostgreSQL JDBC driver connection thread"`. Useful for testing timeout behaviour or for applications that want detailed control of all driver-created threads. ### Changed +* refactor: the worker that runs the connection attempt under `loginTimeout` is now a `FutureTask` (`ConnectTask`) instead of the hand-rolled `ConnectThread`. When the caller hits the timeout, the task is now cancelled with `cancel(true)`, which interrupts the worker thread rather than letting it run to completion. This makes the connection attempt interruptible, so `loginTimeout` can stop a slow connection attempt instead of leaking a thread. As before, a connection that the worker still manages to establish after the caller gives up is closed by the worker so that it does not leak. There are no public API changes and this should only lead to faster background resource cleanup for connections that time out. * chore: `PGXAConnection.ConnectionHandler` now rejects `setAutoCommit(false)` and `setSavepoint(...)` during an active XA branch, in addition to the long-rejected `setAutoCommit(true)` / `commit()` / `rollback()`. The `setSavepoint` rejection was already meant to be in place but the guard misspelled the method name as `setSavePoint`, so savepoints silently went through. Both changes bring the proxy in line with JTA 1.2 ยง3.4. * 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. diff --git a/docs/content/documentation/use.md b/docs/content/documentation/use.md index ffb12a5a01..b87d221132 100644 --- a/docs/content/documentation/use.md +++ b/docs/content/documentation/use.md @@ -254,6 +254,13 @@ The timeout value in seconds that the driver will wait for a query to execute if * **`loginTimeout (`*int*`)`** *Default `0`*\ Specify how long to wait for establishment of a database connection. The timeout is specified in seconds max(2147484). +* **`connectThreadFactory (`*String*`)`** *Default `null`*\ +The fully qualified name of a class implementing `java.util.concurrent.ThreadFactory`. When `loginTimeout` is in effect, `Driver.connect(...)` uses the configured factory to produce the `ThreadFactory` that spawns the worker thread running the connection attempt. +With a null value, the driver uses a daemon thread named `"PostgreSQL JDBC driver connection thread"`. This indirection can be used to customize testing of timeout behavior or for applications that want detailed control of all driver created threads. + +* **`connectThreadFactoryArg (`*String*`)`** \ +An optional String argument passed to the constructor of the `connectThreadFactory` class. + * **`connectTimeout (`*int*`)`** *Default `10`*\ The timeout value used for socket connect operations. If connecting to the server takes longer than this value, the connection is broken. The timeout is specified in seconds max(2147484) and a value of zero means that it is disabled. diff --git a/pgjdbc/src/main/java/org/postgresql/Driver.java b/pgjdbc/src/main/java/org/postgresql/Driver.java index 72e5e50120..33c24e63eb 100644 --- a/pgjdbc/src/main/java/org/postgresql/Driver.java +++ b/pgjdbc/src/main/java/org/postgresql/Driver.java @@ -14,6 +14,7 @@ import org.postgresql.util.DriverInfo; import org.postgresql.util.GT; import org.postgresql.util.HostSpec; +import org.postgresql.util.ObjectFactory; import org.postgresql.util.PGPropertyUtil; import org.postgresql.util.PSQLException; import org.postgresql.util.PSQLState; @@ -38,8 +39,13 @@ import java.util.Enumeration; import java.util.Properties; import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.FutureTask; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Condition; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; @@ -288,19 +294,23 @@ private Properties loadDefaultProperties() throws IOException { // Enforce login timeout, if specified, by running the connection // attempt in a separate thread. If we hit the timeout without the // connection completing, we abandon the connection attempt in - // the calling thread, but the separate thread will keep trying. - // Eventually, the separate thread will either fail or complete - // the connection; at that point we clean up the connection if - // we managed to establish one after all. See ConnectThread for - // more details. + // the calling thread and try to cancel the worker thread. + // If cancellation does not take effect immediately, the worker + // cleans up any connection it manages to establish after + // abandonment. See ConnectTask for more details. long timeout = timeout(props); if (timeout <= 0) { return makeConnection(url, props); } - ConnectThread ct = new ConnectThread(url, props); - Thread thread = new Thread(ct, "PostgreSQL JDBC driver connection thread"); - thread.setDaemon(true); // Don't prevent the VM from shutting down + ConnectTask ct = new ConnectTask(url, props); + ThreadFactory threadFactory = resolveConnectThreadFactory(props); + Thread thread = threadFactory.newThread(ct); + if (thread == null) { + throw new PSQLException( + GT.tr("ThreadFactory returned a null Thread for the connection attempt."), + PSQLState.UNEXPECTED_ERROR); + } thread.start(); return ct.getResult(timeout); } catch (PSQLException ex1) { @@ -337,48 +347,36 @@ private void setupLoggerFromProperties(@SuppressWarnings("UnusedVariable") Prope /** * Perform a connect in a separate thread; supports getting the results from the original thread * while enforcing a login timeout. + * + *

If the caller times out or is interrupted, we mark the attempt as abandoned and try to + * cancel the worker thread. Cancellation is best-effort: if the connection attempt does not stop + * immediately, the worker closes any connection it manages to establish after abandonment so that + * it does not leak.

*/ - private static class ConnectThread implements Runnable { - private final ResourceLock lock = new ResourceLock(); - private final Condition lockCondition = lock.newCondition(); + private static class ConnectTask extends FutureTask { + private final AtomicBoolean abandoned; + private final AtomicReference<@Nullable Connection> establishedConnection; - ConnectThread(String url, Properties props) { - this.url = url; - this.props = props; + ConnectTask(String url, Properties props) { + this(new AtomicBoolean(), new AtomicReference<>(), url, props); } - @Override - public void run() { - Connection conn; - Throwable error; - - try { - conn = makeConnection(url, props); - error = null; - } catch (Throwable t) { - conn = null; - error = t; - } - - try (ResourceLock ignore = lock.obtain()) { - if (abandoned) { - if (conn != null) { - try { - conn.close(); - } catch (SQLException ignored) { - // TODO: should we rethrow it? - } - } - } else { - result = conn; - resultException = error; - lockCondition.signal(); + private ConnectTask(AtomicBoolean abandoned, AtomicReference<@Nullable Connection> establishedConnection, + String url, Properties props) { + super(() -> { + Connection conn = makeConnection(url, props); + establishedConnection.set(conn); + if (abandoned.get() && establishedConnection.compareAndSet(conn, null)) { + closeConnection(conn); } - } + return conn; + }); + this.abandoned = abandoned; + this.establishedConnection = establishedConnection; } /** - * Get the connection result from this (assumed running) thread. If the timeout is reached + * Get the connection result from this (assumed running) task. If the timeout is reached * without a result being available, a SQLException is thrown. * * @param timeout timeout in milliseconds @@ -386,53 +384,49 @@ public void run() { * @throws SQLException if a connection error occurs or the timeout is reached */ Connection getResult(long timeout) throws SQLException { - long expiry = TimeUnit.NANOSECONDS.toMillis(System.nanoTime()) + timeout; - try (ResourceLock ignore = lock.obtain()) { - while (true) { - if (result != null) { - return result; - } - - Throwable resultException = this.resultException; - if (resultException != null) { - if (resultException instanceof SQLException) { - resultException.fillInStackTrace(); - throw (SQLException) resultException; - } else { - throw new PSQLException( - GT.tr( - "Something unusual has occurred to cause the driver to fail. Please report this exception."), - PSQLState.UNEXPECTED_ERROR, resultException); - } - } - - long delay = expiry - TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); - if (delay <= 0) { - abandoned = true; - throw new PSQLException(GT.tr("Connection attempt timed out."), - PSQLState.CONNECTION_UNABLE_TO_CONNECT); - } - - try { - lockCondition.await(delay, TimeUnit.MILLISECONDS); - } catch (InterruptedException ie) { + try { + return get(timeout, TimeUnit.MILLISECONDS); + } catch (TimeoutException te) { + abandon(); + throw new PSQLException(GT.tr("Connection attempt timed out."), + PSQLState.CONNECTION_UNABLE_TO_CONNECT); + } catch (InterruptedException ie) { + abandon(); + + // reset the interrupt flag + Thread.currentThread().interrupt(); + + // throw an unchecked exception which will hopefully not be ignored by the calling code + throw new RuntimeException(GT.tr("Interrupted while attempting to connect.")); + } catch (ExecutionException ee) { + Throwable resultException = ee.getCause(); + if (resultException instanceof SQLException) { + resultException.fillInStackTrace(); + throw (SQLException) resultException; + } else { + throw new PSQLException( + GT.tr( + "Something unusual has occurred to cause the driver to fail. Please report this exception."), + PSQLState.UNEXPECTED_ERROR, resultException); + } + } + } - // reset the interrupt flag - Thread.currentThread().interrupt(); - abandoned = true; + private void abandon() { + abandoned.set(true); + cancel(true); + closeConnection(establishedConnection.getAndSet(null)); + } - // throw an unchecked exception which will hopefully not be ignored by the calling code - throw new RuntimeException(GT.tr("Interrupted while attempting to connect.")); - } + private static void closeConnection(@Nullable Connection conn) { + if (conn != null) { + try { + conn.close(); + } catch (SQLException ignored) { + // best-effort cleanup after abandonment } } } - - private final String url; - private final Properties props; - private @Nullable Connection result; - private @Nullable Throwable resultException; - private boolean abandoned; } /** @@ -712,6 +706,28 @@ private static HostSpec[] hostSpecs(Properties props) { return hostSpecs; } + private static final ThreadFactory DEFAULT_THREAD_FACTORY = r -> { + Thread thread = new Thread(r, "PostgreSQL JDBC driver connection thread"); + thread.setDaemon(true); // Don't prevent the VM from shutting down + return thread; + }; + + private static ThreadFactory resolveConnectThreadFactory(Properties props) + throws PSQLException { + String className = PGProperty.CONNECT_THREAD_FACTORY.getOrDefault(props); + if (className == null || className.isEmpty()) { + return DEFAULT_THREAD_FACTORY; + } + try { + return ObjectFactory.instantiate(ThreadFactory.class, className, props, true, + PGProperty.CONNECT_THREAD_FACTORY_ARG.getOrDefault(props)); + } catch (Exception ex) { + throw new PSQLException( + GT.tr("Could not instantiate connectThreadFactory: {0}", className), + PSQLState.INVALID_PARAMETER_VALUE, ex); + } + } + /** * @return the timeout from the URL, in milliseconds */ diff --git a/pgjdbc/src/main/java/org/postgresql/PGProperty.java b/pgjdbc/src/main/java/org/postgresql/PGProperty.java index 746a3be701..01782ae47c 100644 --- a/pgjdbc/src/main/java/org/postgresql/PGProperty.java +++ b/pgjdbc/src/main/java/org/postgresql/PGProperty.java @@ -169,6 +169,24 @@ public enum PGProperty { false, new String[]{"true", "false"}), + /** + * Factory class used to produce the helper thread used to enforce {@code loginTimeout} during + * connection establishment. Value must be the name of a class implementing {@link java.util.concurrent.ThreadFactory}. + * With a null value, which is the default, the driver uses a daemon thread named {@code "PostgreSQL JDBC driver connection thread"}. + */ + CONNECT_THREAD_FACTORY( + "connectThreadFactory", + null, + "Factory class to instantiate the Thread used for connection attempts with loginTimeout"), + + /** + * The String argument to give to the constructor of the connectThreadFactory class. + */ + CONNECT_THREAD_FACTORY_ARG( + "connectThreadFactoryArg", + null, + "Argument forwarded to constructor of connectThreadFactory class."), + /** * The timeout value used for socket connect operations. If connecting to the server takes longer * than this value, the connection is broken. diff --git a/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java b/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java index 1b0816e041..892f606cd9 100644 --- a/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java +++ b/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java @@ -1865,6 +1865,22 @@ public void setXmlFactoryFactory(@Nullable String xmlFactoryFactory) { PGProperty.XML_FACTORY_FACTORY.set(properties, xmlFactoryFactory); } + public @Nullable String getConnectThreadFactory() { + return PGProperty.CONNECT_THREAD_FACTORY.getOrDefault(properties); + } + + public void setConnectThreadFactory(@Nullable String connectThreadFactory) { + PGProperty.CONNECT_THREAD_FACTORY.set(properties, connectThreadFactory); + } + + public @Nullable String getConnectThreadFactoryArg() { + return PGProperty.CONNECT_THREAD_FACTORY_ARG.getOrDefault(properties); + } + + public void setConnectThreadFactoryArg(@Nullable String connectThreadFactoryArg) { + PGProperty.CONNECT_THREAD_FACTORY_ARG.set(properties, connectThreadFactoryArg); + } + public @Nullable String getPemKeyAlgorithm() { return PGProperty.PEM_KEY_ALGORITHM.getOrDefault(properties); } diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ConnectThreadFactoryTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ConnectThreadFactoryTest.java new file mode 100644 index 0000000000..6871280ef3 --- /dev/null +++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ConnectThreadFactoryTest.java @@ -0,0 +1,131 @@ +/* + * Copyright (c) 2026, PostgreSQL Global Development Group + * See the LICENSE file in the project root for more information. + */ + +package org.postgresql.test.jdbc2; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.postgresql.PGProperty; +import org.postgresql.test.TestUtil; + +import org.junit.jupiter.api.Test; + +import java.sql.Connection; +import java.sql.SQLException; +import java.util.Properties; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicInteger; + +public class ConnectThreadFactoryTest { + @Test + void customFactoryIsInvoked() throws Exception { + AtomicInteger threadsCreated = new AtomicInteger(); + ThreadFactory threadFactory = r -> { + threadsCreated.incrementAndGet(); + Thread t = new Thread(r, "ConnectThreadFactoryTest worker"); + t.setDaemon(true); + return t; + }; + + ThreadLocalThreadFactory.DELEGATE.set(threadFactory); + try { + Properties props = new Properties(); + PGProperty.LOGIN_TIMEOUT.set(props, "10"); + PGProperty.CONNECT_THREAD_FACTORY.set(props, + ThreadLocalThreadFactory.class.getName()); + + try (Connection conn = TestUtil.openDB(props)) { + assertNotNull(conn); + assertTrue(conn.isValid(1)); + } + + assertEquals(1, threadsCreated.get(), + "Configured ThreadFactory should produce exactly one thread for a connect attempt " + + "with loginTimeout > 0"); + } finally { + ThreadLocalThreadFactory.DELEGATE.remove(); + } + } + + @Test + void factoryArgMatching() throws Exception { + Properties props = new Properties(); + PGProperty.LOGIN_TIMEOUT.set(props, "10"); + PGProperty.CONNECT_THREAD_FACTORY.set(props, + ArgValidatingThreadFactory.class.getName()); + PGProperty.CONNECT_THREAD_FACTORY_ARG.set(props, + ArgValidatingThreadFactory.EXPECTED_ARG); + + try (Connection conn = TestUtil.openDB(props)) { + assertNotNull(conn); + assertTrue(conn.isValid(1)); + } + } + + @Test + void factoryArgMismatchFailsConnect() { + Properties props = new Properties(); + PGProperty.LOGIN_TIMEOUT.set(props, "10"); + PGProperty.CONNECT_THREAD_FACTORY.set(props, + ArgValidatingThreadFactory.class.getName()); + PGProperty.CONNECT_THREAD_FACTORY_ARG.set(props, "wrong-arg"); + + assertThrows(SQLException.class, () -> TestUtil.openDB(props), + "Connect should fail when the worker thread's run() rejects the configured arg"); + } + + /** + * Test-only {@link java.util.concurrent.ThreadFactory} that returns delegates to whatever the + * current thread has stashed in {@link #DELEGATE}. Lets each test install its own factory + * without sharing static state across tests. + */ + public static class ThreadLocalThreadFactory implements ThreadFactory { + static final ThreadLocal DELEGATE = new ThreadLocal<>(); + + @Override + public Thread newThread(Runnable r) { + ThreadFactory tf = DELEGATE.get(); + if (tf == null) { + throw new IllegalStateException( + "No ThreadFactory configured in ThreadLocalThreadFactory.DELEGATE"); + } + return tf.newThread(r); + } + } + + /** + * Test-only {@link java.util.concurrent.ThreadFactory} whose constructor captures the + * {@code connectThreadFactoryArg} String. The produced ThreadFactory wraps the + * connection task in a Runnable that throws if the captured arg does not match the expected + * value so the failure happens inside the worker thread's run() rather than in the + * factory's constructor. + */ + public static class ArgValidatingThreadFactory implements ThreadFactory { + static final String EXPECTED_ARG = "expected-arg-value"; + + private final String arg; + + public ArgValidatingThreadFactory(String arg) { + this.arg = arg; + } + + @Override + public Thread newThread(Runnable r) { + String capturedArg = this.arg; + Thread t = new Thread(() -> { + if (!EXPECTED_ARG.equals(capturedArg)) { + throw new IllegalArgumentException( + "Unexpected connectThreadFactoryArg: " + capturedArg); + } + r.run(); + }, "ArgValidatingThreadFactory worker"); + t.setDaemon(true); + return t; + } + } +} diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/LoginTimeoutInterruptTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/LoginTimeoutInterruptTest.java new file mode 100644 index 0000000000..fd2dac8918 --- /dev/null +++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/LoginTimeoutInterruptTest.java @@ -0,0 +1,133 @@ +/* + * Copyright (c) 2026, PostgreSQL Global Development Group + * See the LICENSE file in the project root for more information. + */ + +package org.postgresql.test.jdbc2; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.postgresql.PGProperty; +import org.postgresql.plugin.AuthenticationPlugin; +import org.postgresql.plugin.AuthenticationRequestType; +import org.postgresql.test.TestUtil; +import org.postgresql.util.PSQLException; +import org.postgresql.util.PSQLState; + +import org.junit.jupiter.api.Test; + +import java.sql.SQLException; +import java.util.Properties; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +/** + * Verifies that a non-zero {@code loginTimeout} interrupts a stuck connection attempt. The + * {@link AuthenticationPlugin#getPassword} call is interrupted out of {@link Thread#sleep} + * before the post-sleep code (which would throw a different exception) ever runs, and the + * driver surfaces the timeout instead of that post-sleep exception. + */ +public class LoginTimeoutInterruptTest { + private static final long PLUGIN_SLEEP_MS = 30_000; + private static final long LOGIN_TIMEOUT_SECONDS = 1; + private static final long WORKER_JOIN_TIMEOUT_MS = 5_000; + + @Test + void loginTimeoutInterruptsAuthPluginSleep() throws Exception { + SleepThenThrowAuthPlugin.PLUGIN_INVOKED.set(false); + SleepThenThrowAuthPlugin.INTERRUPTED_DURING_SLEEP.set(false); + SleepThenThrowAuthPlugin.POST_SLEEP_REACHED.set(false); + CapturingThreadFactory.CAPTURED.set(null); + + Properties props = new Properties(); + PGProperty.LOGIN_TIMEOUT.set(props, Long.toString(LOGIN_TIMEOUT_SECONDS)); + PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.set(props, + SleepThenThrowAuthPlugin.class.getName()); + PGProperty.CONNECT_THREAD_FACTORY.set(props, + CapturingThreadFactory.class.getName()); + + long startNanos = System.nanoTime(); + SQLException ex = assertThrows(SQLException.class, () -> TestUtil.openDB(props)); + long elapsedMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos); + + // Join the worker thread so its post-interrupt writes are visible here. If cancel(true) + // in ConnectTask.abandon() does not actually deliver an interrupt, the worker stays in + // Thread.sleep for the full PLUGIN_SLEEP_MS, join() times out, and isAlive() is still + // true. + Thread worker = CapturingThreadFactory.CAPTURED.get(); + assertNotNull(worker, "Custom ThreadFactory should have captured the worker thread"); + worker.join(WORKER_JOIN_TIMEOUT_MS); + assertFalse(worker.isAlive(), + "Worker thread should have exited after cancel(true) interrupted its Thread.sleep," + + " but is still alive after " + WORKER_JOIN_TIMEOUT_MS + " ms"); + + assertTrue(SleepThenThrowAuthPlugin.PLUGIN_INVOKED.get(), + "Auth plugin should have been invoked"); + assertTrue(SleepThenThrowAuthPlugin.INTERRUPTED_DURING_SLEEP.get(), + "Worker thread should have been interrupted out of Thread.sleep via cancel(true)" + + " in ConnectTask.abandon()"); + assertFalse(SleepThenThrowAuthPlugin.POST_SLEEP_REACHED.get(), + "Auth plugin should have been interrupted during sleep, not advanced past it"); + + // Should fail via loginTimeout, well before the plugin's sleep would have completed. + assertTrue(elapsedMs < PLUGIN_SLEEP_MS, + "Connect should be interrupted via loginTimeout, but took " + elapsedMs + " ms" + + " (plugin sleep is " + PLUGIN_SLEEP_MS + " ms)"); + + // The thrown SQLException should be the driver's timeout (CONNECTION_UNABLE_TO_CONNECT), + // not anything bubbled up from the plugin's post-sleep RuntimeException. + assertEquals(PSQLState.CONNECTION_UNABLE_TO_CONNECT.getState(), ex.getSQLState(), + "Expected the loginTimeout SQLException, got: " + ex); + } + + /** + * Test-only {@link java.util.concurrent.ThreadFactory} that captures the worker thread the driver spawns + * for the connection attempt, so the test can {@link Thread#join} it and observe whether + * cancel(true) actually caused the worker to exit. + */ + public static class CapturingThreadFactory implements ThreadFactory { + static final AtomicReference CAPTURED = new AtomicReference<>(); + + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r, "LoginTimeoutInterruptTest worker"); + t.setDaemon(true); + CAPTURED.set(t); + return t; + } + } + + /** + * Test-only {@link AuthenticationPlugin} that sleeps for a long time and then throws. If the + * sleep is interrupted (the expected case under {@code loginTimeout}), it rethrows the + * interrupt as a PSQLException without ever executing the post-sleep statements. + */ + public static class SleepThenThrowAuthPlugin implements AuthenticationPlugin { + static final AtomicBoolean PLUGIN_INVOKED = new AtomicBoolean(); + static final AtomicBoolean INTERRUPTED_DURING_SLEEP = new AtomicBoolean(); + static final AtomicBoolean POST_SLEEP_REACHED = new AtomicBoolean(); + + @Override + public char[] getPassword(AuthenticationRequestType type) throws PSQLException { + PLUGIN_INVOKED.set(true); + try { + Thread.sleep(PLUGIN_SLEEP_MS); + } catch (InterruptedException e) { + INTERRUPTED_DURING_SLEEP.set(true); + Thread.currentThread().interrupt(); + throw new PSQLException("SleepThenThrowAuthPlugin interrupted during sleep", + PSQLState.UNEXPECTED_ERROR, e); + } + POST_SLEEP_REACHED.set(true); + throw new RuntimeException( + "SleepThenThrowAuthPlugin post-sleep throw should not be reached when " + + "loginTimeout interrupts the connection attempt"); + } + } +}