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 2009/10/16 16:07:00 UTC

svn commit: r825901 - in /httpcomponents/httpclient/trunk/httpclient/src: main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java

Author: olegk
Date: Fri Oct 16 14:07:00 2009
New Revision: 825901

URL: http://svn.apache.org/viewvc?rev=825901&view=rev
Log:
HTTPCLIENT-881: It should be safe to release a connection from any thread. The old 'release from current thread hack' is no longer needed

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

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java?rev=825901&r1=825900&r2=825901&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java Fri Oct 16 14:07:00 2009
@@ -67,9 +67,6 @@
  */
 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
@@ -100,14 +97,12 @@
     protected AbstractClientConnAdapter(ClientConnectionManager mgr,
                                         OperatedClientConnection conn) {
         super();
-        executionThread = Thread.currentThread();
         connManager = mgr;
         wrappedConnection = conn;
         markedReusable = false;
         shutdown = false;
         duration = Long.MAX_VALUE;
-    } // <constructor>
-
+    }
 
     /**
      * Detaches this adapter from the wrapped connection.
@@ -316,6 +311,9 @@
     }
 
     public void releaseConnection() {
+        if (shutdown) {
+            return;
+        }
         shutdown = true;
         if (connManager != null) {
             connManager.releaseConnection(this, duration, TimeUnit.MILLISECONDS);
@@ -332,23 +330,8 @@
             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())) {
-            if (connManager != null) {
-                connManager.releaseConnection(this, duration, TimeUnit.MILLISECONDS);
-            }
+        if (connManager != null) {
+            connManager.releaseConnection(this, duration, TimeUnit.MILLISECONDS);
         }
     }
 

Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java?rev=825901&r1=825900&r2=825901&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java Fri Oct 16 14:07:00 2009
@@ -72,20 +72,6 @@
  */
 public class TestTSCCMWithServer extends ServerTestBase {
 
-    // Server-based tests not ported from 3.x TestHttpConnectionManager
-    //
-    // testWriteRequestReleaseConnection
-    //      This tests auto-release in case of an I/O error while writing.
-    //      It's a test of the execution framework, not of the manager.
-    // testConnectMethodFailureRelease
-    //      This tests method.fakeResponse() and auto-release. It's a
-    //      test of a 3.x specific hack and the execution framework.
-    // testResponseAutoRelease
-    //      Auto-release is not part of the connection manager anymore.
-    // testMaxConnectionsPerServer
-    //      Connection limits are already tested without a server.
-
-
     public TestTSCCMWithServer(String testName) {
         super(testName);
     }
@@ -561,18 +547,6 @@
         assertFalse(conn.isOpen());
         assertEquals(0, localServer.getAcceptedConnectionCount());
         
-        // 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
-        }
-
-        // return it back to the manager
-        ((AbstractClientConnAdapter) conn).releaseConnection();
-        
         // the connection is expected to be released back to the manager
         ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS);
         assertFalse("connection should have been closed", conn2.isOpen());
@@ -625,18 +599,6 @@
         assertFalse(conn.isOpen());
         assertEquals(0, localServer.getAcceptedConnectionCount());
         
-        // 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
-        }
-
-        // return it back to the manager
-        ((AbstractClientConnAdapter) conn).releaseConnection();
-        
         // the connection is expected to be released back to the manager
         ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS);
         assertFalse("connection should have been closed", conn2.isOpen());
@@ -694,18 +656,6 @@
         }
         assertEquals(1, localServer.getAcceptedConnectionCount());
         
-        // 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
-        }
-
-        // return it back to the manager
-        ((AbstractClientConnAdapter) conn).releaseConnection();
-        
         // the connection is expected to be released back to the manager
         ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS);
         assertFalse("connection should have been closed", conn2.isOpen());
@@ -770,18 +720,6 @@
         }
         assertEquals(1, localServer.getAcceptedConnectionCount());
         
-        // 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
-        }
-
-        // return it back to the manager
-        ((AbstractClientConnAdapter) conn).releaseConnection();
-        
         // the connection is expected to be released back to the manager
         ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS);
         assertFalse("connection should have been closed", conn2.isOpen());