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 2008/04/21 21:08:29 UTC

svn commit: r650223 - in /httpcomponents/httpclient/trunk/module-client/src: main/java/org/apache/http/impl/conn/ test/java/org/apache/http/impl/conn/

Author: olegk
Date: Mon Apr 21 12:08:17 2008
New Revision: 650223

URL: http://svn.apache.org/viewvc?rev=650223&view=rev
Log:
HTTPCLIENT-763: AbstractClientConnAdapter#abortConnection() does not release the connection if called from the main execution thread while there is no blocking I/O operation.

AbstractClientConnAdapter#abortConnection() is usually expected to be called from a helper thread in order to unblock the main execution thread blocked in an I/O operation. It may be unsafe to call AbstractClientConnAdapter#releaseConnection() from the helper thread, so we have to rely on an IOException thrown by the closed socket on the main thread to trigger the release of the connection back to the connection manager. However, if this method is called from the main execution thread it should be safe to release the connection immediately. Besides, this also helps ensure the connection gets released back to the manager if AbstractClientConnAdapter#abortConnection() is called from the main execution thread while there is no blocking I/O operation.

Modified:
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java
    httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java?rev=650223&r1=650222&r2=650223&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java Mon Apr 21 12:08:17 2008
@@ -78,6 +78,9 @@
 public abstract class AbstractClientConnAdapter
     implements ManagedClientConnection {
 
+    /** Thread that requested this connection. */
+    private final Thread executionThread; 
+    
     /**
      * The connection manager, if any.
      * This attribute MUST NOT be final, so the adapter can be detached
@@ -103,7 +106,8 @@
      */
     protected AbstractClientConnAdapter(ClientConnectionManager mgr,
                                         OperatedClientConnection conn) {
-
+        super();
+        executionThread = Thread.currentThread();
         connManager = mgr;
         wrappedConnection = conn;
         markedReusable = false;
@@ -361,6 +365,22 @@
             try {
                 conn.shutdown();
             } catch (IOException ignore) {
+            }
+            // Usually #abortConnection() is expected to be called from 
+            // a helper thread in order to unblock the main execution thread 
+            // blocked in an I/O operation. It may be unsafe to call 
+            // #releaseConnection() from the helper thread, so we have to rely
+            // on an IOException thrown by the closed socket on the main thread 
+            // to trigger the release of the connection back to the 
+            // connection manager.
+            // 
+            // However, if this method is called from the main execution thread 
+            // it should be safe to release the connection immediately. Besides, 
+            // this also helps ensure the connection gets released back to the 
+            // manager if #abortConnection() is called from the main execution 
+            // thread while there is no blocking I/O operation.
+            if (executionThread.equals(Thread.currentThread())) {
+                releaseConnection();
             }
         }
     }

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java?rev=650223&r1=650222&r2=650223&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java Mon Apr 21 12:08:17 2008
@@ -208,7 +208,7 @@
         assertStillUp();
 
         if (LOG.isDebugEnabled()) {
-            LOG.debug("SingleClientConnManager.getConnection: " + route);
+            LOG.debug("Get connection for route " + route);
         }
 
         if (managedConn != null)
@@ -246,6 +246,11 @@
                 ("Connection class mismatch, " +
                  "connection not obtained from this manager.");
         }
+        
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Releasing connection " + conn);
+        }
+
         ConnAdapter sca = (ConnAdapter) conn;
         if (sca.getManager() != this) {
             throw new IllegalArgumentException
@@ -277,6 +282,7 @@
         } finally {
             sca.detach();
             managedConn = null;
+            uniquePoolEntry.tracker = null;
             lastReleaseTime = System.currentTimeMillis();
         }
     } // releaseConnection
@@ -334,11 +340,7 @@
         if (managedConn == null)
             return;
 
-        // Generate a stack trace, it might help debugging.
-        // Do NOT throw the exception, just log it!
-        IllegalStateException isx = new IllegalStateException
-            ("Revoking connection to " + managedConn.getRoute());
-        LOG.warn(MISUSE_MESSAGE, isx);
+        LOG.warn(MISUSE_MESSAGE);
 
         if (managedConn != null)
             managedConn.detach();

Modified: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java?rev=650223&r1=650222&r2=650223&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java Mon Apr 21 12:08:17 2008
@@ -282,6 +282,58 @@
 
 
     /**
+     * Tests releasing connection from #abort method called from the
+     * main execution thread while there is no blocking I/O operation.
+     */
+    public void testReleaseConnectionOnAbort() throws Exception {
+
+        HttpParams mgrpar = defaultParams.copy();
+        HttpConnectionManagerParams.setMaxTotalConnections(mgrpar, 1);
+
+        ThreadSafeClientConnManager mgr = createTSCCM(mgrpar, null);
+
+        final HttpHost target = getServerHttp();
+        final HttpRoute route = new HttpRoute(target, null, false);
+        final int      rsplen = 8;
+        final String      uri = "/random/" + rsplen;
+
+        HttpRequest request =
+            new BasicHttpRequest("GET", uri, HttpVersion.HTTP_1_1);
+
+        ManagedClientConnection conn = getConnection(mgr, route);
+        conn.open(route, httpContext, defaultParams);
+
+        // a new context is created for each testcase, no need to reset
+        HttpResponse response = Helper.execute(
+                request, conn, target, 
+                httpExecutor, httpProcessor, defaultParams, httpContext);
+
+        assertEquals("wrong status in first response",
+                     HttpStatus.SC_OK,
+                     response.getStatusLine().getStatusCode());
+
+        // check that there are no connections available
+        try {
+            // this should fail quickly, connection has not been released
+            getConnection(mgr, route, 100L, TimeUnit.MILLISECONDS);
+            fail("ConnectionPoolTimeoutException should have been thrown");
+        } catch (ConnectionPoolTimeoutException e) {
+            // expected
+        }
+
+        // abort the connection
+        assertTrue(conn instanceof AbstractClientConnAdapter);
+        ((AbstractClientConnAdapter) conn).abortConnection();
+        
+        // the connection is expected to be released back to the manager
+        conn = getConnection(mgr, route, 5L, TimeUnit.SECONDS);
+        assertFalse("connection should have been closed", conn.isOpen());
+
+        mgr.releaseConnection(conn);
+        mgr.shutdown();
+    }
+
+    /**
      * Tests GC of an unreferenced connection.
      */
     public void testConnectionGC() throws Exception {