RFR: 8376118: java/net/httpclient/StreamingBody.java fails intermittently on Windows

[Volkan Yazici]

On Thu, 22 Jan 2026 15:43:28 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

> This test has been observed failing intermittently in the CI, either in JTreg timeout, where the test passes successfully after the timeout has fired but while the failure handlers are still executing, or with an `SSLHandshakeException` caused by `"An established connection was aborted by the software in your host machine"`.
> 
> This test creates 500 clients and relies on the GC to close them (by design), because it wants to catch bugs where clients would be GC'ed too early. However, relying on the GC to close the clients can put pressure on resource allocation on the machine, which we suspect is the cause for the slow down and the test failures. @Michael-Mc-Mahon suggested we could try to relieve the pressure by making explicit calls to `System.gc()`, in the hope to reclaim the abandonned clients earlier.
> 
> This changes implements the suggestion by making calls to `System.gc()` at random interval from a separate thread, and converts the test to JUnit, making it stop at the first failure (which otherwise has a frustrating tendency to disappear in the JTreg Output Overflow).
> 
> With that change, I have not been able to observe the test failing again.

In `newRequestBuilder`, I see we explicitly set the version (along with the discovery flag) only for HTTP/3. Shouldn't we ideally be doing this for all versions to ensure that, say, the HTTP/2 test indeed uses HTTP/2?

test/jdk/java/net/httpclient/StreamingBody.java line 32:

> 30:  * @run junit/othervm
> 31:  *       -Djdk.httpclient.HttpClient.log=trace,headers,requests
> 32:  *       StreamingBody

Suggestion:

Suggestion:

 *       ${test.main.class}

test/jdk/java/net/httpclient/StreamingBody.java line 94:

> 92:     private static boolean stopAfterFirstFailure() {
> 93:         return true;
> 94:     }

Any particular reason that this is not just a constant, but a method?

test/jdk/java/net/httpclient/StreamingBody.java line 103:

> 101:         long nan = now % 1000_000;
> 102:         return String.format("[%d s, %d ms, %d ns] ", secs, mill, nan);
> 103:     }

*Nit:* You can also be done with just `import static java.time.Instant.now`.

test/jdk/java/net/httpclient/StreamingBody.java line 135:

> 133:     /// wants to verify that HttpClient instances which are no
> 134:     /// longer strongly referenced are not garbage collected
> 135:     /// before pending HTTP request are finished. The test

Suggestion:

    /// before pending HTTP requests are finished. The test

test/jdk/java/net/httpclient/StreamingBody.java line 151:

> 149:         private final Thread runner;
> 150:         private volatile boolean stop;
> 151:         private final Random RANDOM = RandomFactory.getRandom();

1. This can be `static`
2. Doesn't this warrant a `@key randomness`?

test/jdk/java/net/httpclient/StreamingBody.java line 226:

> 224:                         .build()
> 225:                         .sendAsync(request, BodyHandlers.ofInputStream())
> 226:                         .join();

Any particular reason we prefer `sendAsync().join()` over `send()`?

test/jdk/java/net/httpclient/StreamingBody.java line 228:

> 226:                         .join();
> 227: 
> 228:                 String body = new String(response.body().readAllBytes(), UTF_8);

AFAIU, we use an `HttpResponse<InputStream>` and convert it to string _after_ the `HttpResponse` instantiation, instead of directly reaching for `HttpResponse<InputStream>`, because the former does not immediately finalize the request. This creates better odds to leak resources, which we're trying to stress in this test. Assuming my interpretation is correct, you might consider dropping a comment briefly explaining this rationale.

test/jdk/java/net/httpclient/StreamingBody.java line 300:

> 298:         out.println("\n=========================");
> 299:         try {
> 300:             out.printf("%n%sCreated %d servers and %d clients%n",

*Nit:* You might consider using `%s` instead of `%d`, since the latter renders numbers in localized form, and can produce surprising outputs. (@dfuch, this was a nice lesson I learned from you. 🙏)

test/jdk/java/net/httpclient/StreamingBody.java line 313:

> 311:     }
> 312: 
> 313:     static class MessageHandler implements HttpTestHandler {

You can replace this class with `EchoHandler`.

-------------

PR Review: https://git.openjdk.org/jdk/pull/29366#pullrequestreview-3696394502
PR Review Comment: https://git.openjdk.org/jdk/pull/29366#discussion_r2720247052
PR Review Comment: https://git.openjdk.org/jdk/pull/29366#discussion_r2720265502
PR Review Comment: https://git.openjdk.org/jdk/pull/29366#discussion_r2720273882
PR Review Comment: https://git.openjdk.org/jdk/pull/29366#discussion_r2720292028
PR Review Comment: https://git.openjdk.org/jdk/pull/29366#discussion_r2720290856
PR Review Comment: https://git.openjdk.org/jdk/pull/29366#discussion_r2720459391
PR Review Comment: https://git.openjdk.org/jdk/pull/29366#discussion_r2720481565
PR Review Comment: https://git.openjdk.org/jdk/pull/29366#discussion_r2720491459
PR Review Comment: https://git.openjdk.org/jdk/pull/29366#discussion_r2720485094