From a72e883d9ce89f695984a80a15ef603d072010d3 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Wed, 19 Mar 2008 19:36:02 +0000 Subject: [PATCH] HTTPCLIENT-734: Request abort will unblock the thread waiting for a connection Contributed by Sam Berlin Reviewed by Oleg Kalnichevski git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@638979 13f79535-47bb-0310-9956-ffa450edef68 --- RELEASE_NOTES.txt | 3 + .../http/conn/ClientConnectionManager.java | 9 ++ .../http/conn/ClientConnectionRequest.java | 46 ++++++ .../client/DefaultClientRequestDirector.java | 114 ++++++++++---- .../impl/conn/SingleClientConnManager.java | 19 ++- .../apache/http/impl/conn/tsccm/Aborter.java | 62 ++++++++ .../impl/conn/tsccm/AbstractConnPool.java | 13 +- .../http/impl/conn/tsccm/ConnPoolByRoute.java | 54 ++++++- .../impl/conn/tsccm/PoolEntryRequest.java | 73 +++++++++ .../tsccm/ThreadSafeClientConnManager.java | 47 ++++-- .../http/impl/conn/tsccm/WaitingThread.java | 13 ++ .../TestDefaultClientRequestDirector.java | 142 ++++++++++++++++-- .../apache/http/impl/conn/ExecReqThread.java | 4 +- .../apache/http/impl/conn/GetConnThread.java | 31 ++-- .../http/impl/conn/TestTSCCMNoServer.java | 87 ++++++++++- 15 files changed, 643 insertions(+), 74 deletions(-) create mode 100644 module-client/src/main/java/org/apache/http/conn/ClientConnectionRequest.java create mode 100644 module-client/src/main/java/org/apache/http/impl/conn/tsccm/Aborter.java create mode 100644 module-client/src/main/java/org/apache/http/impl/conn/tsccm/PoolEntryRequest.java diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index ec42dfee8..518ba5644 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,6 +1,9 @@ Changes since 4.0 Alpha 3 ------------------- +* [HTTPCLIENT-734] Request abort will unblock the thread waiting for a connection + Contributed by Sam Berlin + * [HTTPCLIENT-759] Ensure release of connections back to the connection manager on exceptions. Contributed by Sam Berlin diff --git a/module-client/src/main/java/org/apache/http/conn/ClientConnectionManager.java b/module-client/src/main/java/org/apache/http/conn/ClientConnectionManager.java index beb2ed421..1b4a55eb2 100644 --- a/module-client/src/main/java/org/apache/http/conn/ClientConnectionManager.java +++ b/module-client/src/main/java/org/apache/http/conn/ClientConnectionManager.java @@ -117,6 +117,15 @@ public interface ClientConnectionManager { TimeUnit tunit) throws ConnectionPoolTimeoutException, InterruptedException ; + + + /** + * Returns a new {@link ClientConnectionRequest}, from which a + * {@link ManagedClientConnection} can be obtained, or the request can be + * aborted. + */ + ClientConnectionRequest newConnectionRequest() + ; /** diff --git a/module-client/src/main/java/org/apache/http/conn/ClientConnectionRequest.java b/module-client/src/main/java/org/apache/http/conn/ClientConnectionRequest.java new file mode 100644 index 000000000..c40c96a3f --- /dev/null +++ b/module-client/src/main/java/org/apache/http/conn/ClientConnectionRequest.java @@ -0,0 +1,46 @@ +package org.apache.http.conn; + +import java.util.concurrent.TimeUnit; + +import org.apache.http.conn.routing.HttpRoute; + +/** + * Encapsulates a request for a {@link ManagedClientConnection}. + */ +public interface ClientConnectionRequest { + + /** + * Obtains a connection within a given time. + * This method will block until a connection becomes available, + * the timeout expires, or the connection manager is + * {@link #shutdown shut down}. + * Timeouts are handled with millisecond precision. + * + * If {@link #abortRequest()} is called while this is blocking or + * before this began, an {@link InterruptedException} will + * be thrown. + * + * @param route where the connection should point to + * @param timeout the timeout, 0 or negative for no timeout + * @param tunit the unit for the timeout, + * may be null only if there is no timeout + * + * @return a connection that can be used to communicate + * along the given route + * + * @throws ConnectionPoolTimeoutException + * in case of a timeout + * @throws InterruptedException + * if the calling thread is interrupted while waiting + */ + ManagedClientConnection getConnection(HttpRoute route, long timeout, + TimeUnit unit) throws InterruptedException, + ConnectionPoolTimeoutException; + + /** + * Aborts the call to {@link #getConnection(HttpRoute, long, TimeUnit)}, + * causing it to throw an {@link InterruptedException}. + */ + void abortRequest(); + +} diff --git a/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java b/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java index 301c701ff..464b2f852 100644 --- a/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java +++ b/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java @@ -47,8 +47,8 @@ import org.apache.http.HttpException; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; -import org.apache.http.ProtocolVersion; import org.apache.http.ProtocolException; +import org.apache.http.ProtocolVersion; import org.apache.http.auth.AuthScheme; import org.apache.http.auth.AuthScope; import org.apache.http.auth.AuthenticationException; @@ -69,21 +69,22 @@ import org.apache.http.client.protocol.ClientContext; import org.apache.http.client.utils.URLUtils; import org.apache.http.conn.BasicManagedEntity; import org.apache.http.conn.ClientConnectionManager; -import org.apache.http.conn.ConnectionPoolTimeoutException; -import org.apache.http.conn.routing.HttpRoute; -import org.apache.http.conn.routing.HttpRoutePlanner; -import org.apache.http.conn.routing.HttpRouteDirector; -import org.apache.http.conn.routing.BasicRouteDirector; -import org.apache.http.conn.Scheme; +import org.apache.http.conn.ClientConnectionRequest; +import org.apache.http.conn.ConnectionReleaseTrigger; import org.apache.http.conn.ManagedClientConnection; +import org.apache.http.conn.Scheme; +import org.apache.http.conn.routing.BasicRouteDirector; +import org.apache.http.conn.routing.HttpRoute; +import org.apache.http.conn.routing.HttpRouteDirector; +import org.apache.http.conn.routing.HttpRoutePlanner; import org.apache.http.entity.BufferedHttpEntity; import org.apache.http.message.BasicHttpRequest; import org.apache.http.params.HttpConnectionParams; import org.apache.http.params.HttpParams; import org.apache.http.params.HttpProtocolParams; +import org.apache.http.protocol.ExecutionContext; import org.apache.http.protocol.HTTP; import org.apache.http.protocol.HttpContext; -import org.apache.http.protocol.ExecutionContext; import org.apache.http.protocol.HttpProcessor; import org.apache.http.protocol.HttpRequestExecutor; @@ -287,15 +288,20 @@ public class DefaultClientRequestDirector // request is still available in 'orig'. HttpRoute route = roureq.getRoute(); + + ReleaseTrigger releaseTrigger = new ReleaseTrigger(); + if (orig instanceof AbortableHttpRequest) { + ((AbortableHttpRequest) orig).setReleaseTrigger(releaseTrigger); + } // Allocate connection if needed if (managedConn == null) { - managedConn = allocateConnection(route, timeout); + ClientConnectionRequest connectionRequest = allocateConnection(); + releaseTrigger.setClientConnectionRequest(connectionRequest); + managedConn = connectionRequest.getConnection(route, timeout, TimeUnit.MILLISECONDS); } - if (orig instanceof AbortableHttpRequest) { - ((AbortableHttpRequest) orig).setReleaseTrigger(managedConn); - } + releaseTrigger.setConnectionReleaseTrigger(managedConn); // Reopen connection if needed if (!managedConn.isOpen()) { @@ -489,23 +495,10 @@ public class DefaultClientRequestDirector /** - * Obtains a connection for the target route. - * - * @param route the route for which to allocate a connection - * @param timeout the timeout in milliseconds, - * 0 or negative for no timeout - * - * @throws HttpException in case of a (protocol) problem - * @throws ConnectionPoolTimeoutException in case of a timeout - * @throws InterruptedException in case of an interrupt + * Obtains a connection request, from which the connection can be retrieved. */ - protected ManagedClientConnection allocateConnection(HttpRoute route, - long timeout) - throws HttpException, ConnectionPoolTimeoutException, - InterruptedException { - - return connManager.getConnection - (route, timeout, TimeUnit.MILLISECONDS); + protected ClientConnectionRequest allocateConnection() { + return connManager.newConnectionRequest(); } // allocateConnection @@ -1027,4 +1020,69 @@ public class DefaultClientRequestDirector authState.setCredentials(creds); } + /** + * A {@link ConnectionReleaseTrigger} that delegates either a + * {@link ClientConnectionRequest} or another ConnectionReleaseTrigger + * for aborting. + */ + private static class ReleaseTrigger implements ConnectionReleaseTrigger { + private boolean aborted = false; + private ClientConnectionRequest delegateRequest; + private ConnectionReleaseTrigger delegateTrigger; + + void setConnectionReleaseTrigger(ConnectionReleaseTrigger releaseTrigger) throws IOException { + synchronized(this) { + if(aborted) { + throw new IOException("already aborted!"); + } + this.delegateTrigger = releaseTrigger; + this.delegateRequest = null; + } + } + + void setClientConnectionRequest(ClientConnectionRequest connectionRequest) throws IOException { + synchronized(this) { + if(aborted) { + throw new IOException("already aborted"); + } + this.delegateRequest = connectionRequest; + this.delegateTrigger = null; + } + } + + + public void abortConnection() throws IOException { + ConnectionReleaseTrigger releaseTrigger; + ClientConnectionRequest connectionRequest; + synchronized(this) { + if(aborted) + throw new IOException("already aborted"); + aborted = true; + // capture references within lock + releaseTrigger = delegateTrigger; + connectionRequest = delegateRequest; + } + + if(connectionRequest != null) + connectionRequest.abortRequest(); + + if(releaseTrigger != null) { + releaseTrigger.abortConnection(); + } + } + + public void releaseConnection() throws IOException { + ConnectionReleaseTrigger releaseTrigger; + synchronized(this) { + releaseTrigger = delegateTrigger; // capture reference within lock + } + + if(releaseTrigger != null) + releaseTrigger.releaseConnection(); + } + } + + + + } // class DefaultClientRequestDirector diff --git a/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java b/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java index 887f23365..9aaef29c9 100644 --- a/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java +++ b/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java @@ -36,12 +36,13 @@ import java.util.concurrent.TimeUnit; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.ClientConnectionOperator; +import org.apache.http.conn.ClientConnectionRequest; import org.apache.http.conn.ManagedClientConnection; import org.apache.http.conn.OperatedClientConnection; import org.apache.http.conn.SchemeRegistry; +import org.apache.http.conn.routing.HttpRoute; import org.apache.http.params.HttpParams; @@ -194,6 +195,22 @@ public class SingleClientConnManager implements ClientConnectionManager { TimeUnit tunit) { return getConnection(route); } + + public final ClientConnectionRequest newConnectionRequest() { + + return new ClientConnectionRequest() { + + public void abortRequest() { + // Nothing to abort, since requests are immediate. + } + + public ManagedClientConnection getConnection(HttpRoute route, + long timeout, TimeUnit tunit) { + return SingleClientConnManager.this.getConnection(route); + } + + }; + } /** diff --git a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/Aborter.java b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/Aborter.java new file mode 100644 index 000000000..1e17a69e2 --- /dev/null +++ b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/Aborter.java @@ -0,0 +1,62 @@ +/* + * $HeadURL:$ + * $Revision:$ + * $Date:$ + * + * ==================================================================== + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ + +package org.apache.http.impl.conn.tsccm; + +/** A simple class that can interrupt a {@link WaitingThread}. */ +public class Aborter { + + private WaitingThread waitingThread; + private boolean aborted; + + /** + * If a waiting thread has been set, interrupts it. + */ + public void abort() { + aborted = true; + + if (waitingThread != null) + waitingThread.interrupt(); + + } + + /** + * Sets the waiting thread. If this has already been aborted, + * the waiting thread is immediately interrupted. + * + * @param waitingThread The thread to interrupt when aborting. + */ + public void setWaitingThread(WaitingThread waitingThread) { + this.waitingThread = waitingThread; + if (aborted) + waitingThread.interrupt(); + } + +} diff --git a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java index 0550d302b..8254154dc 100644 --- a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java +++ b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java @@ -208,11 +208,18 @@ public abstract class AbstractConnPool implements RefQueueHandler { * @throws InterruptedException * if the calling thread was interrupted */ - public abstract + public final BasicPoolEntry getEntry(HttpRoute route, long timeout, TimeUnit tunit, ClientConnectionOperator operator) - throws ConnectionPoolTimeoutException, InterruptedException - ; + throws ConnectionPoolTimeoutException, InterruptedException { + return newPoolEntryRequest().getPoolEntry(route, timeout, tunit, operator); + } + + /** + * Returns a new {@link PoolEntryRequest}, from which a {@link BasicPoolEntry} + * can be obtained, or the request can be aborted. + */ + public abstract PoolEntryRequest newPoolEntryRequest(); /** diff --git a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java index 63ffa8b25..923b8aaf1 100644 --- a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java +++ b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java @@ -205,13 +205,56 @@ public class ConnPoolByRoute extends AbstractConnPool { poolLock.unlock(); } } - - - // non-javadoc, see base class AbstractConnPool + @Override - public BasicPoolEntry getEntry(HttpRoute route, + public PoolEntryRequest newPoolEntryRequest() { + + final Aborter aborter = new Aborter(); + + return new PoolEntryRequest() { + + public void abortRequest() { + try { + poolLock.lock(); + aborter.abort(); + } finally { + poolLock.unlock(); + } + } + + public BasicPoolEntry getPoolEntry(HttpRoute route, long timeout, + TimeUnit tunit, ClientConnectionOperator operator) + throws InterruptedException, ConnectionPoolTimeoutException { + return getEntryBlocking(route, timeout, tunit, operator, aborter); + } + + }; + } + + /** + * Obtains a pool entry with a connection within the given timeout. + * If a {@link WaitingThread} is used to block, {@link Aborter#setWaitingThread(WaitingThread)} + * must be called before blocking, to allow the thread to be interrupted. + * + * @param route the route for which to get the connection + * @param timeout the timeout, 0 or negative for no timeout + * @param tunit the unit for the timeout, + * may be null only if there is no timeout + * @param operator the connection operator, in case + * a connection has to be created + * @param aborter an object which can abort a {@link WaitingThread}. + * + * @return pool entry holding a connection for the route + * + * @throws ConnectionPoolTimeoutException + * if the timeout expired + * @throws InterruptedException + * if the calling thread was interrupted + */ + protected BasicPoolEntry getEntryBlocking(HttpRoute route, long timeout, TimeUnit tunit, - ClientConnectionOperator operator) + ClientConnectionOperator operator, + Aborter aborter) throws ConnectionPoolTimeoutException, InterruptedException { int maxHostConnections = HttpConnectionManagerParams @@ -269,6 +312,7 @@ public class ConnPoolByRoute extends AbstractConnPool { if (waitingThread == null) { waitingThread = newWaitingThread(poolLock.newCondition(), rospl); + aborter.setWaitingThread(waitingThread); } boolean success = false; diff --git a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/PoolEntryRequest.java b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/PoolEntryRequest.java new file mode 100644 index 000000000..8a58565db --- /dev/null +++ b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/PoolEntryRequest.java @@ -0,0 +1,73 @@ +/* + * $HeadURL:$ + * $Revision:$ + * $Date:$ + * + * ==================================================================== + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ + +package org.apache.http.impl.conn.tsccm; + +import java.util.concurrent.TimeUnit; + +import org.apache.http.conn.ClientConnectionOperator; +import org.apache.http.conn.ConnectionPoolTimeoutException; +import org.apache.http.conn.routing.HttpRoute; + +/** + * Encapsulates a request for a {@link BasicPoolEntry}. + */ +public interface PoolEntryRequest { + + /** + * Obtains a pool entry with a connection within the given timeout. + * If {@link #abortRequest()} is called before this completes, + * an {@link InterruptedException} is thrown. + * + * @param route the route for which to get the connection + * @param timeout the timeout, 0 or negative for no timeout + * @param tunit the unit for the timeout, + * may be null only if there is no timeout + * @param operator the connection operator, in case + * a connection has to be created + * + * @return pool entry holding a connection for the route + * + * @throws ConnectionPoolTimeoutException + * if the timeout expired + * @throws InterruptedException + * if the calling thread was interrupted + */ + BasicPoolEntry getPoolEntry(HttpRoute route, long timeout, TimeUnit unit, + ClientConnectionOperator operator) throws InterruptedException, + ConnectionPoolTimeoutException; + + /** + * Aborts the active or next call to + * {@link #getPoolEntry(HttpRoute, long, TimeUnit, ClientConnectionOperator)}. + */ + void abortRequest(); + +} diff --git a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java index 8fa28c1e4..36a36e683 100644 --- a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java +++ b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java @@ -38,6 +38,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.ClientConnectionOperator; +import org.apache.http.conn.ClientConnectionRequest; import org.apache.http.conn.ConnectionPoolTimeoutException; import org.apache.http.conn.ManagedClientConnection; import org.apache.http.conn.OperatedClientConnection; @@ -147,7 +148,7 @@ public class ThreadSafeClientConnManager // non-javadoc, see interface ClientConnectionManager - public ManagedClientConnection getConnection(HttpRoute route) + public final ManagedClientConnection getConnection(HttpRoute route) throws InterruptedException { while (true) { @@ -166,24 +167,44 @@ public class ThreadSafeClientConnManager // non-javadoc, see interface ClientConnectionManager - public ManagedClientConnection getConnection(HttpRoute route, + public final ManagedClientConnection getConnection(HttpRoute route, long timeout, TimeUnit tunit) throws ConnectionPoolTimeoutException, InterruptedException { + + return newConnectionRequest().getConnection(route, timeout, tunit); + } + + + public ClientConnectionRequest newConnectionRequest() { + + final PoolEntryRequest poolRequest = connectionPool.newPoolEntryRequest(); + + return new ClientConnectionRequest() { + + public void abortRequest() { + poolRequest.abortRequest(); + } + + public ManagedClientConnection getConnection(HttpRoute route, + long timeout, TimeUnit tunit) throws InterruptedException, + ConnectionPoolTimeoutException { + if (route == null) { + throw new IllegalArgumentException("Route may not be null."); + } - if (route == null) { - throw new IllegalArgumentException("Route may not be null."); - } + if (LOG.isDebugEnabled()) { + LOG.debug("ThreadSafeClientConnManager.getConnection: " + + route + ", timeout = " + timeout); + } - if (LOG.isDebugEnabled()) { - LOG.debug("ThreadSafeClientConnManager.getConnection: " - + route + ", timeout = " + timeout); - } + final BasicPoolEntry entry = poolRequest.getPoolEntry(route, timeout, tunit, connOperator); - final BasicPoolEntry entry = - connectionPool.getEntry(route, timeout, tunit, connOperator); - - return new BasicPooledConnAdapter(this, entry); + return new BasicPooledConnAdapter(ThreadSafeClientConnManager.this, entry); + } + + }; + } diff --git a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java index 8c5417401..fafc18d9c 100644 --- a/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java +++ b/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java @@ -58,6 +58,9 @@ public class WaitingThread { /** The thread that is waiting for an entry. */ private Thread waiter; + + /** True if this was interrupted. */ + private boolean aborted; /** @@ -143,6 +146,9 @@ public class WaitingThread { "\nwaiter: " + this.waiter); } + if (aborted) + throw new InterruptedException("interrupted already"); + this.waiter = Thread.currentThread(); boolean success = false; @@ -179,6 +185,13 @@ public class WaitingThread { // It probably isn't, but just in case: wake all, not just one. this.cond.signalAll(); } + + public void interrupt() { + aborted = true; + + if (this.waiter != null) + this.waiter.interrupt(); + } } // class WaitingThread diff --git a/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java b/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java index 8c0fa4fef..3b9b68ba1 100644 --- a/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java +++ b/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java @@ -1,7 +1,7 @@ /* - * $HeadURL:$ - * $Revision:$ - * $Date:$ + * $HeadURL$ + * $Revision$ + * $Date$ * ==================================================================== * * Licensed to the Apache Software Foundation (ASF) under one or more @@ -30,7 +30,9 @@ package org.apache.http.impl.client; import java.io.IOException; import java.net.ConnectException; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import junit.framework.Test; import junit.framework.TestSuite; @@ -40,6 +42,7 @@ import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; 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.ManagedClientConnection; import org.apache.http.conn.PlainSocketFactory; @@ -73,6 +76,43 @@ public class TestDefaultClientRequestDirector extends ServerTestBase { return new TestSuite(TestDefaultClientRequestDirector.class); } + /** + * Tests that if abort is called on an {@link AbortableHttpRequest} while + * {@link DefaultClientRequestDirector} is allocating a connection, that the + * connection is properly aborted. + */ + public void testAbortInAllocate() throws Exception { + CountDownLatch connLatch = new CountDownLatch(1); + CountDownLatch awaitLatch = new CountDownLatch(1); + final ConMan conMan = new ConMan(connLatch, awaitLatch); + final AtomicReference throwableRef = new AtomicReference(); + final CountDownLatch getLatch = new CountDownLatch(1); + final DefaultHttpClient client = new DefaultHttpClient(conMan, new BasicHttpParams()); + final HttpContext context = client.getDefaultContext(); + final HttpGet httpget = new HttpGet("http://www.example.com/a"); + + new Thread(new Runnable() { + public void run() { + try { + client.execute(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 InterruptedException, was: " + throwableRef.get(), + throwableRef.get() instanceof InterruptedException); + } + + /** * Tests that if a socket fails to connect, the allocated connection is * properly released back to the connection manager. @@ -160,16 +200,29 @@ public class TestDefaultClientRequestDirector extends ServerTestBase { } public ManagedClientConnection getConnection(HttpRoute route, - long timeout, TimeUnit tunit) - throws ConnectionPoolTimeoutException, InterruptedException { - allocatedConnection = new ClientConnAdapterMockup() { - @Override - public void open(HttpRoute route, HttpContext context, - HttpParams params) throws IOException { - throw new ConnectException(); + long timeout, TimeUnit tunit) { + throw new UnsupportedOperationException("just a mockup"); + } + + public ClientConnectionRequest newConnectionRequest() { + return new ClientConnectionRequest() { + public void abortRequest() { + throw new UnsupportedOperationException("just a mockup"); + } + public ManagedClientConnection getConnection(HttpRoute route, + long timeout, TimeUnit unit) + throws InterruptedException, + ConnectionPoolTimeoutException { + allocatedConnection = new ClientConnAdapterMockup() { + @Override + public void open(HttpRoute route, HttpContext context, + HttpParams params) throws IOException { + throw new ConnectException(); + } + }; + return allocatedConnection; } }; - return allocatedConnection; } public HttpParams getParams() { @@ -191,4 +244,71 @@ public class TestDefaultClientRequestDirector extends ServerTestBase { } } + private static class ConMan implements ClientConnectionManager { + private final CountDownLatch connLatch; + private final CountDownLatch awaitLatch; + + public ConMan(CountDownLatch connLatch, CountDownLatch awaitLatch) { + this.connLatch = connLatch; + this.awaitLatch = awaitLatch; + } + + public void closeIdleConnections(long idletime, TimeUnit tunit) { + throw new UnsupportedOperationException("just a mockup"); + } + + public ManagedClientConnection getConnection(HttpRoute route) + throws InterruptedException { + throw new UnsupportedOperationException("just a mockup"); + } + + public ManagedClientConnection getConnection(HttpRoute route, + long timeout, TimeUnit tunit) { + throw new UnsupportedOperationException("just a mockup"); + } + + public ClientConnectionRequest newConnectionRequest() { + final Thread currentThread = Thread.currentThread(); + return new ClientConnectionRequest() { + public void abortRequest() { + currentThread.interrupt(); + } + + public ManagedClientConnection getConnection(HttpRoute route, + 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(); + } + }; + } + + public HttpParams getParams() { + throw new UnsupportedOperationException("just a mockup"); + } + + public SchemeRegistry getSchemeRegistry() { + SchemeRegistry registry = new SchemeRegistry(); + registry.register(new Scheme("http", new SocketFactoryMockup(null), 80)); + return registry; + } + + public void releaseConnection(ManagedClientConnection conn) { + throw new UnsupportedOperationException("just a mockup"); + } + + public void shutdown() { + throw new UnsupportedOperationException("just a mockup"); + } + } + } diff --git a/module-client/src/test/java/org/apache/http/impl/conn/ExecReqThread.java b/module-client/src/test/java/org/apache/http/impl/conn/ExecReqThread.java index 38fd6b0a0..a31c46f95 100644 --- a/module-client/src/test/java/org/apache/http/impl/conn/ExecReqThread.java +++ b/module-client/src/test/java/org/apache/http/impl/conn/ExecReqThread.java @@ -50,7 +50,8 @@ import org.apache.http.util.EntityUtils; */ public class ExecReqThread extends GetConnThread { - protected RequestSpec request_spec; + protected final ClientConnectionManager conn_manager; + protected final RequestSpec request_spec; protected volatile HttpResponse response; protected volatile byte[] response_data; @@ -71,6 +72,7 @@ public class ExecReqThread extends GetConnThread { HttpRoute route, long timeout, RequestSpec reqspec) { super(mgr, route, timeout); + this.conn_manager = mgr; request_spec = reqspec; } diff --git a/module-client/src/test/java/org/apache/http/impl/conn/GetConnThread.java b/module-client/src/test/java/org/apache/http/impl/conn/GetConnThread.java index 9e98f4f2f..989141838 100644 --- a/module-client/src/test/java/org/apache/http/impl/conn/GetConnThread.java +++ b/module-client/src/test/java/org/apache/http/impl/conn/GetConnThread.java @@ -33,6 +33,7 @@ package org.apache.http.impl.conn; import java.util.concurrent.TimeUnit; import org.apache.http.conn.ClientConnectionManager; +import org.apache.http.conn.ClientConnectionRequest; import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.ManagedClientConnection; @@ -44,26 +45,36 @@ import org.apache.http.conn.ManagedClientConnection; */ public class GetConnThread extends Thread { - protected ClientConnectionManager conn_manager; - protected HttpRoute conn_route; - protected long conn_timeout; + protected final HttpRoute conn_route; + protected final long conn_timeout; + protected final ClientConnectionRequest conn_request; protected volatile ManagedClientConnection connection; protected volatile Throwable exception; /** - * Creates a new thread. + * Creates a new thread for requesting a connection from the given manager. + * * When this thread is started, it will try to obtain a connection. * The timeout is in milliseconds. */ public GetConnThread(ClientConnectionManager mgr, HttpRoute route, long timeout) { - - conn_manager = mgr; - conn_route = route; - conn_timeout = timeout; + this(mgr.newConnectionRequest(), route, timeout); + } + + /** + * Creates a new for requesting a connection from the given request object. + * + * When this thread is started, it will try to obtain a connection. + * The timeout is in milliseconds. + */ + public GetConnThread(ClientConnectionRequest connectionRequest, + HttpRoute route, long timeout) { + conn_route = route; + conn_timeout = timeout; + conn_request = connectionRequest; } - /** * This method is executed when the thread is started. @@ -71,7 +82,7 @@ public class GetConnThread extends Thread { @Override public void run() { try { - connection = conn_manager.getConnection + connection = conn_request.getConnection (conn_route, conn_timeout, TimeUnit.MILLISECONDS); } catch (Throwable dart) { exception = dart; diff --git a/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMNoServer.java b/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMNoServer.java index bb9bbe0e0..ca187b0ef 100644 --- a/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMNoServer.java +++ b/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMNoServer.java @@ -38,18 +38,19 @@ import junit.framework.TestSuite; import org.apache.http.HttpHost; import org.apache.http.HttpVersion; +import org.apache.http.conn.ClientConnectionRequest; import org.apache.http.conn.ConnectionPoolTimeoutException; -import org.apache.http.conn.routing.HttpRoute; 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.SocketFactory; import org.apache.http.conn.params.HttpConnectionManagerParams; +import org.apache.http.conn.routing.HttpRoute; +import org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager; import org.apache.http.params.BasicHttpParams; import org.apache.http.params.HttpParams; import org.apache.http.params.HttpProtocolParams; -import org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager; /** @@ -542,6 +543,88 @@ public class TestTSCCMNoServer extends TestCase { mgr.shutdown(); } + + public void testAbortAfterRequestStarts() throws Exception { + HttpParams params = createDefaultParams(); + HttpConnectionManagerParams.setMaxTotalConnections(params, 1); + ThreadSafeClientConnManager mgr = createTSCCM(params, null); + + HttpHost target = new HttpHost("www.test.invalid", 80, "http"); + HttpRoute route = new HttpRoute(target, null, false); + + // get the only connection, then start an extra thread + ManagedClientConnection conn = mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS); + ClientConnectionRequest request = mgr.newConnectionRequest(); + GetConnThread gct = new GetConnThread(request, route, 0L); // no timeout + gct.start(); + Thread.sleep(100); // give extra thread time to block + + request.abortRequest(); + + gct.join(10000); + assertNotNull("thread should have gotten an exception", + gct.getException()); + assertSame("thread got wrong exception", + InterruptedException.class, + gct.getException().getClass()); + + // make sure the manager is still working + try { + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); + fail("should have gotten a timeout"); + } catch (ConnectionPoolTimeoutException e) { + // expected + } + + mgr.releaseConnection(conn); + // this time: no exception + conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); + assertNotNull("should have gotten a connection", conn); + + mgr.shutdown(); + } + + public void testAbortBeforeRequestStarts() throws Exception { + HttpParams params = createDefaultParams(); + HttpConnectionManagerParams.setMaxTotalConnections(params, 1); + + ThreadSafeClientConnManager mgr = createTSCCM(params, null); + + HttpHost target = new HttpHost("www.test.invalid", 80, "http"); + HttpRoute route = new HttpRoute(target, null, false); + + + // get the only connection, then start an extra thread + ManagedClientConnection conn = mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS); + ClientConnectionRequest request = mgr.newConnectionRequest(); + request.abortRequest(); + + GetConnThread gct = new GetConnThread(request, route, 0L); // no timeout + gct.start(); + Thread.sleep(100); // give extra thread time to block + + gct.join(10000); + assertNotNull("thread should have gotten an exception", + gct.getException()); + assertSame("thread got wrong exception", + InterruptedException.class, + gct.getException().getClass()); + + // make sure the manager is still working + try { + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); + fail("should have gotten a timeout"); + } catch (ConnectionPoolTimeoutException e) { + // expected + } + + mgr.releaseConnection(conn); + // this time: no exception + conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); + assertNotNull("should have gotten a connection", conn); + + mgr.shutdown(); + } } // class TestTSCCMNoServer