mbecke 2003/11/18 16:43:12
Modified: httpclient/src/java/org/apache/commons/httpclient
MultiThreadedHttpConnectionManager.java
httpclient/src/test/org/apache/commons/httpclient
TestHttpConnectionManager.java
Log:
Changed MultiThreadedHttpConnectionManager to move to a single GC thread.
Fixes memory and thread leaks.
PR: 24309
Submitted by: Michael Becke
Reviewed by: Eric Johnson
Revision Changes Path
1.28 +153 -62
jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java
Index: MultiThreadedHttpConnectionManager.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java,v
retrieving revision 1.27
retrieving revision 1.28
diff -u -r1.27 -r1.28
--- MultiThreadedHttpConnectionManager.java 26 Oct 2003 09:49:16 -0000 1.27
+++ MultiThreadedHttpConnectionManager.java 19 Nov 2003 00:43:12 -0000 1.28
@@ -73,7 +73,6 @@
import java.net.SocketException;
import java.util.Collections;
import java.util.HashMap;
-import java.util.Iterator;
import java.util.LinkedList;
import java.util.Map;
@@ -105,6 +104,79 @@
/** The default maximum number of connections allowed overall */
public static final int DEFAULT_MAX_TOTAL_CONNECTIONS = 20;
+ /**
+ * A mapping from Reference to ConnectionSource. Used to reclaim resources
when connections
+ * are lost to the garbage collector.
+ */
+ public static final Map REFERENCE_TO_CONNECTION_SOURCE;
+
+ /**
+ * The reference queue used to track when HttpConnections are lost to the
+ * garbage collector
+ */
+ private static final ReferenceQueue REFERENCE_QUEUE;
+
+ /**
+ * The thread responsible for handling lost connections.
+ */
+ private static ReferenceQueueThread REFERENCE_QUEUE_THREAD;
+
+
+ static {
+ REFERENCE_TO_CONNECTION_SOURCE = Collections.synchronizedMap(new HashMap());
+ REFERENCE_QUEUE = new ReferenceQueue();
+ REFERENCE_QUEUE_THREAD = new ReferenceQueueThread();
+ REFERENCE_QUEUE_THREAD.start();
+ }
+
+ /**
+ * Stores the reference to the given connection along with the hostConfig and
connection pool.
+ * These values will be used to reclaim resources if the connection is lost to
the garbage
+ * collector. This method should be called before a connection is released
from the connection
+ * manager.
+ *
+ * <p>A static reference to the connection manager will also be stored. To
ensure that
+ * the connection manager can be GCed [EMAIL PROTECTED]
#removeReferenceToConnection(HttpConnection)}
+ * should be called for all connections that the connection manager is storing
a reference
+ * to.</p>
+ *
+ * @param connection the connection to create a reference for
+ * @param hostConfiguration the connection's host config
+ * @param connectionPool the connection pool that created the connection
+ *
+ * @see #removeReferenceToConnection(HttpConnection)
+ */
+ private static void storeReferenceToConnection(
+ HttpConnectionWithReference connection,
+ HostConfiguration hostConfiguration,
+ ConnectionPool connectionPool
+ ) {
+
+ ConnectionSource source = new ConnectionSource();
+ source.connectionPool = connectionPool;
+ source.hostConfiguration = hostConfiguration;
+
+ REFERENCE_TO_CONNECTION_SOURCE.put(
+ connection.reference,
+ source
+ );
+ }
+
+ /**
+ * Removes the reference being stored for the given connection. This method
should be called
+ * when the connection manager again has a direct reference to the connection.
+ *
+ * @param connection the connection to remove the reference for
+ *
+ * @see #storeReferenceToConnection(HttpConnection, HostConfiguration,
ConnectionPool)
+ */
+ private static void removeReferenceToConnection(HttpConnectionWithReference
connection) {
+
+ synchronized (REFERENCE_TO_CONNECTION_SOURCE) {
+ REFERENCE_TO_CONNECTION_SOURCE.remove(connection.reference);
+ }
+ }
+
// ----------------------------------------------------- Instance Variables
/**
* Collection of parameters associated with this connection manager.
@@ -114,26 +186,11 @@
/** Connection Pool */
private ConnectionPool connectionPool;
- /** mapping from reference to hostConfiguration */
- private Map referenceToHostConfig;
-
- /**
- * the reference queue used to track when HttpConnections are lost to the
- * garbage collector
- */
- private ReferenceQueue referenceQueue;
-
/**
* No-args constructor
*/
public MultiThreadedHttpConnectionManager() {
-
- this.referenceToHostConfig = Collections.synchronizedMap(new HashMap());
this.connectionPool = new ConnectionPool();
-
- this.referenceQueue = new ReferenceQueue();
-
- new ReferenceQueueThread().start();
}
/**
@@ -241,7 +298,7 @@
"Unexpected exception while waiting for connection",
e
);
- };
+ }
}
}
@@ -524,7 +581,8 @@
* @return a new connection or <code>null</code> if none are available
*/
public synchronized HttpConnection createConnection(HostConfiguration
hostConfiguration) {
- HttpConnection connection = null;
+
+ HttpConnectionWithReference connection = null;
HostConnectionPool hostPool = getHostPool(hostConfiguration);
@@ -534,15 +592,16 @@
if (LOG.isDebugEnabled()) {
LOG.debug("Allocating new connection, hostConfig=" +
hostConfiguration);
}
- connection = new HttpConnection(hostConfiguration);
+ connection = new HttpConnectionWithReference(hostConfiguration);
connection.getParams().setDefaults(MultiThreadedHttpConnectionManager.this.params);
connection.setHttpConnectionManager(MultiThreadedHttpConnectionManager.this);
numConnections++;
hostPool.numConnections++;
- // add a weak reference to this connection
- referenceToHostConfig.put(new WeakReference(connection,
referenceQueue),
- hostConfiguration);
+ // store a reference to this connection so that it can be cleaned up
+ // in the event it is not correctly released
+ storeReferenceToConnection(connection, hostConfiguration, this);
+
} else if (LOG.isDebugEnabled()) {
if (hostPool.numConnections >= getMaxConnectionsPerHost()) {
LOG.debug("No connection allocated, host pool has already
reached "
@@ -558,6 +617,20 @@
}
/**
+ * Handles cleaning up for a lost connection with the given config.
Decrements any
+ * connection counts and notifies waiting threads, if appropriate.
+ *
+ * @param config the host configuration of the connection that was lost
+ */
+ public synchronized void handleLostConnection(HostConfiguration config) {
+ HostConnectionPool hostPool = getHostPool(config);
+ hostPool.numConnections--;
+
+ numConnections--;
+ notifyWaitingThread(config);
+ }
+
+ /**
* Get the pool (list) of connections available for the given hostConfig.
*
* @param hostConfiguration the configuraton for the connection pool
@@ -587,13 +660,16 @@
*/
public synchronized HttpConnection getFreeConnection(HostConfiguration
hostConfiguration) {
- HttpConnection connection = null;
+ HttpConnectionWithReference connection = null;
HostConnectionPool hostPool = getHostPool(hostConfiguration);
if (hostPool.freeConnections.size() > 0) {
- connection = (HttpConnection)
hostPool.freeConnections.removeFirst();
+ connection = (HttpConnectionWithReference)
hostPool.freeConnections.removeFirst();
freeConnections.remove(connection);
+ // store a reference to this connection so that it can be cleaned up
+ // in the event it is not correctly released
+ storeReferenceToConnection(connection, hostConfiguration, this);
if (LOG.isDebugEnabled()) {
LOG.debug("Getting free connection, hostConfig=" +
hostConfiguration);
}
@@ -621,17 +697,6 @@
connection.close();
- // make sure this connection will not be cleaned up again when
garbage
- // collected
- for (Iterator iter = referenceToHostConfig.keySet().iterator();
iter.hasNext();) {
- WeakReference connectionRef = (WeakReference) iter.next();
- if (connectionRef.get() == connection) {
- iter.remove();
- connectionRef.enqueue();
- break;
- }
- }
-
HostConnectionPool hostPool = getHostPool(connectionConfiguration);
hostPool.freeConnections.remove(connection);
@@ -713,6 +778,9 @@
}
freeConnections.add(conn);
+ // we can remove the reference to this connection as we have
control over
+ // it again. this also ensures that the connection manager can be
GCed
+ removeReferenceToConnection((HttpConnectionWithReference) conn);
if (numConnections == 0) {
// for some reason this connection pool didn't already exist
LOG.error("Host connection pool not found, hostConfig="
@@ -726,10 +794,23 @@
}
/**
+ * A simple struct-like class to combine the objects needed to release a
connection's
+ * resources when claimed by the garbage collector.
+ */
+ private static class ConnectionSource {
+
+ /** The connection pool that created the connection */
+ public ConnectionPool connectionPool;
+
+ /** The connection's host configuration */
+ public HostConfiguration hostConfiguration;
+ }
+
+ /**
* A simple struct-like class to combine the connection list and the count
* of created connections.
*/
- private class HostConnectionPool {
+ private static class HostConnectionPool {
/** The hostConfig this pool is for */
public HostConfiguration hostConfiguration;
@@ -747,7 +828,7 @@
* A simple struct-like class to combine the waiting thread and the connection
* pool it is waiting on.
*/
- private class WaitingThread {
+ private static class WaitingThread {
/** The thread that is waiting for a connection */
public Thread thread;
@@ -759,41 +840,35 @@
* A thread for listening for HttpConnections reclaimed by the garbage
* collector.
*/
- private class ReferenceQueueThread extends Thread {
+ private static class ReferenceQueueThread extends Thread {
/**
* Create an instance and make this a daemon thread.
*/
public ReferenceQueueThread() {
setDaemon(true);
+ setName("MultiThreadedHttpConnectionManager cleanup");
}
/**
- * Handles cleaning up for the given reference. Decrements any connection
counts
- * and notifies waiting threads, if appropriate.
+ * Handles cleaning up for the given connection reference.
*
* @param ref the reference to clean up
*/
private void handleReference(Reference ref) {
- synchronized (connectionPool) {
- // only clean up for this reference if it is still associated with
- // a HostConfiguration
- if (referenceToHostConfig.containsKey(ref)) {
- HostConfiguration config = (HostConfiguration)
referenceToHostConfig.get(ref);
- referenceToHostConfig.remove(ref);
-
- if (LOG.isDebugEnabled()) {
- LOG.debug(
- "Connection reclaimed by garbage collector,
hostConfig=" + config);
- }
-
- HostConnectionPool hostPool =
connectionPool.getHostPool(config);
- hostPool.numConnections--;
-
- connectionPool.numConnections--;
- connectionPool.notifyWaitingThread(config);
+
+ ConnectionSource source = (ConnectionSource)
REFERENCE_TO_CONNECTION_SOURCE.remove(ref);
+ // only clean up for this reference if it is still associated with
+ // a ConnectionSource
+ if (source != null) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(
+ "Connection reclaimed by garbage collector, hostConfig="
+ + source.hostConfiguration);
}
- }
+
+
source.connectionPool.handleLostConnection(source.hostConfiguration);
+ }
}
/**
@@ -802,7 +877,7 @@
public void run() {
while (true) {
try {
- Reference ref = referenceQueue.remove();
+ Reference ref = REFERENCE_QUEUE.remove();
if (ref != null) {
handleReference(ref);
}
@@ -813,7 +888,23 @@
}
}
+
+ /**
+ * A connection that keeps a reference to itself.
+ */
+ private static class HttpConnectionWithReference extends HttpConnection {
+
+ public WeakReference reference = new WeakReference(this, REFERENCE_QUEUE);
+
+ /**
+ * @param hostConfiguration
+ */
+ public HttpConnectionWithReference(HostConfiguration hostConfiguration) {
+ super(hostConfiguration);
+ }
+ }
+
/**
* An HttpConnection wrapper that ensures a connection cannot be used
* once released.
1.13 +32 -4
jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestHttpConnectionManager.java
Index: TestHttpConnectionManager.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestHttpConnectionManager.java,v
retrieving revision 1.12
retrieving revision 1.13
diff -u -r1.12 -r1.13
--- TestHttpConnectionManager.java 26 Oct 2003 09:49:16 -0000 1.12
+++ TestHttpConnectionManager.java 19 Nov 2003 00:43:12 -0000 1.13
@@ -63,6 +63,7 @@
package org.apache.commons.httpclient;
import java.io.IOException;
+import java.lang.ref.WeakReference;
import java.net.InetAddress;
import java.net.Socket;
import java.net.UnknownHostException;
@@ -214,6 +215,33 @@
}
+ public void testDroppedThread() throws Exception {
+
+ MultiThreadedHttpConnectionManager mthcm = new
MultiThreadedHttpConnectionManager();
+ HttpClient httpClient = createHttpClient(mthcm);
+ WeakReference wr = new WeakReference(mthcm);
+
+ GetMethod method = new GetMethod("/");
+ httpClient.executeMethod(method);
+ method.releaseConnection();
+
+ mthcm = null;
+ httpClient = null;
+ method = null;
+
+ // this sleep appears to be necessary in order to give the JVM
+ // time to clean up the miscellaneous pointers to the connection manager
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {
+ fail("shouldn't be interrupted.");
+ }
+
+ System.gc();
+ Object connectionManager = wr.get();
+ assertNull("connectionManager should be null", connectionManager);
+ }
+
public void testReleaseConnection() {
MultiThreadedHttpConnectionManager connectionManager = new
MultiThreadedHttpConnectionManager();
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
