RFR: 8369920: HttpClient QuicSelectorThread could be a VirtualThread

Mon Nov 3 21:19:55 UTC 2025

On Wed, 15 Oct 2025 15:58:23 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

> This change makes it possible to use a VirtualThread for the QuicSelectorThread. The default will be to use a VirtualThread on all platforms except the Windows platform. We might consider switching the default for Windows platform too once [JDK-8334574](https://bugs.openjdk.org/browse/JDK-8334574) is fixed.
> 
> The change should be transparent for users of the API. However, using a VirtualThread may result in subtle differences in thread scheduling and class loading, so this change also includes an escape hatch  (a non documented internal system property) that could be used to revert to a platform thread in case of unexpected issues.  That property may be removed in a future version of the JDK.

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicEndpoint.java line 148:

> 146:         MAX_BUFFERED_LOW = maxBufferLow;
> 147:         String useVtForSelector =
> 148:                 System.getProperty("jdk.internal.httpclient.quic.useVTForSelector", "default");

We already have a `jdk.internal.httpclient.quic.poller.usePlatformThreads`. To follow the same naming convention, you might consider renaming this to `j.i.h.q.selector.useVirtualThreads`.

src/java.net.http/share/classes/jdk/internal/net/http/quic/QuicSelector.java line 510:

> 508:     }
> 509: 
> 510:     private record QuicSelectorThread(Thread thread) {

AFAICS, all methods – `start()`, `join()`, `ofPlatform()`, `ofVirtual()`, and `of()` – can be `private`.

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved.

Suggestion:

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 54:

> 52:  * @bug 8369920
> 53:  * @summary Basic test to verify that the QuicSelector
> 54:  *          uses a VirtualThread

Suggestion:

 * @summary Verifies whether `QuicSelector` uses virtual threads when
            no explicit configuration is provided

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 57:

> 55:  * @library /test/lib /test/jdk/java/net/httpclient/lib
> 56:  * @build jdk.test.lib.net.SimpleSSLContext
> 57:  *        jdk.httpclient.test.lib.common.HttpServerAdapters

@sormuras, in this JUnit test, given these `@build` classes are already included in `@library`, do we still need this explicit `@build`?

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 66:

> 64:  * @bug 8369920
> 65:  * @summary Basic test to verify that the QuicSelector
> 66:  *          uses a VirtualThread

This summary contradicts with this particular `@test`. That is, this `@test` verifies `QuicSelector` does *not* use a virtual thread. While I agree that `@test id=...` hints about the semantics, having an accurate `@summary` would not hurt:

Suggestion:

 * @summary Verifies that `QuicSelector` does *not* use virtual threads
            when explicitly configured so

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 79:

> 77:  * @bug 8369920
> 78:  * @summary Basic test to verify that the QuicSelector
> 79:  *          uses a VirtualThread

Suggestion:

 * @summary Verifies that `QuicSelector` does *always* use virtual threads
            when explicitly configured so

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 92:

> 90:  * @bug 8369920
> 91:  * @summary Basic test to verify that the QuicSelector
> 92:  *          uses a VirtualThread

Suggestion:

 * @summary Verifies whether `QuicSelector` uses virtual threads when
            `default` is explicitly configured

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 105:

> 103:  * @bug 8369920
> 104:  * @summary Basic test to verify that the QuicSelector
> 105:  *          uses a VirtualThread

Suggestion:

 * @summary Verifies whether `QuicSelector` uses virtual threads when
            it is configured using an invalid value

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 115:

> 113:  */
> 114: // -Djava.security.debug=all
> 115: public class H3QuicVTTest implements HttpServerAdapters {

Suggestion:

class H3QuicVTTest implements HttpServerAdapters {

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 132:

> 130:     }
> 131: 
> 132:     static boolean isQuicSelectorThreadVirtual() {

Suggestion:

    private static boolean isQuicSelectorThreadVirtual() {

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 141:

> 139: 
> 140:     @BeforeAll
> 141:     public static void beforeClass() throws Exception {

Suggestion:

    static void beforeClass() throws Exception {

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 155:

> 153: 
> 154:     @AfterAll
> 155:     public static void afterClass() throws Exception {

Suggestion:

    static void afterClass() throws Exception {

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 166:

> 164:      */
> 165:     @Test
> 166:     public void testBasicRequests() throws Exception {

Suggestion:

    void testBasicRequests() throws Exception {

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 199:

> 197:     }
> 198: 
> 199:     void assertSelectorThread(HttpClient client) {

Suggestion:

    private static void assertSelectorThread(HttpClient client) {

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 200:

> 198: 
> 199:     void assertSelectorThread(HttpClient client) {
> 200:         Objects.requireNonNull(client);

I doubt if this is needed.

Suggestion:

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 206:

> 204:                 .map(Thread::getName)
> 205:                 .toList());
> 206:         boolean found = threads.contains(name);

Using thread name for this check looks very fragile. Can we devise a more programmatic method? E.g., ensuring no `StackTraceElement` contains `QuicSelectorThread`?

test/jdk/java/net/httpclient/http3/H3QuicVTTest.java line 224:

> 222:             System.out.printf("%s: %s%n", status, msg);
> 223:         }
> 224:         Assertions.assertEquals(!isQuicSelectorThreadVirtual(), threads.contains(name), msg);

Suggestion:

        Assertions.assertEquals(!isQuicSelectorThreadVirtual(), found, msg);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487864987
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487758171
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487761972
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487818322
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487846396
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487823188
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487831510
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487834314
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487835902
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487846842
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487848843
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487847519
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487847819
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487847208
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487849223
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487850445
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487801799
PR Review Comment: https://git.openjdk.org/jdk/pull/27827#discussion_r2487764764