8302732: sun/net/www/http/HttpClient/MultiThreadTest.java still failing intermittently

Reviewed-by: dfuchs

src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2023, 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
@@ -36,7 +36,6 @@ import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
-import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
 import jdk.internal.misc.InnocuousThread;
@@ -51,7 +50,7 @@ import sun.util.logging.PlatformLogger;
  * @author Dave Brown
  */
 public class KeepAliveCache
-    extends HashMap<KeepAliveKey, ClientVector>
+    extends HashMap<KeepAliveKey, KeepAliveCache.ClientVector>
     implements Runnable {
     @java.io.Serial
     private static final long serialVersionUID = -2937172892064557949L;
@@ -243,13 +242,16 @@ public class KeepAliveCache
             // Remove all outdated HttpClients.
             cacheLock.lock();
             try {
+                if (isEmpty()) {
+                    // cache not used in the last LIFETIME - exit
+                    keepAliveTimer = null;
+                    break;
+                }
                 long currentTime = System.currentTimeMillis();
                 List<KeepAliveKey> keysToRemove = new ArrayList<>();
 
                 for (KeepAliveKey key : keySet()) {
                     ClientVector v = get(key);
-                    v.lock();
-                    try {
                     KeepAliveEntry e = v.peekLast();
                     while (e != null) {
                         if ((currentTime - e.idleStartTime) > v.nap) {
@@ -267,9 +269,6 @@ public class KeepAliveCache
                     if (v.isEmpty()) {
                         keysToRemove.add(key);
                     }
-                    } finally {
-                        v.unlock();
-                    }
                 }
 
                 for (KeepAliveKey key : keysToRemove) {
@@ -284,7 +283,7 @@ public class KeepAliveCache
                     }
                 }
             }
-        } while (!isEmpty());
+        } while (keepAliveTimer == Thread.currentThread());
     }
 
     /*
@@ -301,15 +300,13 @@ public class KeepAliveCache
     {
         throw new NotSerializableException();
     }
-}
 
-/* LIFO order for reusing HttpClients. Most recent entries at the front.
+    /* LIFO order for reusing HttpClients. Most recent entries at the front.
      * If > maxConns are in use, discard oldest.
      */
-class ClientVector extends ArrayDeque<KeepAliveEntry> {
+    class ClientVector extends ArrayDeque<KeepAliveEntry> {
         @java.io.Serial
         private static final long serialVersionUID = -8680532108106489459L;
-    private final ReentrantLock lock = new ReentrantLock();
 
         // sleep time in milliseconds, before cache clear
         int nap;
@@ -320,8 +317,7 @@ class ClientVector extends ArrayDeque<KeepAliveEntry> {
 
         /* return a still valid, idle HttpClient */
         HttpClient get() {
-        lock();
+            assert cacheLock.isHeldByCurrentThread();
-        try {
             // check the most recent connection, use if still valid
             KeepAliveEntry e = peekFirst();
             if (e == null) {
@@ -339,36 +335,21 @@ class ClientVector extends ArrayDeque<KeepAliveEntry> {
                 }
                 return e.hc;
             }
-        } finally {
-            unlock();
-        }
         }
 
         HttpClient put(HttpClient h) {
+            assert cacheLock.isHeldByCurrentThread();
             HttpClient staleClient = null;
-        lock();
-        try {
             assert KeepAliveCache.getMaxConnections() > 0;
             if (size() >= KeepAliveCache.getMaxConnections()) {
                 // remove oldest connection
                 staleClient = removeLast().hc;
             }
             addFirst(new KeepAliveEntry(h, System.currentTimeMillis()));
-        } finally {
-            unlock();
-        }
             // close after releasing the locks
             return staleClient;
         }
 
-    final void lock() {
-        lock.lock();
-    }
-
-    final void unlock() {
-        lock.unlock();
-    }
-
         /*
          * Do not serialize this class!
          */
@@ -379,10 +360,10 @@ class ClientVector extends ArrayDeque<KeepAliveEntry> {
 
         @java.io.Serial
         private void readObject(ObjectInputStream stream)
-        throws IOException, ClassNotFoundException
+                throws IOException, ClassNotFoundException {
-    {
             throw new NotSerializableException();
         }
+    }
 }
 
 class KeepAliveKey {

test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2023, 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
@@ -1230,7 +1230,7 @@ public class KeepAliveTest {
             System.out.println("ProxyHostUsingSystemProperty:" + System.getProperty("http.proxyHost"));
             System.out.println("http.keepAlive.time.server=" + System.getProperty("http.keepAlive.time.server"));
             System.out.println("http.keepAlive.time.proxy=" + System.getProperty("http.keepAlive.time.proxy"));
-            Class clientVectorClass = Class.forName("sun.net.www.http.ClientVector");
+            Class clientVectorClass = Class.forName("sun.net.www.http.KeepAliveCache$ClientVector");
             //      System.out.println("clientVectorClass=" + clientVectorClass);
             Field napField = clientVectorClass.getDeclaredField("nap");
             napField.setAccessible(true);