You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2010/06/02 20:52:05 UTC

svn commit: r950717 - in /httpcomponents/httpclient/trunk: ./ httpclient/src/main/java/org/apache/http/impl/conn/ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ httpclient/src/test/java/org/apache/http/impl/conn/

Author: olegk
Date: Wed Jun  2 18:52:05 2010
New Revision: 950717

URL: http://svn.apache.org/viewvc?rev=950717&view=rev
Log:
HTTPCLIENT-948: works around the race condition described in HTTPCLIENT-948

Added:
    httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestIdleConnectionEviction.java
      - copied, changed from r950688, httpcomponents/httpclient/branches/4.0.x/httpclient/src/test/java/org/apache/http/impl/conn/TestIdleConnectionEviction.java
Modified:
    httpcomponents/httpclient/trunk/   (props changed)
    httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/AbstractPoolEntry.java
    httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java
    httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
    httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java

Propchange: httpcomponents/httpclient/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Jun  2 18:52:05 2010
@@ -1 +1,2 @@
+/httpcomponents/httpclient/branches/4.0.x:950681-950688
 /httpcomponents/httpclient/branches/branch_4_1:755593-811107

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/AbstractPoolEntry.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/AbstractPoolEntry.java?rev=950717&r1=950716&r2=950717&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/AbstractPoolEntry.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/AbstractPoolEntry.java Wed Jun  2 18:52:05 2010
@@ -31,6 +31,7 @@ import java.io.IOException;
 import org.apache.http.HttpHost;
 import org.apache.http.params.HttpParams;
 import org.apache.http.protocol.HttpContext;
+import org.apache.http.annotation.NotThreadSafe;
 import org.apache.http.conn.routing.HttpRoute;
 import org.apache.http.conn.routing.RouteTracker;
 import org.apache.http.conn.ClientConnectionOperator;
@@ -52,6 +53,7 @@ import org.apache.http.conn.OperatedClie
  *
  * @since 4.0
  */
+@NotThreadSafe
 public abstract class AbstractPoolEntry {
 
     /** The connection operator. */

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java?rev=950717&r1=950716&r2=950717&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java Wed Jun  2 18:52:05 2010
@@ -28,6 +28,7 @@ package org.apache.http.impl.conn.tsccm;
 
 import java.lang.ref.ReferenceQueue;
 
+import org.apache.http.annotation.NotThreadSafe;
 import org.apache.http.conn.OperatedClientConnection;
 import org.apache.http.conn.ClientConnectionOperator;
 import org.apache.http.conn.routing.HttpRoute;
@@ -38,6 +39,7 @@ import org.apache.http.impl.conn.Abstrac
  *
  * @since 4.0
  */
+@NotThreadSafe
 public class BasicPoolEntry extends AbstractPoolEntry {
 
     /**

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java?rev=950717&r1=950716&r2=950717&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java Wed Jun  2 18:52:05 2010
@@ -302,9 +302,9 @@ public class ConnPoolByRoute extends Abs
                 }
 
                 if (log.isDebugEnabled()) {
-                    log.debug("Total connections kept alive: " + freeConnections.size());
-                    log.debug("Total issued connections: " + leasedConnections.size());
-                    log.debug("Total allocated connection: " + numConnections + " out of " + maxTotalConnections);
+                    log.debug("[" + route + "] kept alive: " + freeConnections.size() +
+                            ", issued: " + leasedConnections.size() +
+                            ", allocated: " + numConnections + " out of " + maxTotalConnections);
                 }
 
                 // the cases to check for:

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java?rev=950717&r1=950716&r2=950717&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java Wed Jun  2 18:52:05 2010
@@ -196,8 +196,7 @@ public class ThreadSafeClientConnManager
                 }
 
                 if (log.isDebugEnabled()) {
-                    log.debug("ThreadSafeClientConnManager.getConnection: "
-                        + route + ", timeout = " + timeout);
+                    log.debug("Get connection: " + route + ", timeout = " + timeout);
                 }
 
                 BasicPoolEntry entry = poolRequest.getPoolEntry(timeout, tunit);
@@ -285,11 +284,11 @@ public class ThreadSafeClientConnManager
      * @return the total number of pooled connections
      */
     public int getConnectionsInPool() {
-        pool.poolLock.lock();
+        connectionPool.poolLock.lock();
         try {
-            return pool.numConnections;
+            return connectionPool.numConnections;
         } finally {
-            pool.poolLock.unlock();
+            connectionPool.poolLock.unlock();
         }
     }
 
@@ -297,14 +296,24 @@ public class ThreadSafeClientConnManager
         if (log.isDebugEnabled()) {
             log.debug("Closing connections idle for " + idleTimeout + " " + tunit);
         }
-        pool.closeIdleConnections(idleTimeout, tunit);
-        pool.deleteClosedConnections();
+        connectionPool.poolLock.lock();
+        try {
+            connectionPool.closeIdleConnections(idleTimeout, tunit);
+            connectionPool.deleteClosedConnections();
+        } finally {
+            connectionPool.poolLock.unlock();
+        }
     }
 
     public void closeExpiredConnections() {
         log.debug("Closing expired connections");
-        pool.closeExpiredConnections();
-        pool.deleteClosedConnections();
+        connectionPool.poolLock.lock();
+        try {
+            connectionPool.closeExpiredConnections();
+            connectionPool.deleteClosedConnections();
+        } finally {
+            connectionPool.poolLock.unlock();
+        }
     }
 
     /**

Copied: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestIdleConnectionEviction.java (from r950688, httpcomponents/httpclient/branches/4.0.x/httpclient/src/test/java/org/apache/http/impl/conn/TestIdleConnectionEviction.java)
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestIdleConnectionEviction.java?p2=httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestIdleConnectionEviction.java&p1=httpcomponents/httpclient/branches/4.0.x/httpclient/src/test/java/org/apache/http/impl/conn/TestIdleConnectionEviction.java&r1=950688&r2=950717&rev=950717&view=diff
==============================================================================
--- httpcomponents/httpclient/branches/4.0.x/httpclient/src/test/java/org/apache/http/impl/conn/TestIdleConnectionEviction.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestIdleConnectionEviction.java Wed Jun  2 18:52:05 2010
@@ -27,11 +27,9 @@
 
 package org.apache.http.impl.conn;
 
+import java.net.InetSocketAddress;
 import java.util.concurrent.TimeUnit;
 
-import junit.framework.Test;
-import junit.framework.TestSuite;
-
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
@@ -40,8 +38,6 @@ import org.apache.http.client.HttpClient
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.conn.ClientConnectionManager;
-import org.apache.http.conn.params.ConnManagerParams;
-import org.apache.http.conn.params.ConnPerRouteBean;
 import org.apache.http.conn.scheme.PlainSocketFactory;
 import org.apache.http.conn.scheme.Scheme;
 import org.apache.http.conn.scheme.SchemeRegistry;
@@ -52,47 +48,38 @@ import org.apache.http.localserver.Serve
 import org.apache.http.params.BasicHttpParams;
 import org.apache.http.params.HttpConnectionParams;
 import org.apache.http.params.HttpParams;
+import org.junit.Before;
+import org.junit.Test;
 
 public class TestIdleConnectionEviction extends ServerTestBase {
 
-    public TestIdleConnectionEviction(String testName) {
-        super(testName);
-    }
-
-    public static void main(String args[]) {
-        String[] testCaseName = { TestIdleConnectionEviction.class.getName() };
-        junit.textui.TestRunner.main(testCaseName);
-    }
-
-    public static Test suite() {
-        return new TestSuite(TestIdleConnectionEviction.class);
-    }
-
-    @Override
-    protected void setUp() throws Exception {
+    @Before
+    public void setUp() throws Exception {
         this.localServer = new LocalTestServer(null, null);
         this.localServer.registerDefaultHandlers();
         this.localServer.start();
     }
-    
+
+    @Test
     public void testIdleConnectionEviction() throws Exception {
         HttpParams params = new BasicHttpParams();
         HttpConnectionParams.setStaleCheckingEnabled(params, false);
-        ConnManagerParams.setMaxTotalConnections(params, 50);
-        ConnManagerParams.setMaxConnectionsPerRoute(params, new ConnPerRouteBean(10));
-        
+
         SchemeRegistry schemeRegistry = new SchemeRegistry();
-        schemeRegistry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));
-        
-        ThreadSafeClientConnManager cm = new ThreadSafeClientConnManager(params, schemeRegistry);
+        schemeRegistry.register(new Scheme("http", 80, PlainSocketFactory.getSocketFactory()));
+
+        ThreadSafeClientConnManager cm = new ThreadSafeClientConnManager(schemeRegistry);
+        cm.setDefaultMaxPerRoute(10);
+        cm.setMaxTotalConnections(50);
+
         DefaultHttpClient httpclient = new DefaultHttpClient(cm, params);
-        
-        IdleConnectionMonitor idleConnectionMonitor = new IdleConnectionMonitor(cm);          
+
+        IdleConnectionMonitor idleConnectionMonitor = new IdleConnectionMonitor(cm);
         idleConnectionMonitor.start();
-        
-        HttpHost target = new HttpHost(this.localServer.getServiceHostName(), 
-                this.localServer.getServicePort());
-        HttpGet httpget = new HttpGet("/random/1024"); 
+
+        InetSocketAddress address = this.localServer.getServiceAddress();
+        HttpHost target = new HttpHost(address.getHostName(), address.getPort());
+        HttpGet httpget = new HttpGet("/random/1024");
         WorkerThread[] workers = new WorkerThread[5];
         for (int i = 0; i < workers.length; i++) {
             workers[i] = new WorkerThread(httpclient, target, httpget, 2000);
@@ -109,20 +96,20 @@ public class TestIdleConnectionEviction 
         }
         idleConnectionMonitor.shutdown();
     }
-    
+
     static class WorkerThread extends Thread {
 
         private final HttpClient httpclient;
         private final HttpHost target;
         private final HttpUriRequest request;
         private final int count;
-        
+
         private volatile Exception ex;
-        
+
         public WorkerThread(
-                final HttpClient httpclient, 
-                final HttpHost target, 
-                final HttpUriRequest request, 
+                final HttpClient httpclient,
+                final HttpHost target,
+                final HttpUriRequest request,
                 int count) {
             super();
             this.httpclient = httpclient;
@@ -130,7 +117,7 @@ public class TestIdleConnectionEviction 
             this.request = request;
             this.count = count;
         }
-        
+
         @Override
         public void run() {
             try {
@@ -154,14 +141,14 @@ public class TestIdleConnectionEviction 
         public Exception getException() {
             return ex;
         }
-        
+
     }
 
     public static class IdleConnectionMonitor extends Thread {
-        
+
         private final ClientConnectionManager cm;
         private volatile boolean shutdown;
-        
+
         public IdleConnectionMonitor(final ClientConnectionManager cm) {
             super();
             this.cm = cm;
@@ -181,14 +168,14 @@ public class TestIdleConnectionEviction 
                 // terminate
             }
         }
-        
+
         public void shutdown() {
             this.shutdown = true;
             synchronized (this) {
                 notifyAll();
             }
         }
-        
+
     }
-    
+
 }