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 2012/03/09 20:45:10 UTC
svn commit: r1299002 - in /httpcomponents/httpcore/trunk/httpcore-nio/src:
main/java/org/apache/http/nio/protocol/
test/java/org/apache/http/nio/protocol/
Author: olegk
Date: Fri Mar 9 19:45:10 2012
New Revision: 1299002
URL: http://svn.apache.org/viewvc?rev=1299002&view=rev
Log:
Added defensive code for a fringe case when the connection leased from the pool gets immediately closed by the opposite endpoint
Modified:
httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/BasicAsyncRequestExecutionHandler.java
httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncRequester.java
httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestBasicAsyncRequestExecutionHandler.java
httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncRequester.java
Modified: httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/BasicAsyncRequestExecutionHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/BasicAsyncRequestExecutionHandler.java?rev=1299002&r1=1299001&r2=1299002&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/BasicAsyncRequestExecutionHandler.java (original)
+++ httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/BasicAsyncRequestExecutionHandler.java Fri Mar 9 19:45:10 2012
@@ -62,6 +62,8 @@ public class BasicAsyncRequestExecutionH
private final ConnectionReuseStrategy reuseStrategy;
private final HttpParams params;
+ private volatile boolean requestSent;
+
public BasicAsyncRequestExecutionHandler(
final HttpAsyncRequestProducer requestProducer,
final HttpAsyncResponseConsumer<T> responseConsumer,
@@ -143,13 +145,11 @@ public class BasicAsyncRequestExecutionH
public void produceContent(
final ContentEncoder encoder, final IOControl ioctrl) throws IOException {
this.requestProducer.produceContent(encoder, ioctrl);
- if (encoder.isCompleted()) {
- this.requestProducer.close();
- }
}
public void requestCompleted(final HttpContext context) {
this.requestProducer.requestCompleted(context);
+ this.requestSent = true;
}
public boolean isRepeatable() {
@@ -171,6 +171,9 @@ public class BasicAsyncRequestExecutionH
public void failed(final Exception ex) {
try {
+ if (!this.requestSent) {
+ this.requestProducer.failed(ex);
+ }
this.responseConsumer.failed(ex);
} finally {
try {
Modified: httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncRequester.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncRequester.java?rev=1299002&r1=1299001&r2=1299002&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncRequester.java (original)
+++ httpcomponents/httpcore/trunk/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncRequester.java Fri Mar 9 19:45:10 2012
@@ -30,6 +30,7 @@ import java.io.Closeable;
import java.io.IOException;
import java.util.concurrent.Future;
+import org.apache.http.ConnectionClosedException;
import org.apache.http.ConnectionReuseStrategy;
import org.apache.http.HttpConnection;
import org.apache.http.HttpHost;
@@ -102,9 +103,22 @@ public class HttpAsyncRequester {
BasicAsyncRequestExecutionHandler<T> handler = new BasicAsyncRequestExecutionHandler<T>(
requestProducer, responseConsumer, callback, context,
this.httppocessor, this.reuseStrategy, this.params);
+ doExecute(handler, conn);
+ return handler.getFuture();
+ }
+
+ private <T> void doExecute(
+ final HttpAsyncRequestExecutionHandler<T> handler, final NHttpClientConnection conn) {
conn.getContext().setAttribute(HttpAsyncRequestExecutor.HTTP_HANDLER, handler);
conn.requestOutput();
- return handler.getFuture();
+ if (!conn.isOpen()) {
+ handler.failed(new ConnectionClosedException("Connection closed"));
+ try {
+ handler.close();
+ } catch (IOException ex) {
+ log(ex);
+ }
+ }
}
/**
@@ -246,8 +260,7 @@ public class HttpAsyncRequester {
this.requestProducer, this.responseConsumer,
new RequestExecutionCallback<T, E>(this.requestFuture, result, this.connPool),
this.context, httppocessor, reuseStrategy, params);
- conn.getContext().setAttribute(HttpAsyncRequestExecutor.HTTP_HANDLER, handler);
- conn.requestOutput();
+ doExecute(handler, conn);
}
public void failed(final Exception ex) {
Modified: httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestBasicAsyncRequestExecutionHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestBasicAsyncRequestExecutionHandler.java?rev=1299002&r1=1299001&r2=1299002&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestBasicAsyncRequestExecutionHandler.java (original)
+++ httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestBasicAsyncRequestExecutionHandler.java Fri Mar 9 19:45:10 2012
@@ -201,7 +201,6 @@ public class TestBasicAsyncRequestExecut
this.exchangeHandler.produceContent(this.encoder, this.conn);
Mockito.verify(this.requestProducer).produceContent(this.encoder, this.conn);
- Mockito.verify(this.requestProducer).close();
}
@Test
@@ -232,6 +231,24 @@ public class TestBasicAsyncRequestExecut
Exception ooopsie = new Exception();
this.exchangeHandler.failed(ooopsie);
+ Mockito.verify(this.requestProducer).failed(ooopsie);
+ Mockito.verify(this.responseConsumer).failed(ooopsie);
+ Mockito.verify(this.requestProducer).close();
+ Mockito.verify(this.responseConsumer).close();
+ try {
+ this.exchangeHandler.getFuture().get();
+ } catch (ExecutionException ex) {
+ Assert.assertSame(ooopsie, ex.getCause());
+ }
+ }
+
+ @Test
+ public void testFailedAfterRequest() throws Exception {
+ Exception ooopsie = new Exception();
+ this.exchangeHandler.requestCompleted(this.context);
+ this.exchangeHandler.failed(ooopsie);
+
+ Mockito.verify(this.requestProducer, Mockito.never()).failed(ooopsie);
Mockito.verify(this.responseConsumer).failed(ooopsie);
Mockito.verify(this.requestProducer).close();
Mockito.verify(this.responseConsumer).close();
Modified: httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncRequester.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncRequester.java?rev=1299002&r1=1299001&r2=1299002&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncRequester.java (original)
+++ httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncRequester.java Fri Mar 9 19:45:10 2012
@@ -27,10 +27,12 @@
package org.apache.http.nio.protocol;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import junit.framework.Assert;
+import org.apache.http.ConnectionClosedException;
import org.apache.http.ConnectionReuseStrategy;
import org.apache.http.HttpHost;
import org.apache.http.concurrent.FutureCallback;
@@ -169,6 +171,30 @@ public class TestHttpAsyncRequester {
Mockito.verify(this.conn).requestOutput();
}
+ @Test
+ public void testExecuteConnectionClosedUnexpectedly() throws Exception {
+ Mockito.when(this.conn.isOpen()).thenReturn(false);
+ Future<Object> future = this.requester.execute(
+ this.requestProducer,
+ this.responseConsumer,
+ this.conn, this.exchangeContext, null);
+ Assert.assertNotNull(future);
+ Mockito.verify(this.requestProducer).failed(Mockito.any(ConnectionClosedException.class));
+ Mockito.verify(this.responseConsumer).failed(Mockito.any(ConnectionClosedException.class));
+ Mockito.verify(this.requestProducer, Mockito.atLeastOnce()).close();
+ Mockito.verify(this.responseConsumer, Mockito.atLeastOnce()).close();
+ Assert.assertTrue(future.isDone());
+ Assert.assertNotNull(future.isDone());
+ try {
+ future.get();
+ } catch (ExecutionException ex) {
+ Throwable cause = ex.getCause();
+ Assert.assertNotNull(cause);
+ Assert.assertTrue(cause instanceof ConnectionClosedException);
+ }
+
+ }
+
@SuppressWarnings({ "rawtypes", "unchecked" })
@Test
public void testPooledConnectionRequestFailed() throws Exception {
@@ -245,6 +271,7 @@ public class TestHttpAsyncRequester {
public void testPooledRequestExecutionSucceeded() throws Exception {
HttpHost host = new HttpHost("somehost");
Mockito.when(this.requestProducer.getTarget()).thenReturn(host);
+ Mockito.when(this.conn.isOpen()).thenReturn(true);
Future<Object> future = this.requester.execute(
this.requestProducer,
@@ -277,6 +304,7 @@ public class TestHttpAsyncRequester {
public void testPooledRequestExecutionFailed() throws Exception {
HttpHost host = new HttpHost("somehost");
Mockito.when(this.requestProducer.getTarget()).thenReturn(host);
+ Mockito.when(this.conn.isOpen()).thenReturn(true);
Future<Object> future = this.requester.execute(
this.requestProducer,
@@ -309,6 +337,7 @@ public class TestHttpAsyncRequester {
public void testPooledRequestExecutionCancelled() throws Exception {
HttpHost host = new HttpHost("somehost");
Mockito.when(this.requestProducer.getTarget()).thenReturn(host);
+ Mockito.when(this.conn.isOpen()).thenReturn(true);
Future<Object> future = this.requester.execute(
this.requestProducer,