Apache
/
httpcomponents-client
mirror of https://github.com/apache/httpcomponents-client.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

GTTPCLIENT-760: Improved handling of request abort in fringe situations such as request abort immediately after a connection has been allocated but before the release trigger has been set 

Contributed by Sam Berlin <sberlin at gmail.com>
Reviewed by Oleg Kalnichevski



git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@639600 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Oleg Kalnichevski 2008-03-21 11:28:15 +00:00
parent c51f3a980f
commit 97bb41b687
8 changed files with 343 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
module-client/src/main/java/org/apache/http/client/ClientRequestDirector.java
View File

@ -92,11 +92,11 @@ public interface ClientRequestDirector {
*
* @throws HttpException in case of a problem
* @throws IOException in case of an IO problem
* @throws InterruptedException in case of an interrupt
* or if the connection was aborted
*/
HttpResponse execute(HttpHost target, HttpRequest request,
HttpContext context)
throws HttpException, IOException, InterruptedException
throws HttpException, IOException
;

16
module-client/src/main/java/org/apache/http/client/HttpClient.java
View File

@ -103,11 +103,11 @@ public interface HttpClient {
*
* @throws HttpException in case of a problem
* @throws IOException in case of an IO problem
* @throws InterruptedException in case of an interrupt
* or the connection was aborted
* <br/><i @@@>timeout exceptions?</i>
*/
HttpResponse execute(HttpUriRequest request)
throws HttpException, IOException, InterruptedException
throws HttpException, IOException
;
@ -128,11 +128,11 @@ public interface HttpClient {
*
* @throws HttpException in case of a problem
* @throws IOException in case of an IO problem
* @throws InterruptedException in case of an interrupt
* or the connection was aborted
* <br/><i @@@>timeout exceptions?</i>
*/
HttpResponse execute(HttpUriRequest request, HttpContext context)
throws HttpException, IOException, InterruptedException
throws HttpException, IOException
;
@ -155,11 +155,11 @@ public interface HttpClient {
*
* @throws HttpException in case of a problem
* @throws IOException in case of an IO problem
* @throws InterruptedException in case of an interrupt
* or the connection was aborted
* <br/><i @@@>timeout exceptions?</i>
*/
HttpResponse execute(HttpHost target, HttpRequest request)
throws HttpException, IOException, InterruptedException
throws HttpException, IOException
;
@ -183,12 +183,12 @@ public interface HttpClient {
*
* @throws HttpException in case of a problem
* @throws IOException in case of an IO problem
* @throws InterruptedException in case of an interrupt
* or the connection was aborted
* <br/><i @@@>timeout exceptions?</i>
*/
HttpResponse execute(HttpHost target, HttpRequest request,
HttpContext context)
throws HttpException, IOException, InterruptedException
throws HttpException, IOException
;

43
module-client/src/main/java/org/apache/http/client/methods/AbortableHttpRequest.java
View File

@ -31,12 +31,18 @@
package org.apache.http.client.methods;
import java.io.IOException;
import org.apache.http.client.HttpClient;
import org.apache.http.conn.ClientConnectionManager;
import org.apache.http.conn.ClientConnectionRequest;
import org.apache.http.conn.ConnectionReleaseTrigger;
import org.apache.http.conn.ManagedClientConnection;
import org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager;
/**
* Interface representing an HTTP request that can be aborted by shutting
* donw the underying HTTP connection.
* down the underlying HTTP connection.
*
* @author <a href="mailto:oleg at ural.ru">Oleg Kalnichevski</a>
*
@ -47,10 +53,39 @@ import org.apache.http.conn.ConnectionReleaseTrigger;
*/
public interface AbortableHttpRequest {
void setConnectionRequest(ClientConnectionRequest connRequest);
void setReleaseTrigger(ConnectionReleaseTrigger releaseTrigger);
/**
* Sets the {@link ClientConnectionRequest} callback that can be
* used to abort a long-lived request for a connection.
* If the request is already aborted, throws an {@link IOException}.
*
* @see ClientConnectionManager
* @see ThreadSafeClientConnManager
*/
void setConnectionRequest(ClientConnectionRequest connRequest) throws IOException;
/**
* Sets the {@link ConnectionReleaseTrigger} callback that can
* be used to abort an active connection.
* Typically, this will be the {@link ManagedClientConnection} itself.
* If the request is already aborted, throws an {@link IOException}.
*/
void setReleaseTrigger(ConnectionReleaseTrigger releaseTrigger) throws IOException;
/**
* Aborts this http request. Any active execution of this method should
* return immediately. If the request has not started, it will abort after
* the next execution. Aborting this request will cause all subsequent
* executions with this request to fail.
*
* @see HttpClient#execute(HttpUriRequest)
* @see HttpClient#execute(org.apache.http.HttpHost,
* org.apache.http.HttpRequest)
* @see HttpClient#execute(HttpUriRequest,
* org.apache.http.protocol.HttpContext)
* @see HttpClient#execute(org.apache.http.HttpHost,
* org.apache.http.HttpRequest, org.apache.http.protocol.HttpContext)
*/
void abort();
}

62
module-client/src/main/java/org/apache/http/client/methods/HttpRequestBase.java
View File

@ -58,7 +58,7 @@ abstract class HttpRequestBase extends AbstractHttpMessage
private final Lock abortLock;
private volatile boolean aborted;
private boolean aborted;
private URI uri;
private ClientConnectionRequest connRequest;
@ -97,24 +97,29 @@ abstract class HttpRequestBase extends AbstractHttpMessage
this.uri = uri;
}
public void setConnectionRequest(final ClientConnectionRequest connRequest) {
if (this.aborted) {
return;
}
public void setConnectionRequest(final ClientConnectionRequest connRequest)
throws IOException {
this.abortLock.lock();
try {
if (this.aborted) {
throw new IOException("Request already aborted");
}
this.releaseTrigger = null;
this.connRequest = connRequest;
} finally {
this.abortLock.unlock();
}
}
public void setReleaseTrigger(final ConnectionReleaseTrigger releaseTrigger) {
if (this.aborted) {
return;
}
public void setReleaseTrigger(final ConnectionReleaseTrigger releaseTrigger)
throws IOException {
this.abortLock.lock();
try {
if (this.aborted) {
throw new IOException("Request already aborted");
}
this.connRequest = null;
this.releaseTrigger = releaseTrigger;
} finally {
@ -123,24 +128,35 @@ abstract class HttpRequestBase extends AbstractHttpMessage
}
public void abort() {
if (this.aborted) {
return;
}
this.aborted = true;
ClientConnectionRequest localRequest;
ConnectionReleaseTrigger localTrigger;
this.abortLock.lock();
try {
if (this.connRequest != null) {
this.connRequest.abortRequest();
}
if (this.releaseTrigger != null) {
try {
this.releaseTrigger.abortConnection();
} catch (IOException ex) {
// ignore
}
}
if (this.aborted) {
return;
}
this.aborted = true;
localRequest = connRequest;
localTrigger = releaseTrigger;
} finally {
this.abortLock.unlock();
}
// Trigger the callbacks outside of the lock, to prevent
// deadlocks in the scenario where the callbacks have
// their own locks that may be used while calling
// setReleaseTrigger or setConnectionRequest.
if (localRequest != null) {
localRequest.abortRequest();
}
if (localTrigger != null) {
try {
localTrigger.abortConnection();
} catch (IOException ex) {
// ignore
}
}
}

8
module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java
View File

@ -416,7 +416,7 @@ public abstract class AbstractHttpClient implements HttpClient {
// non-javadoc, see interface HttpClient
public final HttpResponse execute(HttpUriRequest request)
throws HttpException, IOException, InterruptedException {
throws HttpException, IOException {
return execute(request, null);
}
@ -433,7 +433,7 @@ public abstract class AbstractHttpClient implements HttpClient {
*/
public final HttpResponse execute(HttpUriRequest request,
HttpContext context)
throws HttpException, IOException, InterruptedException {
throws HttpException, IOException {
if (request == null) {
throw new IllegalArgumentException
@ -458,7 +458,7 @@ public abstract class AbstractHttpClient implements HttpClient {
// non-javadoc, see interface HttpClient
public final HttpResponse execute(HttpHost target, HttpRequest request)
throws HttpException, IOException, InterruptedException {
throws HttpException, IOException {
return execute(target, request, null);
}
@ -467,7 +467,7 @@ public abstract class AbstractHttpClient implements HttpClient {
// non-javadoc, see interface HttpClient
public final HttpResponse execute(HttpHost target, HttpRequest request,
HttpContext context)
throws HttpException, IOException, InterruptedException {
throws HttpException, IOException {
if (request == null) {
throw new IllegalArgumentException

16
module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java
View File

@ -32,6 +32,7 @@
package org.apache.http.impl.client;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Map;
@ -267,7 +268,7 @@ public class DefaultClientRequestDirector
// non-javadoc, see interface ClientRequestDirector
public HttpResponse execute(HttpHost target, HttpRequest request,
HttpContext context)
throws HttpException, IOException, InterruptedException {
throws HttpException, IOException {
RoutedRequest roureq = determineRoute(target, request, context);
@ -294,7 +295,14 @@ public class DefaultClientRequestDirector
if (orig instanceof AbortableHttpRequest) {
((AbortableHttpRequest) orig).setConnectionRequest(connRequest);
}
managedConn = connRequest.getConnection(timeout, TimeUnit.MILLISECONDS);
try {
managedConn = connRequest.getConnection(timeout, TimeUnit.MILLISECONDS);
} catch(InterruptedException interrupted) {
InterruptedIOException iox = new InterruptedIOException();
iox.initCause(interrupted);
throw iox;
}
}
if (orig instanceof AbortableHttpRequest) {
@ -444,9 +452,6 @@ public class DefaultClientRequestDirector
} catch (IOException ex) {
abortConnection();
throw ex;
} catch (InterruptedException ex) {
abortConnection();
throw ex;
} catch (RuntimeException ex) {
abortConnection();
throw ex;
@ -916,7 +921,6 @@ public class DefaultClientRequestDirector
* @throws IOException in case of an IO problem
*/
private void abortConnection() throws IOException {
ManagedClientConnection mcc = managedConn;
if (mcc != null) {
// we got here as the result of an exception

242
module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java
View File

@ -30,6 +30,7 @@ package org.apache.http.impl.client;
import java.io.IOException;
import java.net.ConnectException;
import java.net.URISyntaxException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
@ -38,20 +39,28 @@ import junit.framework.Test;
import junit.framework.TestSuite;
import org.apache.http.HttpException;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.ProtocolVersion;
import org.apache.http.client.methods.AbortableHttpRequest;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.conn.ClientConnectionManager;
import org.apache.http.conn.ClientConnectionRequest;
import org.apache.http.conn.ConnectionPoolTimeoutException;
import org.apache.http.conn.ConnectionReleaseTrigger;
import org.apache.http.conn.ManagedClientConnection;
import org.apache.http.conn.PlainSocketFactory;
import org.apache.http.conn.Scheme;
import org.apache.http.conn.SchemeRegistry;
import org.apache.http.conn.routing.HttpRoute;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.conn.ClientConnAdapterMockup;
import org.apache.http.impl.conn.SingleClientConnManager;
import org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager;
import org.apache.http.localserver.ServerTestBase;
import org.apache.http.message.BasicHeader;
import org.apache.http.mockup.SocketFactoryMockup;
import org.apache.http.params.BasicHttpParams;
import org.apache.http.params.HttpParams;
@ -108,8 +117,140 @@ public class TestDefaultClientRequestDirector extends ServerTestBase {
httpget.abort();
assertTrue("should have finished get request", getLatch.await(1, TimeUnit.SECONDS));
assertTrue("should be instanceof InterruptedException, was: " + throwableRef.get(),
throwableRef.get() instanceof InterruptedException);
assertTrue("should be instanceof IOException, was: " + throwableRef.get(),
throwableRef.get() instanceof IOException);
assertTrue("cause should be InterruptedException, was: " + throwableRef.get().getCause(),
throwableRef.get().getCause() instanceof InterruptedException);
}
/**
* Tests that an abort called after the connection has been retrieved
* but before a release trigger is set does still abort the request.
*/
public void testAbortAfterAllocateBeforeRequest() throws Exception {
this.localServer.register("*", new BasicService());
CountDownLatch releaseLatch = new CountDownLatch(1);
SchemeRegistry registry = new SchemeRegistry();
registry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));
SingleClientConnManager conMan = new SingleClientConnManager(new BasicHttpParams(), registry);
final AtomicReference<Throwable> throwableRef = new AtomicReference<Throwable>();
final CountDownLatch getLatch = new CountDownLatch(1);
final DefaultHttpClient client = new DefaultHttpClient(conMan, new BasicHttpParams());
final HttpContext context = client.getDefaultContext();
final HttpGet httpget = new CustomGet("a", releaseLatch);
new Thread(new Runnable() {
public void run() {
try {
client.execute(getServerHttp(), httpget, context);
} catch(Throwable t) {
throwableRef.set(t);
} finally {
getLatch.countDown();
}
}
}).start();
Thread.sleep(100); // Give it a little time to proceed to release...
httpget.abort();
releaseLatch.countDown();
assertTrue("should have finished get request", getLatch.await(1, TimeUnit.SECONDS));
assertTrue("should be instanceof IOException, was: " + throwableRef.get(),
throwableRef.get() instanceof IOException);
}
/**
* Tests that an abort called completely before execute
* still aborts the request.
*/
public void testAbortBeforeExecute() throws Exception {
this.localServer.register("*", new BasicService());
SchemeRegistry registry = new SchemeRegistry();
registry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));
SingleClientConnManager conMan = new SingleClientConnManager(new BasicHttpParams(), registry);
final AtomicReference<Throwable> throwableRef = new AtomicReference<Throwable>();
final CountDownLatch getLatch = new CountDownLatch(1);
final CountDownLatch startLatch = new CountDownLatch(1);
final DefaultHttpClient client = new DefaultHttpClient(conMan, new BasicHttpParams());
final HttpContext context = client.getDefaultContext();
final HttpGet httpget = new HttpGet("a");
new Thread(new Runnable() {
public void run() {
try {
try {
if(!startLatch.await(1, TimeUnit.SECONDS))
throw new RuntimeException("Took too long to start!");
} catch(InterruptedException interrupted) {
throw new RuntimeException("Never started!", interrupted);
}
client.execute(getServerHttp(), httpget, context);
} catch(Throwable t) {
throwableRef.set(t);
} finally {
getLatch.countDown();
}
}
}).start();
httpget.abort();
startLatch.countDown();
assertTrue("should have finished get request", getLatch.await(1, TimeUnit.SECONDS));
assertTrue("should be instanceof IOException, was: " + throwableRef.get(),
throwableRef.get() instanceof IOException);
}
/**
* Tests that an abort called after a redirect has found a new host
* still aborts in the correct place (while trying to get the new
* host's route, not while doing the subsequent request).
*/
public void testAbortAfterRedirectedRoute() throws Exception {
final int port = this.localServer.getServicePort();
this.localServer.register("*", new BasicRedirectService(port));
SchemeRegistry registry = new SchemeRegistry();
registry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));
CountDownLatch connLatch = new CountDownLatch(1);
CountDownLatch awaitLatch = new CountDownLatch(1);
ConnMan4 conMan = new ConnMan4(new BasicHttpParams(), registry, connLatch, awaitLatch);
final AtomicReference<Throwable> throwableRef = new AtomicReference<Throwable>();
final CountDownLatch getLatch = new CountDownLatch(1);
final DefaultHttpClient client = new DefaultHttpClient(conMan, new BasicHttpParams());
final HttpContext context = client.getDefaultContext();
final HttpGet httpget = new HttpGet("a");
new Thread(new Runnable() {
public void run() {
try {
HttpHost host = new HttpHost("127.0.0.1", port);
client.execute(host, httpget, context);
} catch(Throwable t) {
throwableRef.set(t);
} finally {
getLatch.countDown();
}
}
}).start();
assertTrue("should have tried to get a connection", connLatch.await(1, TimeUnit.SECONDS));
httpget.abort();
assertTrue("should have finished get request", getLatch.await(1, TimeUnit.SECONDS));
assertTrue("should be instanceof IOException, was: " + throwableRef.get(),
throwableRef.get() instanceof IOException);
assertTrue("cause should be InterruptedException, was: " + throwableRef.get().getCause(),
throwableRef.get().getCause() instanceof InterruptedException);
}
@ -160,6 +301,81 @@ public class TestDefaultClientRequestDirector extends ServerTestBase {
}
}
private static class BasicService implements HttpRequestHandler {
public void handle(final HttpRequest request,
final HttpResponse response,
final HttpContext context) throws HttpException, IOException {
response.setStatusCode(200);
response.setEntity(new StringEntity("Hello World"));
}
}
private class BasicRedirectService implements HttpRequestHandler {
private int statuscode = HttpStatus.SC_SEE_OTHER;
private int port;
public BasicRedirectService(int port) {
this.port = port;
}
public void handle(final HttpRequest request,
final HttpResponse response, final HttpContext context)
throws HttpException, IOException {
ProtocolVersion ver = request.getRequestLine().getProtocolVersion();
response.setStatusLine(ver, this.statuscode);
response.addHeader(new BasicHeader("Location", "http://localhost:"
+ this.port + "/newlocation/"));
response.addHeader(new BasicHeader("Connection", "close"));
}
}
private static class ConnMan4 extends ThreadSafeClientConnManager {
private final CountDownLatch connLatch;
private final CountDownLatch awaitLatch;
public ConnMan4(HttpParams params, SchemeRegistry schreg,
CountDownLatch connLatch, CountDownLatch awaitLatch) {
super(params, schreg);
this.connLatch = connLatch;
this.awaitLatch = awaitLatch;
}
@Override
public ClientConnectionRequest requestConnection(HttpRoute route) {
// If this is the redirect route, stub the return value
// so-as to pretend the host is waiting on a slot...
if(route.getTargetHost().getHostName().equals("localhost")) {
final Thread currentThread = Thread.currentThread();
return new ClientConnectionRequest() {
public void abortRequest() {
currentThread.interrupt();
}
public ManagedClientConnection getConnection(
long timeout, TimeUnit tunit)
throws InterruptedException,
ConnectionPoolTimeoutException {
connLatch.countDown(); // notify waiter that we're getting a connection
// zero usually means sleep forever, but CountDownLatch doesn't interpret it that way.
if(timeout == 0)
timeout = Integer.MAX_VALUE;
if(!awaitLatch.await(timeout, tunit))
throw new ConnectionPoolTimeoutException();
return new ClientConnAdapterMockup();
}
};
} else {
return super.requestConnection(route);
}
}
}
private static class ConnMan3 extends SingleClientConnManager {
private ManagedClientConnection allocatedConnection;
private ManagedClientConnection releasedConnection;
@ -318,4 +534,26 @@ public class TestDefaultClientRequestDirector extends ServerTestBase {
}
}
private static class CustomGet extends HttpGet {
private final CountDownLatch releaseTriggerLatch;
public CustomGet(String uri, CountDownLatch releaseTriggerLatch) throws URISyntaxException {
super(uri);
this.releaseTriggerLatch = releaseTriggerLatch;
}
@Override
public void setReleaseTrigger(ConnectionReleaseTrigger releaseTrigger) throws IOException {
try {
if(!releaseTriggerLatch.await(1, TimeUnit.SECONDS))
throw new RuntimeException("Waited too long...");
} catch(InterruptedException ie) {
throw new RuntimeException(ie);
}
super.setReleaseTrigger(releaseTrigger);
}
}
}

2
module-client/src/test/java/org/apache/http/impl/conn/ExecReqThread.java
View File

@ -51,7 +51,7 @@ import org.apache.http.util.EntityUtils;
public class ExecReqThread extends GetConnThread {
protected final ClientConnectionManager conn_manager;
protected final RequestSpec request_spec;
protected final RequestSpec request_spec;
protected volatile HttpResponse response;
protected volatile byte[] response_data;