From 4b26224c33e3854454ab95fc69f850b59322a1c3 Mon Sep 17 00:00:00 2001 From: Ryan Schmitt Date: Wed, 22 Apr 2026 17:13:30 -0700 Subject: [PATCH] Fix race conditions in TlsHandshakeTimeoutServer `close()` did not wait for the server thread to finish, so it could read stale state (`requestReceived`, `throwable`) while the thread was still running. This caused two intermittent failures: - "Never received a request": `close()` checked requestReceived before the server thread set it after accept() returned. - "Exception thrown while TlsHandshakeTimerOuter was running": the server thread got an expected IOException or BUFFER_UNDERFLOW when the client timed out and disconnected, and `close()` rethrew it. Fix: store the Thread reference and `join()` it in `close()` after closing the sockets. Remove the `requestReceived` and `throwable` checks since all server-side exceptions are expected (the client is designed to time out) and client-side assertions already validate correctness. --- .../tls/TlsHandshakeTimeoutServer.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/tls/TlsHandshakeTimeoutServer.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/tls/TlsHandshakeTimeoutServer.java index 8f34d657e9..1c7fec8c52 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/tls/TlsHandshakeTimeoutServer.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/tls/TlsHandshakeTimeoutServer.java @@ -54,10 +54,9 @@ public class TlsHandshakeTimeoutServer implements Closeable { private final boolean sendServerHello; private volatile int port = -1; - private volatile boolean requestReceived = false; private volatile ServerSocketChannel serverSocket; private volatile SocketChannel socket; - private volatile Throwable throwable; + private Thread thread; public TlsHandshakeTimeoutServer(final boolean sendServerHello) { this.sendServerHello = sendServerHello; @@ -67,13 +66,13 @@ public void start() throws IOException { this.serverSocket = ServerSocketChannel.open(); this.serverSocket.bind(new InetSocketAddress("0.0.0.0", 0)); this.port = ((InetSocketAddress) this.serverSocket.getLocalAddress()).getPort(); - new Thread(this::run).start(); + this.thread = new Thread(this::run); + this.thread.start(); } private void run() { try { socket = serverSocket.accept(); - requestReceived = true; if (sendServerHello) { final SSLEngine sslEngine = initHandshake(); @@ -81,8 +80,9 @@ private void run() { receiveClientHello(sslEngine); sendServerHello(sslEngine); } - } catch (final Throwable t) { - this.throwable = t; + } catch (final Throwable ignored) { + // Expected: the client will time out and disconnect, or + // close() will close the server socket to unblock accept() } } @@ -155,10 +155,12 @@ public void close() { } catch (final IOException ignore) { } - if (throwable != null) { - throw new RuntimeException("Exception thrown while TlsHandshakeTimerOuter was running", throwable); - } else if (!requestReceived) { - throw new IllegalStateException("Never received a request"); + if (thread != null) { + try { + thread.join(5000); + } catch (final InterruptedException ignore) { + Thread.currentThread().interrupt(); + } } } }