http3: refactor HTTP/3 connection pool management in a separate class

dfuch · dfuch · commit 93a184eec4d8 · 2025-06-03T17:41:45.000+01:00
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http3ClientImpl.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http3ClientImpl.java
@@ -102,8 +102,7 @@ public final class Http3ClientImpl implements AutoCloseable {
     private final Logger debug = Utils.getDebugLogger(this::dbgString);
 
     final HttpClientImpl client;
-    /* Map key is "scheme:host:port" */
-    private final Map<String,Http3Connection> connections = new ConcurrentHashMap<>();
+    private final Http3ConnectionPool connections = new Http3ConnectionPool(debug);
     private final Map<String,ConnectionRecovery> reconnections = new ConcurrentHashMap<>();
     private final Set<Http3Connection> pendingClose = ConcurrentHashMap.newKeySet();
     private final Set<String> noH3 = ConcurrentHashMap.newKeySet();
@@ -244,7 +243,7 @@ Http3Connection findPooledConnectionFor(HttpRequestImpl request,
                 throws IOException {
         if (request.secure() && request.proxy() == null) {
             var config = request.http3Discovery();
-            final var pooled = connections.get(connectionKey(request));
+            final var pooled = connections.lookupFor(request);
             if (pooled == null) {
                 return null;
             }
@@ -266,41 +265,15 @@ Http3Connection findPooledConnectionFor(HttpRequestImpl request,
                         return null;
                     }
                 }
-                boolean suitable = switch (config) {
-                    case HTTP_3_URI_ONLY -> {
-                        if (altService == null) {
-                            // the pooled connection was created as a result of a direct connection
-                            // against the origin, so is a valid one to use for HTTP_3_URI_ONLY request
-                            yield true;
-                        }
-                        // At this point, we have found a pooled connection which matches the request's
-                        // authority and that pooled connection was created because some origin
-                        // advertised the authority as an alternate service. We can use this pooled
-                        // connection with HTTP_3_URI_ONLY, only if the authority of the
-                        // alternate service is the same as the authority of the origin that
-                        // advertised it
-                        yield altService.originHasSameAuthority();
-                    }
-                    // can't use the connection with ALT_SVC unless the endpoint
-                    // was advertised through altService
-                    case ALT_SVC -> altService != null && altService.wasAdvertised();
-                    default -> true;
-                };
-                if (suitable) {
-                    // found a valid connection in pool, return it
-                    if (debug.on()) {
-                        debug.log("Found Http3Connection in connection pool");
-                    }
-                    return pooled;
-                }
                 if (debug.on()) {
-                    if (altService != null) {
-                        debug.log("pooled connection for alt-service %s cannot be used for %s",
-                                altService, config);
-                    }
+                    debug.log("Found Http3Connection in connection pool");
                 }
-                return null;
+                // found a valid connection in pool, return it
+                return pooled;
             } else {
+                if (debug.on()) {
+                    debug.log("Pooled connection expired. Removing it.");
+                }
                 removeFromPool(pooled);
             }
         }
@@ -706,7 +679,7 @@ public void abort(Throwable t) {
             lock.lock();
             try {
                 closed = true;
-                connectionList = new ArrayList<>(connections.values());
+                connectionList = new ArrayList<>(connections.values().toList());
                 connectionList.addAll(pendingClose);
                 pendingClose.clear();
                 connections.clear();
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http3ConnectionPool.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http3ConnectionPool.java
@@ -0,0 +1,184 @@
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jdk.internal.net.http;
+
+import java.net.http.HttpOption.Http3DiscoveryMode;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+
+import jdk.internal.net.http.common.Logger;
+
+import static java.net.http.HttpOption.Http3DiscoveryMode.ALT_SVC;
+import static java.net.http.HttpOption.Http3DiscoveryMode.ANY;
+import static java.net.http.HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY;
+
+/**
+ * This class encapsulate the HTTP/3 connection pool managed
+ * by an instance of {@link Http3ClientImpl}.
+ */
+class Http3ConnectionPool {
+    /* Map key is "scheme:host:port" */
+    private final Map<String,Http3Connection> advertised = new ConcurrentHashMap<>();
+    /* Map key is "scheme:host:port" */
+    private final Map<String,Http3Connection> unadvertised = new ConcurrentHashMap<>();
+
+    private final Logger debug;
+    Http3ConnectionPool(Logger logger) {
+        this.debug = Objects.requireNonNull(logger);
+    }
+
+    // https:<host>:<port>
+    String connectionKey(HttpRequestImpl request) {
+        var uri = request.uri();
+        var scheme = uri.getScheme().toLowerCase(Locale.ROOT);
+        var host = uri.getHost();
+        var port = uri.getPort();
+        assert scheme.equals("https");
+        if (port < 0) port = 443; // https
+        return String.format("%s:%s:%d", scheme, host, port);
+    }
+
+    private Http3Connection lookupUnadvertised(String key, Http3DiscoveryMode discoveryMode) {
+        var unadvertisedConn = unadvertised.get(key);
+        if (unadvertisedConn == null) return null;
+        if (discoveryMode == ANY) return unadvertisedConn;
+        if (discoveryMode == ALT_SVC) return null;
+
+        assert discoveryMode == HTTP_3_URI_ONLY : String.valueOf(discoveryMode);
+
+        // Double check that if there is an alt service, it has same origin.
+        final var altService = Optional.ofNullable(unadvertisedConn)
+                .map(Http3Connection::connection)
+                .flatMap(HttpQuicConnection::getSourceAltService)
+                .orElse(null);
+
+        if (altService == null || altService.originHasSameAuthority()) {
+            return unadvertisedConn;
+        }
+
+        // We should never come here.
+        assert false : "unadvertised connection with different origin: %s -> %s"
+                .formatted(key, altService);
+        return null;
+    }
+
+    Http3Connection lookupFor(HttpRequestImpl request) {
+        var discoveryMode = request.http3Discovery();
+        var key = connectionKey(request);
+
+        // If not ALT_SVC, we can use unadvertised connections
+        if (discoveryMode != ALT_SVC) {
+            var unadvertisedConn = lookupUnadvertised(key, discoveryMode);
+            if (unadvertisedConn != null) return unadvertisedConn;
+        }
+
+        // Then see if we have a connection which was advertised.
+        var advertisedConn = advertised.get(key);
+        // We can use it for HTTP3_URI_ONLY too if it has same origin
+        if (advertisedConn != null) {
+            final var altService = advertisedConn.connection()
+                    .getSourceAltService().orElse(null);
+            assert altService != null && altService.wasAdvertised();
+            switch (discoveryMode) {
+                case ANY -> {
+                    return advertisedConn;
+                }
+                case ALT_SVC -> {
+                    return advertisedConn;
+                }
+                case HTTP_3_URI_ONLY -> {
+                    if (altService != null && altService.originHasSameAuthority()) {
+                        return advertisedConn;
+                    }
+                }
+            }
+        }
+
+        if (debug.on()) {
+            debug.log("No suitable connection found in the pool for discovery mode %s",
+                    discoveryMode);
+        }
+        return null;
+    }
+
+    Http3Connection putIfAbsent(String key, Http3Connection c) {
+        Objects.requireNonNull(key);
+        Objects.requireNonNull(c);
+        assert key.equals(c.key());
+        var altService = c.connection().getSourceAltService().orElse(null);
+        if (altService != null && altService.wasAdvertised()) {
+            return advertised.putIfAbsent(key, c);
+        }
+        assert altService == null || altService.originHasSameAuthority();
+        return unadvertised.putIfAbsent(key, c);
+    }
+
+    Http3Connection put(String key, Http3Connection c) {
+        Objects.requireNonNull(key);
+        Objects.requireNonNull(c);
+        assert key.equals(c.key()) : "key mismatch %s -> %s"
+                .formatted(key, c.key());
+        var altService = c.connection().getSourceAltService().orElse(null);
+        if (altService != null && altService.wasAdvertised()) {
+            return advertised.put(key, c);
+        }
+        assert altService == null || altService.originHasSameAuthority();
+        return unadvertised.put(key, c);
+    }
+
+    boolean remove(String key, Http3Connection c) {
+        Objects.requireNonNull(key);
+        Objects.requireNonNull(c);
+        assert key.equals(c.key()) : "key mismatch %s -> %s"
+                .formatted(key, c.key());
+
+        var altService = c.connection().getSourceAltService().orElse(null);
+        if (altService != null && altService.wasAdvertised()) {
+            boolean remUndavertised = unadvertised.remove(key, c);
+            assert !remUndavertised
+                    : "advertised connection found in unadvertised pool for " + key;
+            return advertised.remove(key, c);
+        }
+
+        assert altService == null || altService.originHasSameAuthority();
+        return unadvertised.remove(key, c);
+    }
+
+    void clear() {
+        advertised.clear();
+        unadvertised.clear();
+    }
+
+    java.util.stream.Stream<Http3Connection> values() {
+        return java.util.stream.Stream.concat(
+                advertised.values().stream(),
+                unadvertised.values().stream());
+    }
+
+}
+
diff --git a/test/jdk/java/net/httpclient/http3/H3ConnectionPoolTest.java b/test/jdk/java/net/httpclient/http3/H3ConnectionPoolTest.java
