You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by mb...@apache.org on 2003/11/19 01:10:45 UTC
cvs commit: jakarta-commons/httpclient/src/test/org/apache/commons/httpclient TestHttpConnectionManager.java
mbecke 2003/11/18 16:10:45
Modified: httpclient/src/java/org/apache/commons/httpclient Tag:
HTTPCLIENT_2_0_BRANCH
MultiThreadedHttpConnectionManager.java
httpclient/src/test/org/apache/commons/httpclient Tag:
HTTPCLIENT_2_0_BRANCH
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
No revision
No revision
1.17.2.5 +153 -61 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.17.2.4
retrieving revision 1.17.2.5
diff -u -r1.17.2.4 -r1.17.2.5
--- MultiThreadedHttpConnectionManager.java 1 Sep 2003 18:05:51 -0000 1.17.2.4
+++ MultiThreadedHttpConnectionManager.java 19 Nov 2003 00:10:45 -0000 1.17.2.5
@@ -103,6 +103,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 {@link #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
/** Maximum number of connections allowed per host */
private int maxHostConnections = DEFAULT_MAX_HOST_CONNECTIONS;
@@ -116,26 +189,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();
}
/**
@@ -451,7 +509,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);
@@ -461,15 +520,16 @@
if (LOG.isDebugEnabled()) {
LOG.debug("Allocating new connection, hostConfig=" + hostConfiguration);
}
- connection = new HttpConnection(hostConfiguration);
+ connection = new HttpConnectionWithReference(hostConfiguration);
connection.setStaleCheckingEnabled(connectionStaleCheckingEnabled);
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 "
@@ -485,6 +545,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
@@ -514,13 +588,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);
}
@@ -548,17 +625,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);
@@ -640,6 +706,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="
@@ -653,10 +722,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;
@@ -674,7 +756,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;
@@ -686,41 +768,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);
+ }
}
/**
@@ -729,7 +805,7 @@
public void run() {
while (true) {
try {
- Reference ref = referenceQueue.remove();
+ Reference ref = REFERENCE_QUEUE.remove();
if (ref != null) {
handleReference(ref);
}
@@ -740,7 +816,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.
No revision
No revision
1.8.2.1 +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.8
retrieving revision 1.8.2.1
diff -u -r1.8 -r1.8.2.1
--- TestHttpConnectionManager.java 28 Apr 2003 23:19:58 -0000 1.8
+++ TestHttpConnectionManager.java 19 Nov 2003 00:10:45 -0000 1.8.2.1
@@ -63,6 +63,7 @@
package org.apache.commons.httpclient;
import java.io.IOException;
+import java.lang.ref.WeakReference;
import junit.framework.Test;
import junit.framework.TestSuite;
@@ -217,6 +218,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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org