hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r650223 - in /httpcomponents/httpclient/trunk/module-client/src: main/java/org/apache/http/impl/conn/ test/java/org/apache/http/impl/conn/
Date Mon, 21 Apr 2008 19:08:29 GMT
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 {



Mime
View raw message