From cfed45d15a36cd92aec873f38c46cd085ee0b9b1 Mon Sep 17 00:00:00 2001 From: Dhamoder Nalla Date: Thu, 8 Feb 2024 10:22:30 +0000 Subject: [PATCH] 8155590: Dubious collection management in sun.net.www.http.KeepAliveCache Reviewed-by: sgehwolf Backport-of: 2e31cc7ee1b875af7c7b3a5367ac8056fbc60650 --- .../sun/net/www/http/KeepAliveCache.java | 130 +++++++++--------- 1 file changed, 64 insertions(+), 66 deletions(-) diff --git a/jdk/src/share/classes/sun/net/www/http/KeepAliveCache.java b/jdk/src/share/classes/sun/net/www/http/KeepAliveCache.java index 16f02d7cdde..d6548e0c531 100644 --- a/jdk/src/share/classes/sun/net/www/http/KeepAliveCache.java +++ b/jdk/src/share/classes/sun/net/www/http/KeepAliveCache.java @@ -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,9 +27,17 @@ 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 sun.security.action.GetIntegerAction; /** * A class that implements a cache of idle Http connections for keep-alive @@ -52,14 +60,14 @@ public class KeepAliveCache 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; @@ -92,8 +100,7 @@ public synchronized void put(final URL url, Object obj, HttpClient http) { * 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() { // We want to create the Keep-Alive-Timer in the // system threadgroup @@ -120,8 +127,8 @@ public Void run() { 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 { @@ -130,12 +137,12 @@ public Void run() { } /* 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); } } @@ -171,39 +178,27 @@ public void run() { try { Thread.sleep(LIFETIME); } catch (InterruptedException e) {} - 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. - */ + // Remove all outdated HttpClients. + synchronized (this) { long currentTime = System.currentTimeMillis(); - - ArrayList keysToRemove - = new ArrayList(); + List 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); } } @@ -213,21 +208,19 @@ public void run() { 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(); } } @@ -235,37 +228,33 @@ private void readObject(java.io.ObjectInputStream stream) /* 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 { +class ClientVector extends ArrayDeque { 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 */ @@ -277,21 +266,30 @@ synchronized void put(HttpClient h) { } } + /* 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;