Index: java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java =================================================================== RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java,v retrieving revision 1.17.2.4 diff -u -r1.17.2.4 MultiThreadedHttpConnectionManager.java --- java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java 1 Sep 2003 18:05:51 -0000 1.17.2.4 +++ java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java 11 Nov 2003 03:15:20 -0000 @@ -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. + * + *
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.
+ * + * @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 ornull 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.
Index: test/org/apache/commons/httpclient/TestHttpConnectionManager.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestHttpConnectionManager.java,v
retrieving revision 1.8
diff -u -r1.8 TestHttpConnectionManager.java
--- test/org/apache/commons/httpclient/TestHttpConnectionManager.java 28 Apr 2003 23:19:58 -0000 1.8
+++ test/org/apache/commons/httpclient/TestHttpConnectionManager.java 11 Nov 2003 03:15:22 -0000
@@ -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();