8311792: java/net/httpclient/ResponsePublisher.java fails intermittently with AssertionError: Found some outstanding operations

Reviewed-by: dfuchs, jpai

Author: Darragh Clarke

test/jdk/java/net/httpclient/ResponsePublisher.java

@@ -36,6 +36,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;
 import jdk.test.lib.net.SimpleSSLContext;
 import org.testng.annotations.AfterTest;
 import org.testng.annotations.BeforeTest;
@@ -100,6 +101,16 @@ public class ResponsePublisher implements HttpServerAdapters {
     // a shared executor helps reduce the amount of threads created by the test
     static final Executor executor = Executors.newCachedThreadPool();
 
+    static final long start = System.nanoTime();
+
+    public static String now() {
+        long now = System.nanoTime() - start;
+        long secs = now / 1000_000_000;
+        long mill = (now % 1000_000_000) / 1000_000;
+        long nan = now % 1000_000;
+        return String.format("[%d s, %d ms, %d ns] ", secs, mill, nan);
+    }
+
     interface BHS extends Supplier<BodyHandler<Publisher<List<ByteBuffer>>>> {
         static BHS of(BHS impl, String name) {
             return new BHSImpl(impl, name);
@@ -216,6 +227,12 @@ public void testExceptions(String uri, boolean sameClient, BHS handlers) throws
             // Get the final result and compare it with the expected body
             String body = ofString.getBody().toCompletableFuture().get();
             assertEquals(body, "");
+            // ensure client closes before next iteration
+            if (!sameClient) {
+                var tracker = TRACKER.getTracker(client);
+                client = null;
+                clientCleanup(tracker);
+            }
         }
     }
 
@@ -239,6 +256,12 @@ public void testNoBody(String uri, boolean sameClient, BHS handlers) throws Exce
             // Get the final result and compare it with the expected body
             String body = ofString.getBody().toCompletableFuture().get();
             assertEquals(body, "");
+            // ensure client closes before next iteration
+            if (!sameClient) {
+                var tracker = TRACKER.getTracker(client);
+                client = null;
+                clientCleanup(tracker);
+            }
         }
     }
 
@@ -265,6 +288,12 @@ public void testNoBodyAsync(String uri, boolean sameClient, BHS handlers) throws
                             });
             // Get the final result and compare it with the expected body
             assertEquals(result.get(), "");
+            // ensure client closes before next iteration
+            if (!sameClient) {
+                var tracker = TRACKER.getTracker(client);
+                client = null;
+                clientCleanup(tracker);
+            }
         }
     }
 
@@ -288,6 +317,12 @@ public void testAsString(String uri, boolean sameClient, BHS handlers) throws Ex
             // Get the final result and compare it with the expected body
             String body = ofString.getBody().toCompletableFuture().get();
             assertEquals(body, WITH_BODY);
+            // ensure client closes before next iteration
+            if (!sameClient) {
+                var tracker = TRACKER.getTracker(client);
+                client = null;
+                clientCleanup(tracker);
+            }
         }
     }
 
@@ -314,6 +349,12 @@ public void testAsStringAsync(String uri, boolean sameClient, BHS handlers) thro
             // Get the final result and compare it with the expected body
             String body = result.get();
             assertEquals(body, WITH_BODY);
+            // ensure client closes before next iteration
+            if (!sameClient) {
+                var tracker = TRACKER.getTracker(client);
+                client = null;
+                clientCleanup(tracker);
+            }
         }
     }
 
@@ -451,6 +492,23 @@ public void teardown() throws Exception {
         }
     }
 
+    // Wait for the client to be garbage collected.
+    // we use the ReferenceTracker API rather than HttpClient::close here,
+    // because we want to get some diagnosis if a client doesn't release
+    // its resources and terminates as expected
+    // By using the ReferenceTracker, we will get some diagnosis about what
+    // is keeping the client alive if it doesn't get GC'ed within the
+    // expected time frame.
+    public void clientCleanup(OperationTrackers.Tracker tracker){
+        System.gc();
+        System.out.println(now() + "waiting for client to shutdown: " + tracker.getName());
+        System.err.println(now() + "waiting for client to shutdown: " + tracker.getName());
+        var error = TRACKER.check(tracker, 10000);
+        if (error != null) throw error;
+        System.out.println(now() + "client shutdown normally: " + tracker.getName());
+        System.err.println(now() + "client shutdown normally: " + tracker.getName());
+    }
+
     static final String WITH_BODY = "Lorem ipsum dolor sit amet, consectetur" +
             " adipiscing elit, sed do eiusmod tempor incididunt ut labore et" +
             " dolore magna aliqua. Ut enim ad minim veniam, quis nostrud" +