Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 64 additions & 66 deletions jdk/src/share/classes/sun/net/www/http/KeepAliveCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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<Void>() {
AccessController.doPrivileged(new PrivilegedAction<Void>() {
public Void run() {
// We want to create the Keep-Alive-Timer in the
// system threadgroup
Expand All @@ -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 {
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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<KeepAliveKey> keysToRemove
= new ArrayList<KeepAliveKey>();
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);
}
}
Expand All @@ -213,59 +208,53 @@ 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();
}
}

/* 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 */
Expand All @@ -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;
Expand Down