From 90f9e38b8dc5b83420679c1eeb493f7ce8a97e6d Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Fri, 16 Oct 2009 14:07:00 +0000 Subject: [PATCH] HTTPCLIENT-881: It should be safe to release a connection from any thread. The old 'release from current thread hack' is no longer needed git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@825901 13f79535-47bb-0310-9956-ffa450edef68 --- .../impl/conn/AbstractClientConnAdapter.java | 29 ++------- .../http/impl/conn/TestTSCCMWithServer.java | 62 ------------------- 2 files changed, 6 insertions(+), 85 deletions(-) diff --git a/httpclient/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java b/httpclient/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java index af7796e5d..771fdcc1a 100644 --- a/httpclient/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java +++ b/httpclient/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java @@ -67,9 +67,6 @@ import org.apache.http.conn.ClientConnectionManager; */ 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 @@ public abstract class AbstractClientConnAdapter implements ManagedClientConnecti protected AbstractClientConnAdapter(ClientConnectionManager mgr, OperatedClientConnection conn) { super(); - executionThread = Thread.currentThread(); connManager = mgr; wrappedConnection = conn; markedReusable = false; shutdown = false; duration = Long.MAX_VALUE; - } // - + } /** * Detaches this adapter from the wrapped connection. @@ -316,6 +311,9 @@ public abstract class AbstractClientConnAdapter implements ManagedClientConnecti } public void releaseConnection() { + if (shutdown) { + return; + } shutdown = true; if (connManager != null) { connManager.releaseConnection(this, duration, TimeUnit.MILLISECONDS); @@ -332,23 +330,8 @@ public abstract class AbstractClientConnAdapter implements ManagedClientConnecti 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); } } diff --git a/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java b/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java index ac3091194..16ec55267 100644 --- a/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java +++ b/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java @@ -72,20 +72,6 @@ import org.apache.http.util.EntityUtils; */ 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 @@ public class TestTSCCMWithServer extends ServerTestBase { 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 @@ public class TestTSCCMWithServer extends ServerTestBase { 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 @@ public class TestTSCCMWithServer extends ServerTestBase { } 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 @@ public class TestTSCCMWithServer extends ServerTestBase { } 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());