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 4 Nov 2003 13:30:17 -0000 @@ -103,6 +103,86 @@ /** 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(); + } + + /** + * Creates and stores a reference to the given connection. The reference will be used + * to reclaim resources if the connection is not properly released. 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( + HttpConnection connection, + HostConfiguration hostConfiguration, + ConnectionPool connectionPool + ) { + + ConnectionSource source = new ConnectionSource(); + source.connectionPool = connectionPool; + source.hostConfiguration = hostConfiguration; + + REFERENCE_TO_CONNECTION_SOURCE.put( + new WeakReference(connection, REFERENCE_QUEUE), + source + ); + } + + /** + * Removes the reference being stored for the given connection. This method should be called + * when a connection has been released and will no longer be used. + * + * @param connection the connection to remove the reference to + * + * @see #storeReferenceToConnection(HttpConnection, HostConfiguration, ConnectionPool) + */ + private static void removeReferenceToConnection(HttpConnection connection) { + + synchronized (REFERENCE_TO_CONNECTION_SOURCE) { + Iterator iter = REFERENCE_TO_CONNECTION_SOURCE.keySet().iterator(); + while (iter.hasNext()) { + WeakReference connectionRef = (WeakReference) iter.next(); + if (connectionRef.get() == connection) { + iter.remove(); + connectionRef.enqueue(); + break; + } + } + } + } + // ----------------------------------------------------- Instance Variables /** Maximum number of connections allowed per host */ private int maxHostConnections = DEFAULT_MAX_HOST_CONNECTIONS; @@ -116,26 +196,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(); } /** @@ -466,10 +531,11 @@ 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 +551,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 @@ -521,6 +601,9 @@ if (hostPool.freeConnections.size() > 0) { connection = (HttpConnection) 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 +631,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 +712,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(conn); if (numConnections == 0) { // for some reason this connection pool didn't already exist LOG.error("Host connection pool not found, hostConfig=" @@ -653,10 +728,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 +762,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 +774,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 +811,7 @@ public void run() { while (true) { try { - Reference ref = referenceQueue.remove(); + Reference ref = REFERENCE_QUEUE.remove(); if (ref != null) { handleReference(ref); } 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 4 Nov 2003 13:30:19 -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();