pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feedFrom: sehrope (@sehrope) <[email protected]>
To: pgjdbc/pgjdbc <[email protected]>
Subject: [pgjdbc/pgjdbc] PR #4120: Add connectThreadFactoryFactory and refactor Driver to use FutureTask for loginTimeout connection attempts
Date: Sun, 31 May 2026 15:39:33 +0000
Message-ID: <[email protected]> (raw)
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/pgjdbc/src/main/java/org/postgresql/Driver.java b/pgjdbc/src/main/java/org/postgresql/Driver.java
index 72e5e50120..d87f80cb37 100644
--- a/pgjdbc/src/main/java/org/postgresql/Driver.java
+++ b/pgjdbc/src/main/java/org/postgresql/Driver.java
@@ -11,10 +11,13 @@
import org.postgresql.jdbc.ResourceLock;
import org.postgresql.jdbcurlresolver.PgPassParser;
import org.postgresql.jdbcurlresolver.PgServiceConfParser;
+import org.postgresql.util.DefaultPGThreadFactoryFactory;
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.PGThreadFactoryFactory;
import org.postgresql.util.PSQLException;
import org.postgresql.util.PSQLState;
import org.postgresql.util.SharedTimer;
@@ -38,8 +41,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 +296,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).newThreadFactory();
+ 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 +349,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.
+ *
+ * <p>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.</p>
*/
- private static class ConnectThread implements Runnable {
- private final ResourceLock lock = new ResourceLock();
- private final Condition lockCondition = lock.newCondition();
+ private static class ConnectTask extends FutureTask<Connection> {
+ 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 +386,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 +708,22 @@ private static HostSpec[] hostSpecs(Properties props) {
return hostSpecs;
}
+ private static PGThreadFactoryFactory resolveConnectThreadFactory(Properties props)
+ throws PSQLException {
+ String className = PGProperty.CONNECT_THREAD_FACTORY_FACTORY.getOrDefault(props);
+ if (className == null || className.isEmpty()) {
+ return DefaultPGThreadFactoryFactory.INSTANCE;
+ }
+ try {
+ return ObjectFactory.instantiate(PGThreadFactoryFactory.class, className, props, true,
+ PGProperty.CONNECT_THREAD_FACTORY_FACTORY_ARG.getOrDefault(props));
+ } catch (Exception ex) {
+ throw new PSQLException(
+ GT.tr("Could not instantiate connectThreadFactoryFactory: {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..c17e0175ab 100644
--- a/pgjdbc/src/main/java/org/postgresql/PGProperty.java
+++ b/pgjdbc/src/main/java/org/postgresql/PGProperty.java
@@ -169,6 +169,26 @@ public enum PGProperty {
false,
new String[]{"true", "false"}),
+ /**
+ * Factory class used to produce the {@link java.util.concurrent.ThreadFactory} that spawns the
+ * helper thread used to enforce {@code loginTimeout} during connection establishment. Value must
+ * be the name of a class implementing {@link org.postgresql.util.PGThreadFactoryFactory}. When
+ * unset, the driver uses a daemon thread named
+ * {@code "PostgreSQL JDBC driver connection thread"}.
+ */
+ CONNECT_THREAD_FACTORY_FACTORY(
+ "connectThreadFactoryFactory",
+ "",
+ "Factory class to instantiate the ThreadFactory used for connection attempts with loginTimeout"),
+
+ /**
+ * The String argument to give to the constructor of the connectThreadFactoryFactory class.
+ */
+ CONNECT_THREAD_FACTORY_FACTORY_ARG(
+ "connectThreadFactoryFactoryArg",
+ null,
+ "Argument forwarded to constructor of connectThreadFactoryFactory 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..dae0f3c784 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 String getConnectThreadFactoryFactory() {
+ return castNonNull(PGProperty.CONNECT_THREAD_FACTORY_FACTORY.getOrDefault(properties));
+ }
+
+ public void setConnectThreadFactoryFactory(@Nullable String connectThreadFactoryFactory) {
+ PGProperty.CONNECT_THREAD_FACTORY_FACTORY.set(properties, connectThreadFactoryFactory);
+ }
+
+ public @Nullable String getConnectThreadFactoryFactoryArg() {
+ return PGProperty.CONNECT_THREAD_FACTORY_FACTORY_ARG.getOrDefault(properties);
+ }
+
+ public void setConnectThreadFactoryFactoryArg(@Nullable String connectThreadFactoryFactoryArg) {
+ PGProperty.CONNECT_THREAD_FACTORY_FACTORY_ARG.set(properties, connectThreadFactoryFactoryArg);
+ }
+
public @Nullable String getPemKeyAlgorithm() {
return PGProperty.PEM_KEY_ALGORITHM.getOrDefault(properties);
}
diff --git a/pgjdbc/src/main/java/org/postgresql/util/DefaultPGThreadFactoryFactory.java b/pgjdbc/src/main/java/org/postgresql/util/DefaultPGThreadFactoryFactory.java
new file mode 100644
index 0000000000..1e599c03d9
--- /dev/null
+++ b/pgjdbc/src/main/java/org/postgresql/util/DefaultPGThreadFactoryFactory.java
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2026, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.util;
+
+import java.util.concurrent.ThreadFactory;
+
+public class DefaultPGThreadFactoryFactory implements PGThreadFactoryFactory {
+ public static final DefaultPGThreadFactoryFactory INSTANCE = new DefaultPGThreadFactoryFactory();
+
+ public DefaultPGThreadFactoryFactory() {
+ }
+
+ @Override
+ public ThreadFactory newThreadFactory() {
+ return r -> {
+ Thread thread = new Thread(r, "PostgreSQL JDBC driver connection thread");
+ thread.setDaemon(true); // Don't prevent the VM from shutting down
+ return thread;
+ };
+ }
+}
diff --git a/pgjdbc/src/main/java/org/postgresql/util/PGThreadFactoryFactory.java b/pgjdbc/src/main/java/org/postgresql/util/PGThreadFactoryFactory.java
new file mode 100644
index 0000000000..e92ddae51b
--- /dev/null
+++ b/pgjdbc/src/main/java/org/postgresql/util/PGThreadFactoryFactory.java
@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2026, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.util;
+
+import java.util.concurrent.ThreadFactory;
+
+/**
+ * Factory of {@link ThreadFactory} instances used by the driver when spawning helper threads for
+ * connection establishment (for example, enforcing {@code loginTimeout}).
+ */
+public interface PGThreadFactoryFactory {
+ ThreadFactory newThreadFactory();
+}
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ConnectThreadFactoryFactoryTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ConnectThreadFactoryFactoryTest.java
new file mode 100644
index 0000000000..3525bdb228
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/ConnectThreadFactoryFactoryTest.java
@@ -0,0 +1,134 @@
+/*
+ * 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.postgresql.util.PGThreadFactoryFactory;
+
+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 ConnectThreadFactoryFactoryTest {
+ @Test
+ void customFactoryIsInvoked() throws Exception {
+ AtomicInteger threadsCreated = new AtomicInteger();
+ ThreadFactory threadFactory = r -> {
+ threadsCreated.incrementAndGet();
+ Thread t = new Thread(r, "ConnectThreadFactoryFactoryTest worker");
+ t.setDaemon(true);
+ return t;
+ };
+
+ ThreadLocalFactoryFactory.DELEGATE.set(threadFactory);
+ try {
+ Properties props = new Properties();
+ PGProperty.LOGIN_TIMEOUT.set(props, "10");
+ PGProperty.CONNECT_THREAD_FACTORY_FACTORY.set(props,
+ ThreadLocalFactoryFactory.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 {
+ ThreadLocalFactoryFactory.DELEGATE.remove();
+ }
+ }
+
+ @Test
+ void factoryArgMatching() throws Exception {
+ Properties props = new Properties();
+ PGProperty.LOGIN_TIMEOUT.set(props, "10");
+ PGProperty.CONNECT_THREAD_FACTORY_FACTORY.set(props,
+ ArgValidatingFactoryFactory.class.getName());
+ PGProperty.CONNECT_THREAD_FACTORY_FACTORY_ARG.set(props,
+ ArgValidatingFactoryFactory.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_FACTORY.set(props,
+ ArgValidatingFactoryFactory.class.getName());
+ PGProperty.CONNECT_THREAD_FACTORY_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 PGThreadFactoryFactory} that returns whatever {@link ThreadFactory} the
+ * current thread has stashed in {@link #DELEGATE}. Lets each test install its own factory
+ * without sharing static state across tests.
+ */
+ public static class ThreadLocalFactoryFactory implements PGThreadFactoryFactory {
+ static final ThreadLocal<ThreadFactory> DELEGATE = new ThreadLocal<>();
+
+ @Override
+ public ThreadFactory newThreadFactory() {
+ ThreadFactory tf = DELEGATE.get();
+ if (tf == null) {
+ throw new IllegalStateException(
+ "No ThreadFactory configured in ThreadLocalFactoryFactory.DELEGATE");
+ }
+ return tf;
+ }
+ }
+
+ /**
+ * Test-only {@link PGThreadFactoryFactory} whose constructor captures the
+ * {@code connectThreadFactoryFactoryArg} 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 ArgValidatingFactoryFactory implements PGThreadFactoryFactory {
+ static final String EXPECTED_ARG = "expected-arg-value";
+
+ private final String arg;
+
+ public ArgValidatingFactoryFactory(String arg) {
+ this.arg = arg;
+ }
+
+ @Override
+ public ThreadFactory newThreadFactory() {
+ String capturedArg = this.arg;
+ return r -> {
+ Thread t = new Thread(() -> {
+ if (!EXPECTED_ARG.equals(capturedArg)) {
+ throw new IllegalArgumentException(
+ "Unexpected connectThreadFactoryFactoryArg: " + capturedArg);
+ }
+ r.run();
+ }, "ArgValidatingFactoryFactory 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..ca99aa0481
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/LoginTimeoutInterruptTest.java
@@ -0,0 +1,136 @@
+/*
+ * 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.PGThreadFactoryFactory;
+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);
+ CapturingThreadFactoryFactory.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_FACTORY.set(props,
+ CapturingThreadFactoryFactory.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 = CapturingThreadFactoryFactory.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 PGThreadFactoryFactory} 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 CapturingThreadFactoryFactory implements PGThreadFactoryFactory {
+ static final AtomicReference<Thread> CAPTURED = new AtomicReference<>();
+
+ @Override
+ public ThreadFactory newThreadFactory() {
+ return 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");
+ }
+ }
+}
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 #4120: Add connectThreadFactoryFactory and refactor Driver to use FutureTask for loginTimeout connection attempts
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