You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ro...@apache.org on 2007/08/11 14:27:19 UTC

svn commit: r564906 - in /jakarta/httpcomponents/oac.hc3x/trunk: release_notes.txt src/java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java

Author: rolandw
Date: Sat Aug 11 05:27:18 2007
New Revision: 564906

URL: http://svn.apache.org/viewvc?view=rev&rev=564906
Log:
HTTPCLIENT-675 and HTTPCLIENT-676

Modified:
    jakarta/httpcomponents/oac.hc3x/trunk/release_notes.txt
    jakarta/httpcomponents/oac.hc3x/trunk/src/java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java

Modified: jakarta/httpcomponents/oac.hc3x/trunk/release_notes.txt
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/oac.hc3x/trunk/release_notes.txt?view=diff&rev=564906&r1=564905&r2=564906
==============================================================================
--- jakarta/httpcomponents/oac.hc3x/trunk/release_notes.txt (original)
+++ jakarta/httpcomponents/oac.hc3x/trunk/release_notes.txt Sat Aug 11 05:27:18 2007
@@ -1,5 +1,11 @@
 Changes since 3.1 RC 1
 
+* [HTTPCLIENT-676] - Fixed memory leak in MultiThreadedHttpConnectionManager.
+           Contributed by Roland Weber <rolandw at apache.org>
+
+* [HTTPCLIENT-675] - Fixed potential race condition in MultiThreadedHttpConnectionManager.
+           Contributed by Roland Weber <rolandw at apache.org>
+
 * [HTTPCLIENT-665] - Internal collections of HttpState visible to subclasses.
            Contributed by Roland Weber <rolandw at apache.org>
 

Modified: jakarta/httpcomponents/oac.hc3x/trunk/src/java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/oac.hc3x/trunk/src/java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java?view=diff&rev=564906&r1=564905&r2=564906
==============================================================================
--- jakarta/httpcomponents/oac.hc3x/trunk/src/java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java (original)
+++ jakarta/httpcomponents/oac.hc3x/trunk/src/java/org/apache/commons/httpclient/MultiThreadedHttpConnectionManager.java Sat Aug 11 05:27:18 2007
@@ -449,7 +449,7 @@
             // we clone the hostConfiguration
             // so that it cannot be changed once the connection has been retrieved
             hostConfiguration = new HostConfiguration(hostConfiguration);
-            HostConnectionPool hostPool = connectionPool.getHostPool(hostConfiguration);
+            HostConnectionPool hostPool = connectionPool.getHostPool(hostConfiguration, true);
             WaitingThread waitingThread = null;
 
             boolean useTimeout = (timeout > 0);
@@ -557,8 +557,8 @@
      */
     public int getConnectionsInPool(HostConfiguration hostConfiguration) {
         synchronized (connectionPool) {
-            HostConnectionPool hostPool = connectionPool.getHostPool(hostConfiguration);
-            return hostPool.numConnections;
+            HostConnectionPool hostPool = connectionPool.getHostPool(hostConfiguration, false);
+            return (hostPool != null) ? hostPool.numConnections : 0;
         }
     }
 
@@ -755,7 +755,7 @@
          * @return a new connection or <code>null</code> if none are available
          */
         public synchronized HttpConnection createConnection(HostConfiguration hostConfiguration) {
-            HostConnectionPool hostPool = getHostPool(hostConfiguration);
+            HostConnectionPool hostPool = getHostPool(hostConfiguration, true);
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Allocating new connection, hostConfig=" + hostConfiguration);
             }
@@ -779,9 +779,13 @@
          * @param config the host configuration of the connection that was lost
          */
         public synchronized void handleLostConnection(HostConfiguration config) {
-            HostConnectionPool hostPool = getHostPool(config);
+            HostConnectionPool hostPool = getHostPool(config, true);
             hostPool.numConnections--;
-            if (hostPool.numConnections == 0) mapHosts.remove(config);
+            if ((hostPool.numConnections == 0) &&
+                hostPool.waitingThreads.isEmpty()) {
+
+                mapHosts.remove(config);
+            }
             
             numConnections--;
             notifyWaitingThread(config);
@@ -791,15 +795,19 @@
          * Get the pool (list) of connections available for the given hostConfig.
          *
          * @param hostConfiguration the configuraton for the connection pool
-         * @return a pool (list) of connections available for the given config
+         * @param create <code>true</code> to create a pool if not found,
+         *               <code>false</code> to return <code>null</code>
+         *
+         * @return a pool (list) of connections available for the given config,
+         *         or <code>null</code> if neither found nor created
          */
-        public synchronized HostConnectionPool getHostPool(HostConfiguration hostConfiguration) {
+        public synchronized HostConnectionPool getHostPool(HostConfiguration hostConfiguration, boolean create) {
             LOG.trace("enter HttpConnectionManager.ConnectionPool.getHostPool(HostConfiguration)");
 
             // Look for a list of connections for the given config
             HostConnectionPool listConnections = (HostConnectionPool) 
                 mapHosts.get(hostConfiguration);
-            if (listConnections == null) {
+            if ((listConnections == null) && create) {
                 // First time for this config
                 listConnections = new HostConnectionPool();
                 listConnections.hostConfiguration = hostConfiguration;
@@ -819,9 +827,9 @@
 
             HttpConnectionWithReference connection = null;
             
-            HostConnectionPool hostPool = getHostPool(hostConfiguration);
+            HostConnectionPool hostPool = getHostPool(hostConfiguration, false);
 
-            if (hostPool.freeConnections.size() > 0) {
+            if ((hostPool != null) && (hostPool.freeConnections.size() > 0)) {
                 connection = (HttpConnectionWithReference) hostPool.freeConnections.removeLast();
                 freeConnections.remove(connection);
                 // store a reference to this connection so that it can be cleaned up
@@ -883,12 +891,16 @@
 
             connection.close();
 
-            HostConnectionPool hostPool = getHostPool(connectionConfiguration);
+            HostConnectionPool hostPool = getHostPool(connectionConfiguration, true);
             
             hostPool.freeConnections.remove(connection);
             hostPool.numConnections--;
             numConnections--;
-            if (hostPool.numConnections == 0) mapHosts.remove(connectionConfiguration);
+            if ((hostPool.numConnections == 0) &&
+                hostPool.waitingThreads.isEmpty()) {
+
+                mapHosts.remove(connectionConfiguration);
+            }
             
             // remove the connection from the timeout handler
             idleConnectionHandler.remove(connection);            
@@ -915,7 +927,7 @@
          * @see #notifyWaitingThread(HostConnectionPool)
          */
         public synchronized void notifyWaitingThread(HostConfiguration configuration) {
-            notifyWaitingThread(getHostPool(configuration));
+            notifyWaitingThread(getHostPool(configuration, true));
         }
 
         /**
@@ -976,7 +988,7 @@
                     return;
                 }
                 
-                HostConnectionPool hostPool = getHostPool(connectionConfiguration);
+                HostConnectionPool hostPool = getHostPool(connectionConfiguration, true);
 
                 // Put the connect back in the available list and notify a waiter
                 hostPool.freeConnections.add(conn);