From 78cf770aec88b6bfad9dfbdc12d8b92df2d84616 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Fri, 5 Jun 2015 09:17:41 +0000 Subject: [PATCH] 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/branches/4.5.x@1683684 13f79535-47bb-0310-9956-ffa450edef68 --- .../http/conn/ConnectionReleaseTrigger.java | 3 +- .../http/impl/execchain/ConnectionHolder.java | 73 ++++++++++--------- .../impl/execchain/HttpResponseProxy.java | 2 +- .../impl/execchain/ResponseEntityProxy.java | 34 +++++++-- .../impl/execchain/TestMainClientExec.java | 2 +- .../impl/execchain/TestMinimalClientExec.java | 2 +- .../execchain/TestResponseEntityWrapper.java | 2 +- 7 files changed, 68 insertions(+), 50 deletions(-) diff --git a/httpclient/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java b/httpclient/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java index 1317c227d..8a4763722 100644 --- a/httpclient/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java +++ b/httpclient/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java @@ -31,8 +31,7 @@ import java.io.IOException; /** * 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} - * or the {@link ManagedClientConnection connection} itself. + * a {@link EofSensorInputStream} or the {@link ManagedHttpClientConnection} itself. *

* The methods in this interface can safely be called multiple times. * The first invocation releases the connection, subsequent calls diff --git a/httpclient/src/main/java/org/apache/http/impl/execchain/ConnectionHolder.java b/httpclient/src/main/java/org/apache/http/impl/execchain/ConnectionHolder.java index 1ebf02dae..0f6e4f3e8 100644 --- a/httpclient/src/main/java/org/apache/http/impl/execchain/ConnectionHolder.java +++ b/httpclient/src/main/java/org/apache/http/impl/execchain/ConnectionHolder.java @@ -30,6 +30,7 @@ package org.apache.http.impl.execchain; 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 @@ class ConnectionHolder implements ConnectionReleaseTrigger, Cancellable, Closeab this.log = log; this.manager = manager; this.managedConn = managedConn; + this.released = new AtomicBoolean(false); } public boolean isReusable() { @@ -90,19 +91,40 @@ class ConnectionHolder implements ConnectionReleaseTrigger, Cancellable, Closeab } } + private void releaseConnection(final boolean reusable) { + if (this.released.compareAndSet(false, true)) { + synchronized (this.managedConn) { + if (reusable) { + this.manager.releaseConnection(this.managedConn, + this.state, this.validDuration, this.tunit); + } else { + try { + this.managedConn.close(); + log.debug("Connection discarded"); + } catch (final IOException ex) { + if (this.log.isDebugEnabled()) { + this.log.debug(ex.getMessage(), ex); + } + } finally { + this.manager.releaseConnection( + this.managedConn, null, 0, TimeUnit.MILLISECONDS); + } + } + } + } + } + @Override public void releaseConnection() { - synchronized (this.managedConn) { - if (this.released) { - return; - } - this.released = true; - if (this.reusable) { - this.manager.releaseConnection(this.managedConn, - this.state, this.validDuration, this.tunit); - } else { + releaseConnection(this.reusable); + } + + @Override + public void abortConnection() { + if (this.released.compareAndSet(false, true)) { + synchronized (this.managedConn) { try { - this.managedConn.close(); + this.managedConn.shutdown(); log.debug("Connection discarded"); } catch (final IOException ex) { if (this.log.isDebugEnabled()) { @@ -116,42 +138,21 @@ class ConnectionHolder implements ConnectionReleaseTrigger, Cancellable, Closeab } } - @Override - public void abortConnection() { - synchronized (this.managedConn) { - if (this.released) { - return; - } - this.released = true; - try { - this.managedConn.shutdown(); - log.debug("Connection discarded"); - } catch (final IOException ex) { - if (this.log.isDebugEnabled()) { - this.log.debug(ex.getMessage(), ex); - } - } finally { - this.manager.releaseConnection( - this.managedConn, null, 0, TimeUnit.MILLISECONDS); - } - } - } - @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); } } diff --git a/httpclient/src/main/java/org/apache/http/impl/execchain/HttpResponseProxy.java b/httpclient/src/main/java/org/apache/http/impl/execchain/HttpResponseProxy.java index 4a6ca05b7..0184ec250 100644 --- a/httpclient/src/main/java/org/apache/http/impl/execchain/HttpResponseProxy.java +++ b/httpclient/src/main/java/org/apache/http/impl/execchain/HttpResponseProxy.java @@ -61,7 +61,7 @@ class HttpResponseProxy implements CloseableHttpResponse { @Override public void close() throws IOException { if (this.connHolder != null) { - this.connHolder.abortConnection(); + this.connHolder.close(); } } diff --git a/httpclient/src/main/java/org/apache/http/impl/execchain/ResponseEntityProxy.java b/httpclient/src/main/java/org/apache/http/impl/execchain/ResponseEntityProxy.java index 9f66de65c..7f2c42d35 100644 --- a/httpclient/src/main/java/org/apache/http/impl/execchain/ResponseEntityProxy.java +++ b/httpclient/src/main/java/org/apache/http/impl/execchain/ResponseEntityProxy.java @@ -61,7 +61,13 @@ class ResponseEntityProxy extends HttpEntityWrapper implements EofSensorWatcher 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,13 +75,7 @@ class ResponseEntityProxy extends HttpEntityWrapper implements EofSensorWatcher public void releaseConnection() throws IOException { if (this.connHolder != null) { - try { - if (this.connHolder.isReusable()) { - this.connHolder.releaseConnection(); - } - } finally { - cleanup(); - } + this.connHolder.releaseConnection(); } } @@ -100,6 +100,12 @@ class ResponseEntityProxy extends HttpEntityWrapper implements EofSensorWatcher try { this.wrappedEntity.writeTo(outstream); releaseConnection(); + } catch (IOException ex) { + abortConnection(); + throw ex; + } catch (RuntimeException ex) { + abortConnection(); + throw ex; } finally { cleanup(); } @@ -112,6 +118,12 @@ class ResponseEntityProxy extends HttpEntityWrapper implements EofSensorWatcher // 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 @@ class ResponseEntityProxy extends HttpEntityWrapper implements EofSensorWatcher throw ex; } } + } catch (IOException ex) { + abortConnection(); + throw ex; + } catch (RuntimeException ex) { + abortConnection(); + throw ex; } finally { cleanup(); } diff --git a/httpclient/src/test/java/org/apache/http/impl/execchain/TestMainClientExec.java b/httpclient/src/test/java/org/apache/http/impl/execchain/TestMainClientExec.java index 70e23a815..2261da82b 100644 --- a/httpclient/src/test/java/org/apache/http/impl/execchain/TestMainClientExec.java +++ b/httpclient/src/test/java/org/apache/http/impl/execchain/TestMainClientExec.java @@ -305,7 +305,7 @@ public class TestMainClientExec { 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 diff --git a/httpclient/src/test/java/org/apache/http/impl/execchain/TestMinimalClientExec.java b/httpclient/src/test/java/org/apache/http/impl/execchain/TestMinimalClientExec.java index 7c79f148b..9a96ba686 100644 --- a/httpclient/src/test/java/org/apache/http/impl/execchain/TestMinimalClientExec.java +++ b/httpclient/src/test/java/org/apache/http/impl/execchain/TestMinimalClientExec.java @@ -202,7 +202,7 @@ public class TestMinimalClientExec { 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 diff --git a/httpclient/src/test/java/org/apache/http/impl/execchain/TestResponseEntityWrapper.java b/httpclient/src/test/java/org/apache/http/impl/execchain/TestResponseEntityWrapper.java index 00c06d160..803cb94cd 100644 --- a/httpclient/src/test/java/org/apache/http/impl/execchain/TestResponseEntityWrapper.java +++ b/httpclient/src/test/java/org/apache/http/impl/execchain/TestResponseEntityWrapper.java @@ -85,7 +85,7 @@ public class TestResponseEntityWrapper { 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