8155590: Dubious collection management in sun.net.www.http.KeepAliveCache

Reviewed-by: rriggs, vtewari
This commit is contained in:
Christoph Langer 2017-10-19 09:01:29 +02:00
parent 26aeea1790
commit 2e31cc7ee1

View File

@ -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 @@ package sun.net.www.http;
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 @@ 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;
@ -93,8 +101,7 @@ public class KeepAliveCache
* 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 @@ public class KeepAliveCache
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 @@ public class KeepAliveCache
}
/* 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 @@ public class KeepAliveCache
* 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 @@ public class KeepAliveCache
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 @@ public class KeepAliveCache
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 @@ public class KeepAliveCache
/* 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 @@ class ClientVector extends java.util.Stack<KeepAliveEntry> {
}
}
/* 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;