HTTPCLIENT-881: Methods mutating the internal state of the AbstractClientConnAdapter class and its subclasses made synchronized; ThreadSafeClientConnManager#releaseConnection method now synchronizes access to the BasicPooledConnAdapter instance

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@825999 13f79535-47bb-0310-9956-ffa450edef68

httpclient/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java

@@ -108,7 +108,7 @@ public abstract class AbstractClientConnAdapter implements ManagedClientConnecti
      * Detaches this adapter from the wrapped connection.
      * This adapter becomes useless.
      */
-    protected void detach() {
+    protected synchronized void detach() {
         wrappedConnection = null;
         connManager = null; // base class attribute
         duration = Long.MAX_VALUE;
@@ -310,7 +310,7 @@ public abstract class AbstractClientConnAdapter implements ManagedClientConnecti
         }
     }
 
-    public void releaseConnection() {
+    public synchronized void releaseConnection() {
         if (shutdown) {
             return;
         }
@@ -320,7 +320,7 @@ public abstract class AbstractClientConnAdapter implements ManagedClientConnecti
         }
     }
 
-    public void abortConnection() {
+    public synchronized void abortConnection() {
         if (shutdown) {
             return;
         }

httpclient/src/main/java/org/apache/http/impl/conn/AbstractPooledConnAdapter.java

@@ -81,9 +81,9 @@ public abstract class AbstractPooledConnAdapter extends AbstractClientConnAdapte
      * This adapter becomes useless.
      */
     @Override
-    protected void detach() {
+    protected synchronized void detach() {
-        super.detach();
         poolEntry = null;
+        super.detach();
     }
 
     public HttpRoute getRoute() {

httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPooledConnAdapter.java

@@ -26,13 +26,10 @@
 
 package org.apache.http.impl.conn.tsccm;
 
-
 import org.apache.http.conn.ClientConnectionManager;
 import org.apache.http.impl.conn.AbstractPoolEntry;
 import org.apache.http.impl.conn.AbstractPooledConnAdapter;
 
-
-
 /**
  * A connection wrapper and callback handler.
  * All connections given out by the manager are wrappers which
@@ -72,7 +69,6 @@ public class BasicPooledConnAdapter extends AbstractPooledConnAdapter {
     }
 
 
-    // non-javadoc, see base class
     @Override
     protected void detach() {
         // override needed only to make method visible in this package

httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java

@@ -220,37 +220,38 @@ public class ThreadSafeClientConnManager implements ClientConnectionManager {
             throw new IllegalArgumentException
                 ("Connection not obtained from this manager.");
         }
-
+        synchronized (hca) {
-        try {
-            // make sure that the response has been read completely
-            if (hca.isOpen() && !hca.isMarkedReusable()) {
-                // In MTHCM, there would be a call to
-                // SimpleHttpConnectionManager.finishLastResponse(conn);
-                // Consuming the response is handled outside in 4.0.
-
-                // make sure this connection will not be re-used
-                // Shut down rather than close, we might have gotten here
-                // because of a shutdown trigger.
-                // Shutdown of the adapter also clears the tracked route.
-                hca.shutdown();
-            }
-        } catch (IOException iox) {
-            //@@@ log as warning? let pass?
-            if (log.isDebugEnabled())
-                log.debug("Exception shutting down released connection.",
-                          iox);
-        } finally {
             BasicPoolEntry entry = (BasicPoolEntry) hca.getPoolEntry();
-            boolean reusable = hca.isMarkedReusable();
+            if (entry == null) {
-            if (log.isDebugEnabled()) {
+                return;
-                if (reusable) {
-                    log.debug("Released connection is reusable.");
-                } else {
-                    log.debug("Released connection is not reusable.");
-                }
             }
-            hca.detach();
+            try {
-            if (entry != null) {
+                // make sure that the response has been read completely
+                if (hca.isOpen() && !hca.isMarkedReusable()) {
+                    // In MTHCM, there would be a call to
+                    // SimpleHttpConnectionManager.finishLastResponse(conn);
+                    // Consuming the response is handled outside in 4.0.
+
+                    // make sure this connection will not be re-used
+                    // Shut down rather than close, we might have gotten here
+                    // because of a shutdown trigger.
+                    // Shutdown of the adapter also clears the tracked route.
+                    hca.shutdown();
+                }
+            } catch (IOException iox) {
+                if (log.isDebugEnabled())
+                    log.debug("Exception shutting down released connection.",
+                              iox);
+            } finally {
+                boolean reusable = hca.isMarkedReusable();
+                if (log.isDebugEnabled()) {
+                    if (reusable) {
+                        log.debug("Released connection is reusable.");
+                    } else {
+                        log.debug("Released connection is not reusable.");
+                    }
+                }
+                hca.detach();
                 pool.freeEntry(entry, reusable, validDuration, timeUnit);
             }
         }