From 5a93c3995a6dd7d335e1079477f02d114e2328ba Mon Sep 17 00:00:00 2001 From: Nassim Boutekedjiret Date: Wed, 7 Oct 2026 16:54:55 +0200 Subject: [PATCH 1/2] Re-arm the streaming deadline on an explicit reconnect (#198) In streaming mode, post() armed a fresh per-leg deadline, but the explicit transport.connect() that WsmanClient.send() makes after a dropped connection reused the previous leg's deadline. After a pause longer than the inactivity timeout, that deadline had expired and the TCP connect and TLS handshake got a 1 ms budget, surfacing as a spurious "No response from the WinRM service" timeout. connect() now arms the per-leg deadline itself, and post() goes through connect(). The pollTimeout (shared absolute deadline) and blocking (no deadline) modes are unchanged. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 +++ .../metricshub/winrm/light/HttpTransport.java | 18 ++++--- .../light/HttpTransportDeadlineTest.java | 47 +++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5631deb..52006a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,13 @@ Consequences: ### Fixed +- **A streaming read resumed after a long pause no longer times out spuriously** (#198). When a + streaming consumer (e.g. a `RemoteProcess` read slowly) paused longer than the inactivity + timeout and the host dropped the idle connection meanwhile, the reconnection inherited the + expired deadline of the previous request: its TCP connect and TLS handshake got a 1 ms budget + and failed with a timeout (*No response from the WinRM service*). Each + reconnection now gets a full inactivity timeout of its own. + - **WQL array properties keep all their elements** (#189). A WMI array (`IPAddress`, `DefaultIPGateway`, `Capabilities`, ...) used to yield only its last element, without any error. Its elements are now joined with `|` (`"192.0.2.10|fe80::1"`); the new diff --git a/src/main/java/org/metricshub/winrm/light/HttpTransport.java b/src/main/java/org/metricshub/winrm/light/HttpTransport.java index a178c55..3cb9a72 100644 --- a/src/main/java/org/metricshub/winrm/light/HttpTransport.java +++ b/src/main/java/org/metricshub/winrm/light/HttpTransport.java @@ -136,7 +136,8 @@ void pollTimeout(final int budgetMillis) { * A server that enforces the WSMan OperationTimeout by answering with the op-timeout fault * reaches the caller through that fault instead; both surface as the same timeout. *

- * The bound is absolute per request leg (armed at the start of each {@link #post}): one whole + * The bound is absolute per request leg (armed at the start of each {@link #post} and + * {@link #connect}): one whole * response must arrive within the inactivity timeout — a peer trickling bytes must not restart * the clock with every byte and hold a streaming fetch forever. * @@ -281,10 +282,18 @@ private boolean isStalePeerClosed() { * Establish (or validate) the connection now instead of lazily on the next {@link #post}. Lets * the caller separate "could not reach the endpoint" — where nothing has been sent and a retry * is provably safe — from a failure of a request that may already be executing. + *

+ * In streaming mode this starts a request leg of its own, with a fresh inactivity deadline: + * a reconnection after a long pause must not inherit the expired deadline of the previous leg. * * @throws IOException when the connection cannot be established */ void connect() throws IOException { + if (deadlinePerLeg) { + // Streaming mode: this whole leg — a reconnect included — must complete within the + // inactivity timeout, however many reads it takes (see inactivityTimeout(int)). + deadlineEpochMillis = Utils.getCurrentTimeMillis() + readTimeoutMillis; + } ensureConnected(); } @@ -344,12 +353,7 @@ private void ensureConnected() throws IOException { Response post(final String path, final byte[] body, final String contentType, final String authorization) throws IOException { - if (deadlinePerLeg) { - // Streaming mode: this whole leg — a reconnect included — must complete within the - // inactivity timeout, however many reads it takes (see inactivityTimeout(int)). - deadlineEpochMillis = Utils.getCurrentTimeMillis() + readTimeoutMillis; - } - ensureConnected(); + connect(); try { if (deadlineEpochMillis != 0) { // Several HTTP legs can run under one poll deadline (reconnect, authentication diff --git a/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java b/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java index 96590ea..2147b6c 100644 --- a/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java +++ b/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java @@ -20,6 +20,7 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -31,6 +32,7 @@ import java.net.Socket; import java.net.SocketTimeoutException; import java.nio.charset.StandardCharsets; +import javax.net.ssl.SSLContext; import org.junit.jupiter.api.Test; /** @@ -118,6 +120,51 @@ void aTricklingPeerCannotStretchAStreamingRoundTrip() throws Exception { } } + @Test + void aStreamingReconnectGetsAFreshDeadline() throws Exception { + try (ServerSocket server = new ServerSocket(0)) { + // Every connection: take the ClientHello, stay silent 200 ms, then hang up. + final Thread handler = new Thread( + () -> { + try { + while (true) { + try (Socket socket = server.accept()) { + socket.getInputStream().read(new byte[4096]); + Thread.sleep(200); + } + } + } catch (final IOException | InterruptedException ignored) { + // server socket closed: test over + } + }, + "hang-up-tls-server" + ); + handler.setDaemon(true); + handler.start(); + + final HttpTransport transport = new HttpTransport( + "127.0.0.1", + server.getLocalPort(), + 60_000, + SSLContext.getDefault().getSocketFactory(), + false + ); + try { + // A first streaming leg arms its deadline, then fails (the server hangs up)... + transport.inactivityTimeout(1_000); + assertThrows(IOException.class, () -> transport.post("/wsman", new byte[0], null, null)); + // ...and the consumer pauses past that leg's deadline before reconnecting explicitly. + Thread.sleep(1_100); + // The reconnection is a leg of its own: its handshake must get a fresh 1 s budget and + // see the hang-up, not time out on the expired deadline's 1 ms floor. + final IOException e = assertThrows(IOException.class, transport::connect); + assertFalse(e instanceof SocketTimeoutException, "the reconnection inherited an expired deadline: " + e); + } finally { + transport.close(); + } + } + } + /** * One connection: read the request head, then trickle the response one byte every 300 ms. * {@code SO_TIMEOUT} applies per read, so without an absolute bound every byte would reset the From ce0fdb1eaef7dc655b05b7c3be7eff81ab8d02f6 Mon Sep 17 00:00:00 2001 From: Nassim Boutekedjiret Date: Wed, 7 Oct 2026 19:18:25 +0200 Subject: [PATCH 2/2] Address review: positive assertion in the reconnect test, doc touch-ups Co-Authored-By: Claude Opus 5.5 --- .../java/org/metricshub/winrm/light/HttpTransport.java | 5 ++--- .../winrm/light/HttpTransportDeadlineTest.java | 9 +++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/metricshub/winrm/light/HttpTransport.java b/src/main/java/org/metricshub/winrm/light/HttpTransport.java index 3cb9a72..102a882 100644 --- a/src/main/java/org/metricshub/winrm/light/HttpTransport.java +++ b/src/main/java/org/metricshub/winrm/light/HttpTransport.java @@ -137,9 +137,8 @@ void pollTimeout(final int budgetMillis) { * reaches the caller through that fault instead; both surface as the same timeout. *

* The bound is absolute per request leg (armed at the start of each {@link #post} and - * {@link #connect}): one whole - * response must arrive within the inactivity timeout — a peer trickling bytes must not restart - * the clock with every byte and hold a streaming fetch forever. + * {@link #connect}): one whole response must arrive within the inactivity timeout — a peer + * trickling bytes must not restart the clock with every byte and hold a streaming fetch forever. * * @param inactivityTimeoutMillis the longest tolerated silence in milliseconds */ diff --git a/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java b/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java index 2147b6c..7ba05de 100644 --- a/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java +++ b/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java @@ -40,6 +40,9 @@ * span several HTTP round trips (a reconnect plus a re-authentication exchange), and every leg * must be capped by what is LEFT of the poll's budget — a peer answering each leg just fast enough * must not be able to stretch the poll to several multiples of the requested wait. + *

+ * Also covers the streaming {@link HttpTransport#inactivityTimeout(int)} deadline, armed afresh + * for every request leg, an explicit reconnection included. */ class HttpTransportDeadlineTest { @@ -157,8 +160,14 @@ void aStreamingReconnectGetsAFreshDeadline() throws Exception { Thread.sleep(1_100); // The reconnection is a leg of its own: its handshake must get a fresh 1 s budget and // see the hang-up, not time out on the expired deadline's 1 ms floor. + final long start = System.nanoTime(); final IOException e = assertThrows(IOException.class, transport::connect); + final long elapsedMillis = (System.nanoTime() - start) / 1_000_000; assertFalse(e instanceof SocketTimeoutException, "the reconnection inherited an expired deadline: " + e); + assertTrue( + elapsedMillis >= 150, + "the handshake must wait for the server's hang-up; took " + elapsedMillis + " ms" + ); } finally { transport.close(); }