8155590: Dubious collection management in sun.net.www.http.KeepAliveCache
authorclanger
Thu, 19 Oct 2017 09:01:29 +0200
changeset 47361 74e1913a98c0
parent 47360 31c9cf5eca62
child 47362 e729cef2af4b
8155590: Dubious collection management in sun.net.www.http.KeepAliveCache Reviewed-by: rriggs, vtewari
src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java
--- a/src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java	Thu Oct 19 14:49:20 2017 +0800
+++ b/src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java	Thu Oct 19 09:01:29 2017 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2017, 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
@@ -27,10 +27,18 @@
 
 import java.io.IOException;
 import java.io.NotSerializableException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.net.URL;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.net.URL;
+import java.util.List;
+
 import jdk.internal.misc.InnocuousThread;
+import sun.security.action.GetIntegerAction;
 
 /**
  * A class that implements a cache of idle Http connections for keep-alive
@@ -53,14 +61,14 @@
     static int result = -1;
     static int getMaxConnections() {
         if (result == -1) {
-            result = java.security.AccessController.doPrivileged(
-                new sun.security.action.GetIntegerAction("http.maxConnections",
-                                                         MAX_CONNECTIONS))
+            result = AccessController.doPrivileged(
+                new GetIntegerAction("http.maxConnections", MAX_CONNECTIONS))
                 .intValue();
-            if (result <= 0)
+            if (result <= 0) {
                 result = MAX_CONNECTIONS;
+            }
         }
-            return result;
+        return result;
     }
 
     static final int LIFETIME = 5000;
@@ -93,8 +101,7 @@
              * The robustness to get around this is in HttpClient.parseHTTP()
              */
             final KeepAliveCache cache = this;
-            java.security.AccessController.doPrivileged(
-                new java.security.PrivilegedAction<>() {
+            AccessController.doPrivileged(new PrivilegedAction<>() {
                 public Void run() {
                     keepAliveTimer = InnocuousThread.newSystemThread("Keep-Alive-Timer", cache);
                     keepAliveTimer.setDaemon(true);
@@ -110,8 +117,8 @@
 
         if (v == null) {
             int keepAliveTimeout = http.getKeepAliveTimeout();
-            v = new ClientVector(keepAliveTimeout > 0?
-                                 keepAliveTimeout*1000 : LIFETIME);
+            v = new ClientVector(keepAliveTimeout > 0 ?
+                                 keepAliveTimeout * 1000 : LIFETIME);
             v.put(http);
             super.put(key, v);
         } else {
@@ -120,12 +127,12 @@
     }
 
     /* remove an obsolete HttpClient from its VectorCache */
-    public synchronized void remove (HttpClient h, Object obj) {
+    public synchronized void remove(HttpClient h, Object obj) {
         KeepAliveKey key = new KeepAliveKey(h.url, obj);
         ClientVector v = super.get(key);
         if (v != null) {
             v.remove(h);
-            if (v.empty()) {
+            if (v.isEmpty()) {
                 removeVector(key);
             }
         }
@@ -142,7 +149,6 @@
      * Check to see if this URL has a cached HttpClient
      */
     public synchronized HttpClient get(URL url, Object obj) {
-
         KeepAliveKey key = new KeepAliveKey(url, obj);
         ClientVector v = super.get(key);
         if (v == null) { // nothing in cache yet
@@ -161,39 +167,27 @@
             try {
                 Thread.sleep(LIFETIME);
             } catch (InterruptedException e) {}
+
+            // Remove all outdated HttpClients.
             synchronized (this) {
-                /* Remove all unused HttpClients.  Starting from the
-                 * bottom of the stack (the least-recently used first).
-                 * REMIND: It'd be nice to not remove *all* connections
-                 * that aren't presently in use.  One could have been added
-                 * a second ago that's still perfectly valid, and we're
-                 * needlessly axing it.  But it's not clear how to do this
-                 * cleanly, and doing it right may be more trouble than it's
-                 * worth.
-                 */
-
                 long currentTime = System.currentTimeMillis();
-
-                ArrayList<KeepAliveKey> keysToRemove
-                    = new ArrayList<>();
+                List<KeepAliveKey> keysToRemove = new ArrayList<>();
 
                 for (KeepAliveKey key : keySet()) {
                     ClientVector v = get(key);
                     synchronized (v) {
-                        int i;
-
-                        for (i = 0; i < v.size(); i++) {
-                            KeepAliveEntry e = v.elementAt(i);
+                        KeepAliveEntry e = v.peek();
+                        while (e != null) {
                             if ((currentTime - e.idleStartTime) > v.nap) {
-                                HttpClient h = e.hc;
-                                h.closeServer();
+                                v.poll();
+                                e.hc.closeServer();
                             } else {
                                 break;
                             }
+                            e = v.peek();
                         }
-                        v.subList(0, i).clear();
 
-                        if (v.size() == 0) {
+                        if (v.isEmpty()) {
                             keysToRemove.add(key);
                         }
                     }
@@ -203,21 +197,19 @@
                     removeVector(key);
                 }
             }
-        } while (size() > 0);
-
-        return;
+        } while (!isEmpty());
     }
 
     /*
      * Do not serialize this class!
      */
-    private void writeObject(java.io.ObjectOutputStream stream)
-    throws IOException {
+    private void writeObject(ObjectOutputStream stream) throws IOException {
         throw new NotSerializableException();
     }
 
-    private void readObject(java.io.ObjectInputStream stream)
-    throws IOException, ClassNotFoundException {
+    private void readObject(ObjectInputStream stream)
+        throws IOException, ClassNotFoundException
+    {
         throw new NotSerializableException();
     }
 }
@@ -225,37 +217,33 @@
 /* FILO order for recycling HttpClients, should run in a thread
  * to time them out.  If > maxConns are in use, block.
  */
-
-
-class ClientVector extends java.util.Stack<KeepAliveEntry> {
+class ClientVector extends ArrayDeque<KeepAliveEntry> {
     private static final long serialVersionUID = -8680532108106489459L;
 
     // sleep time in milliseconds, before cache clear
     int nap;
 
-
-
-    ClientVector (int nap) {
+    ClientVector(int nap) {
         this.nap = nap;
     }
 
     synchronized HttpClient get() {
-        if (empty()) {
+        if (isEmpty()) {
             return null;
-        } else {
-            // Loop until we find a connection that has not timed out
-            HttpClient hc = null;
-            long currentTime = System.currentTimeMillis();
-            do {
-                KeepAliveEntry e = pop();
-                if ((currentTime - e.idleStartTime) > nap) {
-                    e.hc.closeServer();
-                } else {
-                    hc = e.hc;
-                }
-            } while ((hc== null) && (!empty()));
-            return hc;
         }
+
+        // Loop until we find a connection that has not timed out
+        HttpClient hc = null;
+        long currentTime = System.currentTimeMillis();
+        do {
+            KeepAliveEntry e = pop();
+            if ((currentTime - e.idleStartTime) > nap) {
+                e.hc.closeServer();
+            } else {
+                hc = e.hc;
+            }
+        } while ((hc == null) && (!isEmpty()));
+        return hc;
     }
 
     /* return a still valid, unused HttpClient */
@@ -267,21 +255,30 @@
         }
     }
 
+    /* remove an HttpClient */
+    synchronized boolean remove(HttpClient h) {
+        for (KeepAliveEntry curr : this) {
+            if (curr.hc == h) {
+                return super.remove(curr);
+            }
+        }
+        return false;
+    }
+
     /*
      * Do not serialize this class!
      */
-    private void writeObject(java.io.ObjectOutputStream stream)
-    throws IOException {
+    private void writeObject(ObjectOutputStream stream) throws IOException {
         throw new NotSerializableException();
     }
 
-    private void readObject(java.io.ObjectInputStream stream)
-    throws IOException, ClassNotFoundException {
+    private void readObject(ObjectInputStream stream)
+        throws IOException, ClassNotFoundException
+    {
         throw new NotSerializableException();
     }
 }
 
-
 class KeepAliveKey {
     private String      protocol = null;
     private String      host = null;