commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mbe...@apache.org
Subject cvs commit: jakarta-commons/httpclient/src/test/org/apache/commons/httpclient TestHttpConnectionManager.java
Date Wed, 19 Nov 2003 00:43:12 GMT
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 {@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
       /**
        * 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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message