Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/conn/GetConnThread.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/conn/GetConnThread.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/conn/GetConnThread.java (working copy) @@ -32,6 +32,7 @@ import java.util.concurrent.TimeUnit; +import org.apache.http.client.methods.AbortableHttpRequest; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.ManagedClientConnection; @@ -47,6 +48,7 @@ protected ClientConnectionManager conn_manager; protected HttpRoute conn_route; protected long conn_timeout; + protected AbortableHttpRequest conn_aborter; protected volatile ManagedClientConnection connection; protected volatile Throwable exception; @@ -58,10 +60,16 @@ */ public GetConnThread(ClientConnectionManager mgr, HttpRoute route, long timeout) { - + this(mgr, route, timeout, null); + } + + public GetConnThread(ClientConnectionManager mgr, + HttpRoute route, long timeout, + AbortableHttpRequest aborter) { conn_manager = mgr; conn_route = route; conn_timeout = timeout; + conn_aborter = aborter; } @@ -72,7 +80,7 @@ public void run() { try { connection = conn_manager.getConnection - (conn_route, conn_timeout, TimeUnit.MILLISECONDS); + (conn_route, conn_timeout, TimeUnit.MILLISECONDS, conn_aborter); } catch (Throwable dart) { exception = dart; } Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java (working copy) @@ -212,7 +212,7 @@ // check that there is no auto-release by default try { // this should fail quickly, connection has not been released - mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -296,7 +296,7 @@ // first check that we can't get another connection try { // this should fail quickly, connection has not been released - mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -315,7 +315,7 @@ Thread.sleep(1000); assertNull("connection not garbage collected", wref.get()); - conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); + conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS, null); assertFalse("GCed connection not closed", conn.isOpen()); mgr.shutdown(); Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMNoServer.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMNoServer.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMNoServer.java (working copy) @@ -30,6 +30,7 @@ package org.apache.http.impl.conn; +import java.io.IOException; import java.util.concurrent.TimeUnit; import junit.framework.Test; @@ -38,18 +39,20 @@ import org.apache.http.HttpHost; import org.apache.http.HttpVersion; +import org.apache.http.client.methods.AbortableHttpRequest; import org.apache.http.conn.ConnectionPoolTimeoutException; -import org.apache.http.conn.routing.HttpRoute; +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.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; /** @@ -206,7 +209,7 @@ try { // this should fail quickly, connection has not been released - mgr.getConnection(route2, 100L, TimeUnit.MILLISECONDS); + mgr.getConnection(route2, 100L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -218,7 +221,7 @@ // there should be a connection available now try { - conn2 = mgr.getConnection(route2, 100L, TimeUnit.MILLISECONDS); + conn2 = mgr.getConnection(route2, 100L, TimeUnit.MILLISECONDS, null); } catch (ConnectionPoolTimeoutException cptx) { cptx.printStackTrace(); fail("connection should have been available: " + cptx); @@ -247,38 +250,38 @@ // route 3, limit 3 ManagedClientConnection conn1 = - mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS, null); assertNotNull(conn1); ManagedClientConnection conn2 = - mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS, null); assertNotNull(conn2); ManagedClientConnection conn3 = - mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS, null); assertNotNull(conn3); try { // should fail quickly, connection has not been released - mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } // route 2, limit 2 - conn1 = mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); - conn2 = mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); + conn1 = mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS, null); + conn2 = mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS, null); try { // should fail quickly, connection has not been released - mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } // route 1, should use default limit of 1 - conn1 = mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); + conn1 = mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS, null); try { // should fail quickly, connection has not been released - mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -320,11 +323,11 @@ // the first three allocations should pass ManagedClientConnection conn1 = - mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS, null); ManagedClientConnection conn2 = - mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS, null); ManagedClientConnection conn3 = - mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS, null); assertNotNull(conn1); assertNotNull(conn2); assertNotNull(conn3); @@ -332,19 +335,19 @@ // obtaining another connection for either of the three should fail // this is somehow redundant with testMaxConnPerHost try { - mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } try { - mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } try { - mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -354,16 +357,16 @@ mgr.releaseConnection(conn2); conn2 = null; try { - mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } // this one succeeds - conn2 = mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); + conn2 = mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS, null); assertNotNull(conn2); try { - mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS, null); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -421,7 +424,7 @@ // on shutdown, the extra thread should get an exception ManagedClientConnection conn = - mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS); + mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS, null); GetConnThread gct = new GetConnThread(mgr, route, 0L); // no timeout gct.start(); Thread.sleep(100); // give extra thread time to block @@ -445,7 +448,7 @@ // the manager is down, we should not be able to get a connection try { - conn = mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS); + conn = mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS, null); fail("shut-down manager does not raise exception"); } catch (IllegalStateException isx) { // expected @@ -466,7 +469,7 @@ // get the only connection, then start an extra thread ManagedClientConnection conn = - mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS); + mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS, null); GetConnThread gct = new GetConnThread(mgr, route, 0L); // no timeout gct.start(); Thread.sleep(100); // give extra thread time to block @@ -485,7 +488,7 @@ // make sure the manager is still working try { - mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS, null); fail("should have gotten a timeout"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -493,7 +496,7 @@ mgr.releaseConnection(conn); // this time: no exception - conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); + conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS, null); assertNotNull("should have gotten a connection", conn); mgr.shutdown(); @@ -516,7 +519,7 @@ // get the only connection, then start two extra threads ManagedClientConnection conn = - mgr.getConnection(route1, 1L, TimeUnit.MILLISECONDS); + mgr.getConnection(route1, 1L, TimeUnit.MILLISECONDS, null); GetConnThread gct1 = new GetConnThread(mgr, route1, 1000L); GetConnThread gct2 = new GetConnThread(mgr, route2, 1000L); @@ -542,6 +545,121 @@ mgr.shutdown(); } + + public void testAbortWaitingRequest() 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); + + Aborter abort = new Aborter(); + + // get the only connection, then start an extra thread + ManagedClientConnection conn = + mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS, null); + GetConnThread gct = new GetConnThread(mgr, route, 0L, abort); // no timeout + gct.start(); + Thread.sleep(100); // give extra thread time to block + + abort.abort(); + + gct.join(10000); + assertNotNull("thread should have gotten an exception", + gct.getException()); + assertSame("thread got wrong exception", + InterruptedException.class, + gct.getException().getClass()); + assertNotNull("trigger should have been set", abort.trigger); + + // make sure the manager is still working + try { + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS, null); + fail("should have gotten a timeout"); + } catch (ConnectionPoolTimeoutException e) { + // expected + } + + mgr.releaseConnection(conn); + // this time: no exception + conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS, null); + assertNotNull("should have gotten a connection", conn); + + mgr.shutdown(); + } + + // Note: This test makes sure that if the AbortableHttpRequest throws an + // IOException on setReleaseTrigger, this aborts correctly. + // It does *not* test that behavior is right if the release trigger + // is set and abort is called before WaitingThread.await() enters. + // There's no good way to test that... + public void testAbortBeforeWaitingRequestQueues() 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); + + Aborter abort = new Aborter(); + + // get the only connection, then start an extra thread + ManagedClientConnection conn = + mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS, null); + + abort.abort(); + + GetConnThread gct = new GetConnThread(mgr, route, 0L, abort); // 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()); + assertNull("trigger should not have been set", abort.trigger); + + // make sure the manager is still working + try { + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS, null); + fail("should have gotten a timeout"); + } catch (ConnectionPoolTimeoutException e) { + // expected + } + + mgr.releaseConnection(conn); + // this time: no exception + conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS, null); + assertNotNull("should have gotten a connection", conn); + + mgr.shutdown(); + } + + private static class Aborter implements AbortableHttpRequest { + private volatile ConnectionReleaseTrigger trigger; + private volatile boolean aborted; + + public void abort() { + aborted = true; + if(trigger != null) { + try { + trigger.abortConnection(); + } catch(IOException ignored) {} + } + } + + public void setReleaseTrigger(ConnectionReleaseTrigger releaseTrigger) + throws IOException { + if(aborted) + throw new IOException("already aborted"); + this.trigger = releaseTrigger; + } + } + + } // class TestTSCCMNoServer Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/test/java/org/apache/http/impl/client/TestDefaultClientRequestDirector.java (working copy) @@ -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 @@ 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; @@ -38,9 +40,11 @@ import org.apache.http.HttpException; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; +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.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; @@ -74,6 +78,43 @@ } /** + * 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,7 +201,7 @@ } public ManagedClientConnection getConnection(HttpRoute route, - long timeout, TimeUnit tunit) + long timeout, TimeUnit tunit, AbortableHttpRequest abort) throws ConnectionPoolTimeoutException, InterruptedException { allocatedConnection = new ClientConnAdapterMockup() { @Override @@ -191,4 +232,70 @@ } } + 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, AbortableHttpRequest abort) + throws ConnectionPoolTimeoutException, InterruptedException { + final Thread currentThread = Thread.currentThread(); + try { + abort.setReleaseTrigger(new ConnectionReleaseTrigger() { + public void abortConnection() throws IOException { + currentThread.interrupt(); + } + public void releaseConnection() throws IOException { + throw new UnsupportedOperationException("just a mockup"); + } + }); + } catch(IOException iox) { + throw new RuntimeException("should have aborted yet", iox); + } + + 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"); + } + } + } Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java (working copy) @@ -41,6 +41,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.http.client.methods.AbortableHttpRequest; import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.ClientConnectionOperator; @@ -211,6 +212,7 @@ @Override public BasicPoolEntry getEntry(HttpRoute route, long timeout, TimeUnit tunit, + AbortableHttpRequest abort, ClientConnectionOperator operator) throws ConnectionPoolTimeoutException, InterruptedException { @@ -269,6 +271,8 @@ if (waitingThread == null) { waitingThread = newWaitingThread(poolLock.newCondition(), rospl); + if(abort != null) + waitingThread.setReleaseTriggerOn(abort, poolLock); } boolean success = false; Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/WaitingThread.java (working copy) @@ -31,10 +31,15 @@ package org.apache.http.impl.conn.tsccm; +import java.io.IOException; import java.util.Date; import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import org.apache.http.client.methods.AbortableHttpRequest; +import org.apache.http.conn.ConnectionReleaseTrigger; + /** * Represents a thread waiting for a connection. * This class implements throwaway objects. It is instantiated whenever @@ -58,6 +63,9 @@ /** The thread that is waiting for an entry. */ private Thread waiter; + + /** True if the {@link ConnectionReleaseTrigger} aborted this wait. */ + private boolean aborted; /** @@ -142,6 +150,9 @@ "\ncaller: " + Thread.currentThread() + "\nwaiter: " + this.waiter); } + + if(aborted) + throw new InterruptedException("Pre-aborted"); this.waiter = Thread.currentThread(); @@ -156,6 +167,7 @@ } finally { this.waiter = null; } + return success; } // await @@ -181,4 +193,43 @@ } + /** + * Sets a release trigger on the {@link AbortableHttpRequest}, + * allowing {@link WaitingThread#await(Date)} to be aborted. + * If the {@link AbortableHttpRequest} was already aborted, + * this throws an {@link InterruptedException}. + * + * @param abort the request the release trigger should be set on. + * @param lock the lock this must hold + * + * @throws InterruptedException if the request was alredy aborted + */ + public void setReleaseTriggerOn(AbortableHttpRequest abort, final Lock lock) throws InterruptedException { + try { + abort.setReleaseTrigger(new ConnectionReleaseTrigger() { + public void abortConnection() throws IOException { + // By holding the lock, we guarantee that either + // await hasn't been entered yet or completed, + // or that it is actively waiting & will be interrupted. + try { + lock.lock(); + aborted = true; // to support a not-yet-active await + if(waiter != null) + waiter.interrupt(); + } finally { + lock.unlock(); + } + } + + public void releaseConnection() throws IOException { + // does nothing -- the connection cannot be released, + // because it doesn't exist yet. + } + }); + } catch(IOException iox) { + throw new InterruptedException("Request already aborted"); + } + } + + } // class WaitingThread Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java (working copy) @@ -43,6 +43,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.http.client.methods.AbortableHttpRequest; import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.ClientConnectionOperator; import org.apache.http.conn.ClientConnectionManager; @@ -193,14 +194,20 @@ /** * Obtains a pool entry with a connection within the given timeout. + * If this method is blocking and a non-null {@link AbortableHttpRequest} + * is provided, calling {@link AbortableHttpRequest#abort()} from another + * thread after this method is called will result in this method + * immediately throwing an {@link InterruptedException}. * * @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 abort an AbortableHttpRequest that a + * ConnectionReleaseTrigger will be set on. + * may be if no release trigger should be set. * @param operator the connection operator, in case * a connection has to be created - * * @return pool entry holding a connection for the route * * @throws ConnectionPoolTimeoutException @@ -210,7 +217,7 @@ */ public abstract BasicPoolEntry getEntry(HttpRoute route, long timeout, TimeUnit tunit, - ClientConnectionOperator operator) + AbortableHttpRequest abort, ClientConnectionOperator operator) throws ConnectionPoolTimeoutException, InterruptedException ; Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java (working copy) @@ -35,6 +35,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.http.client.methods.AbortableHttpRequest; import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.ClientConnectionOperator; @@ -152,7 +153,7 @@ while (true) { try { - return getConnection(route, 0, null); + return getConnection(route, 0, null, null); } catch (ConnectionPoolTimeoutException e) { // We'll go ahead and log this, but it should never happen. // These exceptions are only thrown when the timeout occurs @@ -168,7 +169,8 @@ // non-javadoc, see interface ClientConnectionManager public ManagedClientConnection getConnection(HttpRoute route, long timeout, - TimeUnit tunit) + TimeUnit tunit, + AbortableHttpRequest abort) throws ConnectionPoolTimeoutException, InterruptedException { if (route == null) { @@ -181,7 +183,7 @@ } final BasicPoolEntry entry = - connectionPool.getEntry(route, timeout, tunit, connOperator); + connectionPool.getEntry(route, timeout, tunit, abort, connOperator); return new BasicPooledConnAdapter(this, entry); } Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java (working copy) @@ -36,6 +36,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.http.client.methods.AbortableHttpRequest; import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.ClientConnectionOperator; @@ -185,13 +186,13 @@ * @param route where the connection should point to * @param timeout ignored * @param tunit ignored - * * @return a connection that can be used to communicate * along the given route */ public final ManagedClientConnection getConnection(HttpRoute route, long timeout, - TimeUnit tunit) { + TimeUnit tunit, + AbortableHttpRequest abort) { return getConnection(route); } Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/impl/client/DefaultClientRequestDirector.java (working copy) @@ -47,8 +47,8 @@ import org.apache.http.HttpHost; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; +import org.apache.http.ProtocolException; import org.apache.http.ProtocolVersion; -import org.apache.http.ProtocolException; import org.apache.http.auth.AuthScheme; import org.apache.http.auth.AuthScope; import org.apache.http.auth.AuthenticationException; @@ -70,20 +70,21 @@ import org.apache.http.conn.BasicManagedEntity; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.ConnectionPoolTimeoutException; +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.conn.routing.HttpRouteDirector; -import org.apache.http.conn.routing.BasicRouteDirector; -import org.apache.http.conn.Scheme; -import org.apache.http.conn.ManagedClientConnection; 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,18 @@ // 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); + managedConn = allocateConnection(route, timeout, releaseTrigger); } - if (orig instanceof AbortableHttpRequest) { - ((AbortableHttpRequest) orig).setReleaseTrigger(managedConn); - } + releaseTrigger.setReleaseTrigger(managedConn); // Reopen connection if needed if (!managedConn.isOpen()) { @@ -494,18 +498,21 @@ * @param route the route for which to allocate a connection * @param timeout the timeout in milliseconds, * 0 or negative for no timeout - * + * @param abort an AbortableHttpRequest that a + * ConnectionReleaseTrigger will be set on. + * may be if no release trigger should be set. * @throws HttpException in case of a (protocol) problem * @throws ConnectionPoolTimeoutException in case of a timeout * @throws InterruptedException in case of an interrupt */ protected ManagedClientConnection allocateConnection(HttpRoute route, - long timeout) + long timeout, + AbortableHttpRequest abort) throws HttpException, ConnectionPoolTimeoutException, InterruptedException { return connManager.getConnection - (route, timeout, TimeUnit.MILLISECONDS); + (route, timeout, TimeUnit.MILLISECONDS, abort); } // allocateConnection @@ -1027,4 +1034,53 @@ authState.setCredentials(creds); } + /** + * A {@link ConnectionReleaseTrigger} that delegates to another + * trigger. This also implements {@link AbortableHttpRequest} to support + * changing the trigger. + * + * This delegating class is used because it is possible that + */ + private static class ReleaseTrigger implements ConnectionReleaseTrigger, AbortableHttpRequest { + private boolean aborted = false; + private ConnectionReleaseTrigger delegateTrigger; + + public void abort() { + throw new IllegalStateException("this should not be called"); + } + + public void setReleaseTrigger(ConnectionReleaseTrigger releaseTrigger) throws IOException { + synchronized(this) { + if(aborted) { + throw new IOException("already aborted!"); + } + this.delegateTrigger = releaseTrigger; + } + } + + public void abortConnection() throws IOException { + ConnectionReleaseTrigger releaseTrigger; + synchronized(this) { + if(aborted) + throw new IOException("already aborted"); + aborted = true; + releaseTrigger = delegateTrigger; // capture reference within lock + } + + 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 Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/conn/ClientConnectionManager.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/conn/ClientConnectionManager.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/conn/ClientConnectionManager.java (working copy) @@ -36,6 +36,7 @@ import org.apache.http.params.HttpParams; +import org.apache.http.client.methods.AbortableHttpRequest; import org.apache.http.conn.routing.HttpRoute; @@ -97,24 +98,33 @@ * 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 + * Timeouts are handled with millisecond precision. + * If a non-null {@link AbortableHttpRequest} is used, + * then this method will set a {@link ConnectionReleaseTrigger} using + * {@link AbortableHttpRequest#setReleaseTrigger(ConnectionReleaseTrigger)}. + * The method can be interrupted by aborting that HttpRequest. * * @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 - * + * @param abort an AbortableHttpRequest that a + * ConnectionReleaseTrigger will be set on. + * may be if no release trigger should be set. * @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 + * if the calling thread is interrupted while waiting, + * or if the {@link AbortableHttpRequest} is aborted + * while waiting. */ ManagedClientConnection getConnection(HttpRoute route, long timeout, - TimeUnit tunit) + TimeUnit tunit, + AbortableHttpRequest abort) throws ConnectionPoolTimeoutException, InterruptedException ; Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/client/methods/HttpRequestBase.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/client/methods/HttpRequestBase.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/client/methods/HttpRequestBase.java (working copy) @@ -54,7 +54,9 @@ implements HttpUriRequest, AbortableHttpRequest { private URI uri; - private ConnectionReleaseTrigger releaseTrigger; + // volatile because the trigger is intended to be used from different threads + // than which it's set. + private volatile ConnectionReleaseTrigger releaseTrigger; public HttpRequestBase() { super(); @@ -88,13 +90,20 @@ this.uri = uri; } - public void setReleaseTrigger(final ConnectionReleaseTrigger releaseTrigger) { - this.releaseTrigger = releaseTrigger; + public void setReleaseTrigger(final ConnectionReleaseTrigger releaseTrigger) + throws IOException { + this.releaseTrigger = releaseTrigger; } + // TODO: It is possible to capture the abort event and throw an IOX from + // setReleaseTrigger if abort was called. This would prevent an aborted + // request from being reused on a different connection, though. + // If that behavior is OK, then this should act like 'ReleaseTrigger' + // within DefaultClientRequestDirector, and that class can disappear + // in favor of directly using this. public void abort() { if (this.releaseTrigger != null) { - try { + try { this.releaseTrigger.abortConnection(); } catch (IOException ex) { // ignore Index: C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/client/methods/AbortableHttpRequest.java =================================================================== --- C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/client/methods/AbortableHttpRequest.java (revision 638555) +++ C:/Documents and Settings/sberlin/workspace/httpcomponents-client-TRUNK/module-client/src/main/java/org/apache/http/client/methods/AbortableHttpRequest.java (working copy) @@ -31,11 +31,13 @@ package org.apache.http.client.methods; +import java.io.IOException; + import org.apache.http.conn.ConnectionReleaseTrigger; /** * Interface representing an HTTP request that can be aborted by shutting - * donw the underying HTTP connection. + * down the underlying HTTP connection. * * @author Oleg Kalnichevski * @@ -46,8 +48,17 @@ */ public interface AbortableHttpRequest { - void setReleaseTrigger(ConnectionReleaseTrigger releaseTrigger); + /** + * Sets the new release trigger for this request. + * If the request was already aborted, this may throw an {@link IOException}. + */ + void setReleaseTrigger(ConnectionReleaseTrigger releaseTrigger) throws IOException; + /** + * Aborts the request. + * Typically, this will cause another thread that is executing the request + * to either throw an {@link IOException} or an {@link InterruptedException}. + */ void abort(); }