HTTPCLIENT-1655: HttpClient sends RST instead of FIN ACK sequence when using non-persistant connections

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

httpclient/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java

@@ -31,8 +31,7 @@
 /**
  * Interface for releasing a connection. This can be implemented by various
  * "trigger" objects which are associated with a connection, for example
- * a {@link EofSensorInputStream stream} or an {@link BasicManagedEntity entity}
+ * a {@link EofSensorInputStream} or the {@link ManagedHttpClientConnection} itself.
- * or the {@link ManagedClientConnection connection} itself.
  * <p>
  * The methods in this interface can safely be called multiple times.
  * The first invocation releases the connection, subsequent calls

httpclient/src/main/java/org/apache/http/impl/execchain/ConnectionHolder.java

@@ -30,6 +30,7 @@
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.commons.logging.Log;
 import org.apache.http.HttpClientConnection;
@@ -50,13 +51,12 @@ class ConnectionHolder implements ConnectionReleaseTrigger, Cancellable, Closeab
 
     private final HttpClientConnectionManager manager;
     private final HttpClientConnection managedConn;
+    private final AtomicBoolean released;
     private volatile boolean reusable;
     private volatile Object state;
     private volatile long validDuration;
     private volatile TimeUnit tunit;
 
-    private volatile boolean released;
-
     public ConnectionHolder(
             final Log log,
             final HttpClientConnectionManager manager,
@@ -65,6 +65,7 @@ public ConnectionHolder(
         this.log = log;
         this.manager = manager;
         this.managedConn = managedConn;
+        this.released = new AtomicBoolean(false);
     }
 
     public boolean isReusable() {
@@ -90,14 +91,10 @@ public void setValidFor(final long duration, final TimeUnit tunit) {
         }
     }
 
-    @Override
+    private void releaseConnection(final boolean reusable) {
-    public void releaseConnection() {
+        if (this.released.compareAndSet(false, true)) {
             synchronized (this.managedConn) {
-            if (this.released) {
+                if (reusable) {
-                return;
-            }
-            this.released = true;
-            if (this.reusable) {
                     this.manager.releaseConnection(this.managedConn,
                             this.state, this.validDuration, this.tunit);
                 } else {
@@ -115,14 +112,17 @@ public void releaseConnection() {
                 }
             }
         }
+    }
+
+    @Override
+    public void releaseConnection() {
+        releaseConnection(this.reusable);
+    }
 
     @Override
     public void abortConnection() {
+        if (this.released.compareAndSet(false, true)) {
             synchronized (this.managedConn) {
-            if (this.released) {
-                return;
-            }
-            this.released = true;
                 try {
                     this.managedConn.shutdown();
                     log.debug("Connection discarded");
@@ -136,22 +136,23 @@ public void abortConnection() {
                 }
             }
         }
+    }
 
     @Override
     public boolean cancel() {
-        final boolean alreadyReleased = this.released;
+        final boolean alreadyReleased = this.released.get();
         log.debug("Cancelling request execution");
         abortConnection();
         return !alreadyReleased;
     }
 
     public boolean isReleased() {
-        return this.released;
+        return this.released.get();
     }
 
     @Override
     public void close() throws IOException {
-        abortConnection();
+        releaseConnection(false);
     }
 
 }

httpclient/src/main/java/org/apache/http/impl/execchain/HttpResponseProxy.java

@@ -61,7 +61,7 @@ public HttpResponseProxy(final HttpResponse original, final ConnectionHolder con
     @Override
     public void close() throws IOException {
         if (this.connHolder != null) {
-            this.connHolder.abortConnection();
+            this.connHolder.close();
         }
     }
 

httpclient/src/main/java/org/apache/http/impl/execchain/ResponseEntityProxy.java

@@ -61,7 +61,13 @@ public static void enchance(final HttpResponse response, final ConnectionHolder
         this.connHolder = connHolder;
     }
 
-    private void cleanup() {
+    private void cleanup() throws IOException {
+        if (this.connHolder != null) {
+            this.connHolder.close();
+        }
+    }
+
+    private void abortConnection() throws IOException {
         if (this.connHolder != null) {
             this.connHolder.abortConnection();
         }
@@ -69,14 +75,8 @@ private void cleanup() {
 
     public void releaseConnection() throws IOException {
         if (this.connHolder != null) {
-            try {
-                if (this.connHolder.isReusable()) {
             this.connHolder.releaseConnection();
         }
-            } finally {
-                cleanup();
-            }
-        }
     }
 
     @Override
@@ -100,6 +100,12 @@ public void writeTo(final OutputStream outstream) throws IOException {
         try {
             this.wrappedEntity.writeTo(outstream);
             releaseConnection();
+        } catch (IOException ex) {
+            abortConnection();
+            throw ex;
+        } catch (RuntimeException ex) {
+            abortConnection();
+            throw ex;
         } finally {
             cleanup();
         }
@@ -112,6 +118,12 @@ public boolean eofDetected(final InputStream wrapped) throws IOException {
             // reading trailers after the response body:
             wrapped.close();
             releaseConnection();
+        } catch (IOException ex) {
+            abortConnection();
+            throw ex;
+        } catch (RuntimeException ex) {
+            abortConnection();
+            throw ex;
         } finally {
             cleanup();
         }
@@ -132,6 +144,12 @@ public boolean streamClosed(final InputStream wrapped) throws IOException {
                     throw ex;
                 }
             }
+        } catch (IOException ex) {
+            abortConnection();
+            throw ex;
+        } catch (RuntimeException ex) {
+            abortConnection();
+            throw ex;
         } finally {
             cleanup();
         }

httpclient/src/test/java/org/apache/http/impl/execchain/TestMainClientExec.java

@@ -305,7 +305,7 @@ public void testExecRequestConnectionRelease() throws Exception {
 
         Mockito.verify(connManager, Mockito.times(1)).releaseConnection(
                 managedConn, null, 0, TimeUnit.MILLISECONDS);
-        Mockito.verify(managedConn, Mockito.times(1)).shutdown();
+        Mockito.verify(managedConn, Mockito.times(1)).close();
     }
 
     @Test

httpclient/src/test/java/org/apache/http/impl/execchain/TestMinimalClientExec.java

@@ -202,7 +202,7 @@ public void testExecRequestConnectionRelease() throws Exception {
 
         Mockito.verify(connManager, Mockito.times(1)).releaseConnection(
                 managedConn, null, 0, TimeUnit.MILLISECONDS);
-        Mockito.verify(managedConn, Mockito.times(1)).shutdown();
+        Mockito.verify(managedConn, Mockito.times(1)).close();
     }
 
     @Test

httpclient/src/test/java/org/apache/http/impl/execchain/TestResponseEntityWrapper.java

@@ -85,7 +85,7 @@ public void testEntityStreamClosedIOErrorAlreadyReleased() throws Exception {
         Mockito.when(connHolder.isReleased()).thenReturn(true);
         Mockito.doThrow(new SocketException()).when(instream).close();
         EntityUtils.consume(wrapper);
-        Mockito.verify(connHolder).abortConnection();
+        Mockito.verify(connHolder).close();
     }
 
     @Test