From d1a4c2ce6de6afd3fedcb23fc1a333c3887a4eaf Mon Sep 17 00:00:00 2001 From: Roland Weber Date: Sat, 29 Dec 2007 15:39:11 +0000 Subject: [PATCH] HTTPCLIENT-725: time unit when getting a connection git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@607430 13f79535-47bb-0310-9956-ffa450edef68 --- .../http/conn/ClientConnectionManager.java | 10 ++- .../client/DefaultClientRequestDirector.java | 6 +- .../impl/conn/SingleClientConnManager.java | 5 +- .../impl/conn/tsccm/AbstractConnPool.java | 7 +- .../http/impl/conn/tsccm/ConnPoolByRoute.java | 6 +- .../tsccm/ThreadSafeClientConnManager.java | 10 +-- .../http/impl/conn/tsccm/WaitingThread.java | 1 - .../apache/http/impl/conn/GetConnThread.java | 6 +- .../http/impl/conn/TestTSCCMNoServer.java | 64 +++++++++++-------- .../http/impl/conn/TestTSCCMWithServer.java | 7 +- 10 files changed, 79 insertions(+), 43 deletions(-) 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 07f20e3ef..6a41db774 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 @@ -32,6 +32,8 @@ package org.apache.http.conn; +import java.util.concurrent.TimeUnit; + import org.apache.http.params.HttpParams; @@ -93,9 +95,12 @@ public interface ClientConnectionManager { * 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 * * @param route where the connection should point to - * @param timeout the timeout in milliseconds + * @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 @@ -106,7 +111,8 @@ public interface ClientConnectionManager { * if the calling thread is interrupted while waiting */ ManagedClientConnection getConnection(HttpRoute route, - long timeout) + long timeout, + TimeUnit tunit) throws ConnectionPoolTimeoutException, InterruptedException ; 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 b42bab681..27272c617 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 @@ -35,6 +35,7 @@ import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; import java.util.Map; +import java.util.concurrent.TimeUnit; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -501,6 +502,8 @@ 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 @@ -511,7 +514,8 @@ public class DefaultClientRequestDirector throws HttpException, ConnectionPoolTimeoutException, InterruptedException { - return connManager.getConnection(route, timeout); + return connManager.getConnection + (route, timeout, TimeUnit.MILLISECONDS); } // allocateConnection 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 193390bc0..ec0fc419f 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 @@ -32,6 +32,7 @@ package org.apache.http.impl.conn; import java.io.IOException; +import java.util.concurrent.TimeUnit; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -183,12 +184,14 @@ public class SingleClientConnManager implements ClientConnectionManager { * * @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) { + long timeout, + TimeUnit tunit) { return getConnection(route); } 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 d1decf382..ec0d87529 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 @@ -37,6 +37,7 @@ import java.lang.ref.WeakReference; import java.util.Set; import java.util.HashSet; import java.util.Iterator; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -167,7 +168,9 @@ public abstract class AbstractConnPool implements RefQueueHandler { * Obtains a pool entry with a connection within the given timeout. * * @param route the route for which to get the connection - * @param timeout the timeout, or 0 for no timeout + * @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 * @@ -179,7 +182,7 @@ public abstract class AbstractConnPool implements RefQueueHandler { * if the calling thread was interrupted */ public abstract - BasicPoolEntry getEntry(HttpRoute route, long timeout, + BasicPoolEntry getEntry(HttpRoute route, long timeout, TimeUnit tunit, ClientConnectionOperator operator) throws ConnectionPoolTimeoutException, InterruptedException ; 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 aa978ad60..337af756b 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 @@ -208,7 +208,8 @@ public class ConnPoolByRoute extends AbstractConnPool { // non-javadoc, see base class AbstractConnPool - public BasicPoolEntry getEntry(HttpRoute route, long timeout, + public BasicPoolEntry getEntry(HttpRoute route, + long timeout, TimeUnit tunit, ClientConnectionOperator operator) throws ConnectionPoolTimeoutException, InterruptedException { @@ -219,7 +220,8 @@ public class ConnPoolByRoute extends AbstractConnPool { Date deadline = null; if (timeout > 0) { - deadline = new Date(System.currentTimeMillis() + timeout); + deadline = new Date + (System.currentTimeMillis() + tunit.toMillis(timeout)); } BasicPoolEntry entry = null; 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 873282058..56614483d 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 @@ -31,6 +31,7 @@ package org.apache.http.impl.conn.tsccm; import java.io.IOException; +import java.util.concurrent.TimeUnit; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -146,13 +147,13 @@ public class ThreadSafeClientConnManager while (true) { try { - return getConnection(route, 0); + return getConnection(route, 0, 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 // and since we have no timeout, it doesn't happen. LOG.debug - ("Unexpected exception while waiting for connection", e); + ("Unexpected exception while waiting for connection.", e); //@@@ throw RuntimeException or Error to indicate the problem? } } @@ -161,7 +162,8 @@ public class ThreadSafeClientConnManager // non-javadoc, see interface ClientConnectionManager public ManagedClientConnection getConnection(HttpRoute route, - long timeout) + long timeout, + TimeUnit tunit) throws ConnectionPoolTimeoutException, InterruptedException { if (route == null) { @@ -174,7 +176,7 @@ public class ThreadSafeClientConnManager } final BasicPoolEntry entry = - connectionPool.getEntry(route, timeout, connOperator); + connectionPool.getEntry(route, timeout, tunit, connOperator); return new BasicPooledConnAdapter(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 9a4ab9629..8c5417401 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 @@ -32,7 +32,6 @@ package org.apache.http.impl.conn.tsccm; import java.util.Date; -import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; 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 b530b17d3..8e51fbf94 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 @@ -30,6 +30,8 @@ package org.apache.http.impl.conn; +import java.util.concurrent.TimeUnit; + import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.HttpRoute; import org.apache.http.conn.ManagedClientConnection; @@ -52,6 +54,7 @@ public class GetConnThread extends Thread { /** * Creates a new thread. * 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) { @@ -67,7 +70,8 @@ public class GetConnThread extends Thread { */ public void run() { try { - connection = conn_manager.getConnection(conn_route, conn_timeout); + connection = conn_manager.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 deadf5bdb..1005d539c 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 @@ -30,6 +30,8 @@ package org.apache.http.impl.conn; +import java.util.concurrent.TimeUnit; + import junit.framework.Test; import junit.framework.TestCase; import junit.framework.TestSuite; @@ -204,7 +206,7 @@ public class TestTSCCMNoServer extends TestCase { try { // this should fail quickly, connection has not been released - mgr.getConnection(route2, 100L); + mgr.getConnection(route2, 100L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -216,7 +218,7 @@ public class TestTSCCMNoServer extends TestCase { // there should be a connection available now try { - conn2 = mgr.getConnection(route2, 100L); + conn2 = mgr.getConnection(route2, 100L, TimeUnit.MILLISECONDS); } catch (ConnectionPoolTimeoutException cptx) { cptx.printStackTrace(); fail("connection should have been available: " + cptx); @@ -244,36 +246,39 @@ public class TestTSCCMNoServer extends TestCase { ThreadSafeClientConnManager mgr = createTSCCM(params, null); // route 3, limit 3 - ManagedClientConnection conn1 = mgr.getConnection(route3, 10L); + ManagedClientConnection conn1 = + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); assertNotNull(conn1); - ManagedClientConnection conn2 = mgr.getConnection(route3, 10L); + ManagedClientConnection conn2 = + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); assertNotNull(conn2); - ManagedClientConnection conn3 = mgr.getConnection(route3, 10L); + ManagedClientConnection conn3 = + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); assertNotNull(conn3); try { // should fail quickly, connection has not been released - mgr.getConnection(route3, 10L); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } // route 2, limit 2 - conn1 = mgr.getConnection(route2, 10L); - conn2 = mgr.getConnection(route2, 10L); + conn1 = mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); + conn2 = mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); try { // should fail quickly, connection has not been released - mgr.getConnection(route2, 10L); + mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } // route 1, should use default limit of 1 - conn1 = mgr.getConnection(route1, 10L); + conn1 = mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); try { // should fail quickly, connection has not been released - mgr.getConnection(route1, 10L); + mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -314,9 +319,12 @@ public class TestTSCCMNoServer extends TestCase { HttpRoute route3 = new HttpRoute(target3, null, false); // the first three allocations should pass - ManagedClientConnection conn1 = mgr.getConnection(route1, 10L); - ManagedClientConnection conn2 = mgr.getConnection(route2, 10L); - ManagedClientConnection conn3 = mgr.getConnection(route3, 10L); + ManagedClientConnection conn1 = + mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); + ManagedClientConnection conn2 = + mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); + ManagedClientConnection conn3 = + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); assertNotNull(conn1); assertNotNull(conn2); assertNotNull(conn3); @@ -324,19 +332,19 @@ public class TestTSCCMNoServer extends TestCase { // obtaining another connection for either of the three should fail // this is somehow redundant with testMaxConnPerHost try { - mgr.getConnection(route1, 10L); + mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } try { - mgr.getConnection(route2, 10L); + mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } try { - mgr.getConnection(route3, 10L); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -346,16 +354,16 @@ public class TestTSCCMNoServer extends TestCase { mgr.releaseConnection(conn2); conn2 = null; try { - mgr.getConnection(route1, 10L); + mgr.getConnection(route1, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected } // this one succeeds - conn2 = mgr.getConnection(route2, 10L); + conn2 = mgr.getConnection(route2, 10L, TimeUnit.MILLISECONDS); assertNotNull(conn2); try { - mgr.getConnection(route3, 10L); + mgr.getConnection(route3, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -411,7 +419,8 @@ public class TestTSCCMNoServer extends TestCase { // get the only connection, then start an extra thread // on shutdown, the extra thread should get an exception - ManagedClientConnection conn = mgr.getConnection(route, 1L); + ManagedClientConnection conn = + mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS); GetConnThread gct = new GetConnThread(mgr, route, 0L); // no timeout gct.start(); Thread.sleep(100); // give extra thread time to block @@ -435,7 +444,7 @@ public class TestTSCCMNoServer extends TestCase { // the manager is down, we should not be able to get a connection try { - conn = mgr.getConnection(route, 1L); + conn = mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS); fail("shut-down manager does not raise exception"); } catch (IllegalStateException isx) { // expected @@ -455,7 +464,8 @@ public class TestTSCCMNoServer extends TestCase { HttpRoute route = new HttpRoute(target, null, false); // get the only connection, then start an extra thread - ManagedClientConnection conn = mgr.getConnection(route, 1L); + ManagedClientConnection conn = + mgr.getConnection(route, 1L, TimeUnit.MILLISECONDS); GetConnThread gct = new GetConnThread(mgr, route, 0L); // no timeout gct.start(); Thread.sleep(100); // give extra thread time to block @@ -474,14 +484,15 @@ public class TestTSCCMNoServer extends TestCase { // make sure the manager is still working try { - mgr.getConnection(route, 10L); + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); fail("should have gotten a timeout"); } catch (ConnectionPoolTimeoutException e) { // expected } mgr.releaseConnection(conn); - conn = mgr.getConnection(route, 10L); // this time, no exception + // this time: no exception + conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); assertNotNull("should have gotten a connection", conn); mgr.shutdown(); @@ -503,7 +514,8 @@ public class TestTSCCMNoServer extends TestCase { HttpRoute route2 = new HttpRoute(target2, null, false); // get the only connection, then start two extra threads - ManagedClientConnection conn = mgr.getConnection(route1, 1L); + ManagedClientConnection conn = + mgr.getConnection(route1, 1L, TimeUnit.MILLISECONDS); GetConnThread gct1 = new GetConnThread(mgr, route1, 1000L); GetConnThread gct2 = new GetConnThread(mgr, route2, 1000L); diff --git a/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java b/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java index b85d4af1d..35303594c 100644 --- a/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java +++ b/module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java @@ -32,6 +32,7 @@ package org.apache.http.impl.conn; import java.lang.ref.WeakReference; +import java.util.concurrent.TimeUnit; import junit.framework.Test; import junit.framework.TestSuite; @@ -211,7 +212,7 @@ public class TestTSCCMWithServer extends ServerTestBase { // check that there is no auto-release by default try { // this should fail quickly, connection has not been released - mgr.getConnection(route, 10L); + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -295,7 +296,7 @@ public class TestTSCCMWithServer extends ServerTestBase { // first check that we can't get another connection try { // this should fail quickly, connection has not been released - mgr.getConnection(route, 10L); + mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); fail("ConnectionPoolTimeoutException should have been thrown"); } catch (ConnectionPoolTimeoutException e) { // expected @@ -314,7 +315,7 @@ public class TestTSCCMWithServer extends ServerTestBase { Thread.sleep(1000); assertNull("connection not garbage collected", wref.get()); - conn = mgr.getConnection(route, 10L); + conn = mgr.getConnection(route, 10L, TimeUnit.MILLISECONDS); assertFalse("GCed connection not closed", conn.isOpen()); mgr.shutdown();