Backport d83a07b72cfd4dc42c5d4815262fcba05c653bd5

GoeLin · GoeLin · commit be44aba30227 · 2025-06-25T11:33:59.000+02:00
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java b/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java
@@ -355,6 +355,7 @@ static void abortPendingRequests(HttpClientImpl client, Throwable reason) {
     // nature of the API, we also need to wait until all pending operations
     // have completed.
     private final WeakReference<HttpClientFacade> facadeRef;
+    private final WeakReference<HttpClientImpl> implRef;
 
     private final ConcurrentSkipListSet<PlainHttpConnection> openedConnections
             = new ConcurrentSkipListSet<>(HttpConnection.COMPARE_BY_ID);
@@ -398,6 +399,7 @@ static void abortPendingRequests(HttpClientImpl client, Throwable reason) {
     private final AtomicLong pendingTCPConnectionCount = new AtomicLong();
     private final AtomicLong pendingSubscribersCount = new AtomicLong();
     private final AtomicBoolean isAlive = new AtomicBoolean();
+    private final AtomicBoolean isStarted = new AtomicBoolean();
 
     /** A Set of, deadline first, ordered timeout events. */
     private final TreeSet<TimeoutEvent> timeouts;
@@ -455,6 +457,7 @@ private HttpClientImpl(HttpClientBuilderImpl builder,
         delegatingExecutor = new DelegatingExecutor(this::isSelectorThread, ex,
                 this::onSubmitFailure);
         facadeRef = new WeakReference<>(facadeFactory.createFacade(this));
+        implRef = new WeakReference<>(this);
         client2 = new Http2ClientImpl(this);
         cookieHandler = builder.cookieHandler;
         connectTimeout = builder.connectTimeout;
@@ -504,7 +507,12 @@ void onSubmitFailure(Runnable command, Throwable failure) {
     }
 
     private void start() {
-        selmgr.start();
+        try {
+            selmgr.start();
+        } catch (Throwable t) {
+            isStarted.set(true);
+            throw t;
+        }
     }
 
     // Called from the SelectorManager thread, just before exiting.
@@ -699,7 +707,9 @@ final static class HttpClientTracker implements Tracker {
         final AtomicLong connnectionsCount;
         final AtomicLong subscribersCount;
         final Reference<?> reference;
+        final Reference<?> implRef;
         final AtomicBoolean isAlive;
+        final AtomicBoolean isStarted;
         final String name;
         HttpClientTracker(AtomicLong request,
                           AtomicLong http,
@@ -709,7 +719,9 @@ final static class HttpClientTracker implements Tracker {
                           AtomicLong conns,
                           AtomicLong subscribers,
                           Reference<?> ref,
+                          Reference<?> implRef,
                           AtomicBoolean isAlive,
+                          AtomicBoolean isStarted,
                           String name) {
             this.requestCount = request;
             this.httpCount = http;
@@ -719,7 +731,9 @@ final static class HttpClientTracker implements Tracker {
             this.connnectionsCount = conns;
             this.subscribersCount = subscribers;
             this.reference = ref;
+            this.implRef = implRef;
             this.isAlive = isAlive;
+            this.isStarted = isStarted;
             this.name = name;
         }
         @Override
@@ -750,8 +764,12 @@ public long getOutstandingWebSocketOperations() {
         public boolean isFacadeReferenced() {
             return !reference.refersTo(null);
         }
+        public boolean isImplementationReferenced() {
+            return !implRef.refersTo(null);
+        }
+        // The selector is considered alive if it's not yet started
         @Override
-        public boolean isSelectorAlive() { return isAlive.get(); }
+        public boolean isSelectorAlive() { return isAlive.get() || !isStarted.get(); }
         @Override
         public String getName() {
             return name;
@@ -768,7 +786,9 @@ public Tracker getOperationsTracker() {
                 pendingTCPConnectionCount,
                 pendingSubscribersCount,
                 facadeRef,
+                implRef,
                 isAlive,
+                isStarted,
                 dbgTag);
     }
 
@@ -1144,7 +1164,8 @@ public void run() {
             List<Pair<AsyncEvent,IOException>> errorList = new ArrayList<>();
             List<AsyncEvent> readyList = new ArrayList<>();
             List<Runnable> resetList = new ArrayList<>();
-            owner.isAlive.set(true);
+            owner.isAlive.set(true);   // goes back to false when run exits
+            owner.isStarted.set(true); // never goes back to false
             try {
                 if (Log.channel()) Log.logChannel(getName() + ": starting");
                 while (!Thread.currentThread().isInterrupted() && !closed) {
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/common/OperationTrackers.java b/src/java.net.http/share/classes/jdk/internal/net/http/common/OperationTrackers.java
@@ -64,6 +64,9 @@ public interface Tracker {
         // Whether the facade returned to the
         // user is still referenced
         boolean isFacadeReferenced();
+        // Whether the implementation of the facade
+        // is still referenced
+        boolean isImplementationReferenced();
         // whether the Selector Manager thread is still running
         boolean isSelectorAlive();
         // The name of the object being tracked.
diff --git a/test/jdk/java/net/httpclient/ReferenceTracker.java b/test/jdk/java/net/httpclient/ReferenceTracker.java
@@ -30,6 +30,7 @@
 import java.lang.management.MonitorInfo;
 import java.lang.management.ThreadInfo;
 import java.net.http.HttpClient;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.Objects;
 import java.util.concurrent.ConcurrentLinkedQueue;
@@ -103,7 +104,7 @@ public AssertionError check(Tracker tracker, long graceDelayMs) {
                 hasOperations.or(hasSubscribers)
                         .or(Tracker::isFacadeReferenced)
                         .or(Tracker::isSelectorAlive),
-                "outstanding operations or unreleased resources", false);
+                "outstanding operations or unreleased resources", true);
     }
 
     public AssertionError check(long graceDelayMs) {
@@ -210,32 +211,46 @@ public AssertionError check(Tracker tracker,
         graceDelayMs = Math.max(graceDelayMs, 100);
         long delay = Math.min(graceDelayMs, 10);
         var count = delay > 0 ? graceDelayMs / delay : 1;
+        long waitStart = System.nanoTime();
+        long waited = 0;
+        long toWait = Math.min(graceDelayMs, Math.max(delay, 1));
         for (int i = 0; i < count; i++) {
             if (hasOutstanding.test(tracker)) {
                 System.gc();
                 try {
                     if (i == 0) {
                         System.out.println("Waiting for HTTP operations to terminate...");
                     }
-                    Thread.sleep(Math.min(graceDelayMs, Math.max(delay, 1)));
+                    waited += toWait;
+                    Thread.sleep(toWait);
                 } catch (InterruptedException x) {
                     // OK
                 }
-            } else break;
+            } else {
+                System.out.println("No outstanding HTTP operations remaining after "
+                        + i + "/" + count + " iterations and " + waited + "/" + graceDelayMs
+                        + " ms, (wait/iteration " + toWait + " ms)");
+                break;
+            }
         }
+        long duration = Duration.ofNanos(System.nanoTime() - waitStart).toMillis();
         if (hasOutstanding.test(tracker)) {
             StringBuilder warnings = diagnose(tracker, new StringBuilder(), hasOutstanding);
             if (hasOutstanding.test(tracker)) {
                 fail = new AssertionError(warnings.toString());
             }
         } else {
-            System.out.println("PASSED: No " + description + " found in " + tracker.getName());
+            System.out.println("PASSED: No " + description + " found in "
+                    + tracker.getName() + " in " + duration + " ms");
         }
         if (fail != null) {
             if (printThreads && tracker.isSelectorAlive()) {
-                printThreads("Some selector manager threads are still alive: ", System.out);
-                printThreads("Some selector manager threads are still alive: ", System.err);
+                var msg = "Selector manager threads are still alive for " + tracker.getName() + ": ";
+                printThreads(msg, System.out);
+                printThreads(msg, System.err);
             }
+            System.out.println("AssertionError: Found some " + description + " in "
+                    + tracker.getName() + " after " + duration + " ms, waited " + waited + " ms");
         }
         return fail;
     }
diff --git a/test/jdk/java/net/httpclient/SpecialHeadersTest.java b/test/jdk/java/net/httpclient/SpecialHeadersTest.java
@@ -25,7 +25,7 @@
  * @test
  * @summary  Verify that some special headers - such as User-Agent
  *           can be specified by the caller.
- * @bug 8203771 8218546
+ * @bug 8203771 8218546 8297200
  * @library /test/lib /test/jdk/java/net/httpclient/lib
  * @build jdk.httpclient.test.lib.common.HttpServerAdapters
  *        jdk.httpclient.test.lib.http2.Http2TestServer
@@ -42,6 +42,7 @@
 import com.sun.net.httpserver.HttpServer;
 import com.sun.net.httpserver.HttpsConfigurator;
 import com.sun.net.httpserver.HttpsServer;
+import jdk.internal.net.http.common.OperationTrackers.Tracker;
 import jdk.test.lib.net.SimpleSSLContext;
 import org.testng.ITestContext;
 import org.testng.ITestResult;
@@ -300,13 +301,18 @@ static String userAgent() {
     static final Map<String, Function<URI,String>> DEFAULTS = Map.of(
         "USER-AGENT", u -> userAgent(), "HOST", u -> u.getRawAuthority());
 
+    static void throwIfNotNull(Throwable throwable) throws Exception {
+        if (throwable instanceof Exception ex) throw ex;
+        if (throwable instanceof Error e) throw e;
+    }
+
     @Test(dataProvider = "variants")
     void test(String uriString,
               String headerNameAndValue,
               boolean sameClient)
         throws Exception
     {
-        out.println("\n--- Starting ");
+        out.println("\n--- Starting test " + now());
 
         int index = headerNameAndValue.indexOf(":");
         String name = headerNameAndValue.substring(0, index);
@@ -318,10 +324,14 @@ void test(String uriString,
         String value =  useDefault ? DEFAULTS.get(key).apply(uri) : v;
 
         HttpClient client = null;
+        Tracker tracker = null;
+        Throwable thrown = null;
         for (int i=0; i< ITERATION_COUNT; i++) {
             try {
-                if (!sameClient || client == null)
+                if (!sameClient || client == null) {
                     client = newHttpClient("test", sameClient);
+                    tracker = TRACKER.getTracker(client);
+                }
 
                 HttpRequest.Builder requestBuilder = HttpRequest.newBuilder(uri);
                 if (!useDefault) {
@@ -361,14 +371,20 @@ void test(String uriString,
                         assertEquals(resp.headers().allValues("X-" + key).size(), 0);
                     }
                 }
+            } catch (Throwable x) {
+                thrown = x;
             } finally {
                 if (!sameClient) {
                     client = null;
                     System.gc();
-                    var error = TRACKER.check(500);
-                    if (error != null) throw error;
+                    var error = TRACKER.check(tracker, 500);
+                    if (error != null) {
+                        if (thrown != null) error.addSuppressed(thrown);
+                        throw error;
+                    }
                 }
             }
+            throwIfNotNull(thrown);
         }
     }
 
@@ -378,11 +394,12 @@ void testHomeMadeIllegalHeader(String uriString,
                                    boolean sameClient)
         throws Exception
     {
-        out.println("\n--- Starting ");
+        out.println("\n--- Starting testHomeMadeIllegalHeader " + now());
         final URI uri = URI.create(uriString);
 
         HttpClient client = newHttpClient("testHomeMadeIllegalHeader", sameClient);
-
+        Tracker tracker = TRACKER.getTracker(client);
+        Throwable thrown = null;
         try {
             // Test a request which contains an illegal header created
             HttpRequest req = new HttpRequest() {
@@ -429,19 +446,29 @@ public HttpHeaders headers() {
             } catch (IllegalArgumentException ee) {
                 out.println("Got IAE as expected");
             }
+        } catch (Throwable x) {
+            thrown = x;
         } finally {
             if (!sameClient) {
                 client = null;
                 System.gc();
-                var error = TRACKER.check(500);
-                if (error != null) throw error;
+                var error = TRACKER.check(tracker, 500);
+                if (error != null) {
+                    if (thrown != null) error.addSuppressed(thrown);
+                    throw error;
+                }
             }
         }
+        throwIfNotNull(thrown);
     }
 
+
+
     @Test(dataProvider = "variants")
-    void testAsync(String uriString, String headerNameAndValue, boolean sameClient) {
-        out.println("\n--- Starting ");
+    void testAsync(String uriString, String headerNameAndValue, boolean sameClient)
+            throws Exception
+    {
+        out.println("\n--- Starting testAsync " + now());
         int index = headerNameAndValue.indexOf(":");
         String name = headerNameAndValue.substring(0, index);
         String v = headerNameAndValue.substring(index+1).trim();
@@ -452,10 +479,14 @@ void testAsync(String uriString, String headerNameAndValue, boolean sameClient)
         String value =  useDefault ? DEFAULTS.get(key).apply(uri) : v;
 
         HttpClient client = null;
+        Tracker tracker = null;
+        Throwable thrown = null;
         for (int i=0; i< ITERATION_COUNT; i++) {
             try {
-                if (!sameClient || client == null)
+                if (!sameClient || client == null) {
                     client = newHttpClient("testAsync", sameClient);
+                    tracker = TRACKER.getTracker(client);
+                }
 
                 HttpRequest.Builder requestBuilder = HttpRequest.newBuilder(uri);
                 if (!useDefault) {
@@ -498,14 +529,20 @@ void testAsync(String uriString, String headerNameAndValue, boolean sameClient)
                             }
                         })
                         .join();
+            } catch (Throwable x) {
+                thrown = x;
             } finally {
                 if (!sameClient) {
                     client = null;
                     System.gc();
-                    var error = TRACKER.check(500);
-                    if (error != null) throw error;
+                    var error = TRACKER.check(tracker, 500);
+                    if (error != null) {
+                        if (thrown != null) error.addSuppressed(thrown);
+                        throw error;
+                    }
                 }
             }
+            throwIfNotNull(thrown);
         }
     }
 
@@ -516,6 +553,7 @@ static String serverAuthority(HttpTestServer server) {
 
     @BeforeTest
     public void setup() throws Exception {
+        out.println("--- Starting setup " + now());
         sslContext = new SimpleSSLContext().get();
         if (sslContext == null)
             throw new AssertionError("Unexpected null sslContext");
@@ -545,13 +583,15 @@ public void setup() throws Exception {
 
     @AfterTest
     public void teardown() throws Exception {
+        out.println("\n--- Teardown " + now());
         HttpClient shared = sharedClient;
         String sharedClientName =
                 shared == null ? null : shared.toString();
         if (shared != null) TRACKER.track(shared);
         shared = sharedClient = null;
         Thread.sleep(100);
-        AssertionError fail = TRACKER.check(500);
+        AssertionError fail = TRACKER.check(2500);
+        out.println("--- Stopping servers " + now());
         try {
             httpTestServer.stop();
             httpsTestServer.stop();
