From 4752936837cc213f44d369ee6bc13bf47c5ac540 Mon Sep 17 00:00:00 2001 From: Sam Berlin Date: Thu, 26 Jun 2008 04:03:24 +0000 Subject: [PATCH] HTTPCLIENT-781 -- respect keep-alive header. git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@671717 13f79535-47bb-0310-9956-ffa450edef68 --- RELEASE_NOTES.txt | 3 + .../http/conn/BasicEofSensorWatcher.java | 4 +- .../apache/http/conn/BasicManagedEntity.java | 27 ++- .../http/conn/ClientConnectionManager.java | 25 ++- .../conn/ConnectionKeepAliveStrategy.java | 76 ++++++++ .../http/conn/ConnectionReleaseTrigger.java | 24 ++- .../http/conn/EofSensorInputStream.java | 3 +- .../http/impl/client/AbstractHttpClient.java | 20 ++ .../client/DefaultClientRequestDirector.java | 36 +++- .../DefaultConnectionKeepAliveStrategy.java | 131 ++++++++++++++ .../http/impl/client/DefaultHttpClient.java | 6 + .../impl/conn/AbstractClientConnAdapter.java | 8 +- .../http/impl/conn/IdleConnectionHandler.java | 76 +++++++- .../impl/conn/SingleClientConnManager.java | 18 +- .../impl/conn/tsccm/AbstractConnPool.java | 16 +- .../http/impl/conn/tsccm/ConnPoolByRoute.java | 55 +++--- .../tsccm/ThreadSafeClientConnManager.java | 9 +- .../http/conn/TestConnectionAutoRelease.java | 8 +- .../apache/http/conn/TestConnectionReuse.java | 88 ++++++++- .../TestDefaultClientRequestDirector.java | 16 +- .../apache/http/impl/conn/ExecReqThread.java | 2 +- .../http/impl/conn/TestSCMWithServer.java | 138 +++++++++++++- .../http/impl/conn/TestTSCCMNoServer.java | 23 ++- .../http/impl/conn/TestTSCCMWithServer.java | 171 ++++++++++++++++-- .../impl/conn/tsccm/TestConnPoolByRoute.java | 14 +- 25 files changed, 890 insertions(+), 107 deletions(-) create mode 100644 module-client/src/main/java/org/apache/http/conn/ConnectionKeepAliveStrategy.java create mode 100644 module-client/src/main/java/org/apache/http/impl/client/DefaultConnectionKeepAliveStrategy.java diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 8eae06aec..f56e83601 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,6 +1,9 @@ Changes since 4.0 Alpha 4 ------------------- +* [HTTPCLIENT-781] Respect Keep-Alive header's timeout value. + Contributed by Sam Berlin + * [HTTPCLIENT-779] Top-level classes (HttpClient, and HttpGet, HttpPut and similar HttpMethods) throw fewer exceptions. Contributed by Sam Berlin diff --git a/module-client/src/main/java/org/apache/http/conn/BasicEofSensorWatcher.java b/module-client/src/main/java/org/apache/http/conn/BasicEofSensorWatcher.java index b2e676c1c..c9c95edb5 100644 --- a/module-client/src/main/java/org/apache/http/conn/BasicEofSensorWatcher.java +++ b/module-client/src/main/java/org/apache/http/conn/BasicEofSensorWatcher.java @@ -86,7 +86,7 @@ public class BasicEofSensorWatcher implements EofSensorWatcher { managedConn.markReusable(); } } finally { - managedConn.releaseConnection(); + managedConn.releaseConnection(-1, null); } return false; } @@ -104,7 +104,7 @@ public class BasicEofSensorWatcher implements EofSensorWatcher { managedConn.markReusable(); } } finally { - managedConn.releaseConnection(); + managedConn.releaseConnection(-1, null); } return false; } diff --git a/module-client/src/main/java/org/apache/http/conn/BasicManagedEntity.java b/module-client/src/main/java/org/apache/http/conn/BasicManagedEntity.java index 8f16b96d5..d1680c40b 100644 --- a/module-client/src/main/java/org/apache/http/conn/BasicManagedEntity.java +++ b/module-client/src/main/java/org/apache/http/conn/BasicManagedEntity.java @@ -33,6 +33,7 @@ package org.apache.http.conn; import java.io.InputStream; import java.io.IOException; import java.io.OutputStream; +import java.util.concurrent.TimeUnit; import org.apache.http.HttpEntity; import org.apache.http.entity.HttpEntityWrapper; @@ -59,7 +60,13 @@ public class BasicManagedEntity extends HttpEntityWrapper protected ManagedClientConnection managedConn; /** Whether to keep the connection alive. */ - protected boolean attemptReuse; + protected final boolean attemptReuse; + + /** The duration this is valid for. */ + protected final long validDuration; + + /** The unit of time the duration is valid for. */ + protected final TimeUnit validUnit; /** @@ -74,15 +81,19 @@ public class BasicManagedEntity extends HttpEntityWrapper */ public BasicManagedEntity(HttpEntity entity, ManagedClientConnection conn, - boolean reuse) { + boolean reuse, + long validDuration, + TimeUnit validUnit) { super(entity); if (conn == null) throw new IllegalArgumentException ("Connection may not be null."); - managedConn = conn; - attemptReuse = reuse; + this.managedConn = conn; + this.attemptReuse = reuse; + this.validDuration = validDuration; + this.validUnit = validUnit; } @@ -129,7 +140,7 @@ public class BasicManagedEntity extends HttpEntityWrapper // non-javadoc, see interface ConnectionReleaseTrigger - public void releaseConnection() + public void releaseConnection(long validDuration, TimeUnit timeUnit) throws IOException { this.consumeContent(); @@ -210,7 +221,11 @@ public class BasicManagedEntity extends HttpEntityWrapper if (managedConn != null) { try { - managedConn.releaseConnection(); + // TODO: Should this be subtracting the elapsed time from + // when the entity was created till now? + // There's no good specification for when the 'timeout' + // starts counting. + managedConn.releaseConnection(validDuration, validUnit); } finally { managedConn = null; } 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 e05a3b0f8..f71de5436 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 @@ -73,12 +73,21 @@ public interface ClientConnectionManager { /** * Releases a connection for use by others. - * If the argument connection has been released before, + * You may optionally specify how long the connection is valid + * to be reused. Values <= 0 are considered to be valid forever. + * If the connection is not marked as reusable, the connection will + * not be reused regardless of the valid duration. + * + * If the connection has been released before, * the call will be ignored. * * @param conn the connection to release + * @param validDuration the duration of time this connection is valid for reuse + * @param timeUnit the unit of time validDuration is measured in + * + * @see #closeExpiredConnections() */ - void releaseConnection(ManagedClientConnection conn) + void releaseConnection(ManagedClientConnection conn, long validDuration, TimeUnit timeUnit) ; @@ -89,12 +98,24 @@ public interface ClientConnectionManager { * Currently allocated connections are not subject to this method. * Times will be checked with milliseconds precision * + * All expired connections will also be closed. + * * @param idletime the idle time of connections to be closed * @param tunit the unit for the idletime + * + * @see #closeExpiredConnections() */ void closeIdleConnections(long idletime, TimeUnit tunit) ; + /** + * Closes all expired connections in the pool. + * Open connections in the pool that have not been used for + * the timespan defined when the connection was released will be closed. + * Currently allocated connections are not subject to this method. + * Times will be checked with milliseconds precision. + */ + void closeExpiredConnections(); /** * Shuts down this connection manager and releases allocated resources. diff --git a/module-client/src/main/java/org/apache/http/conn/ConnectionKeepAliveStrategy.java b/module-client/src/main/java/org/apache/http/conn/ConnectionKeepAliveStrategy.java new file mode 100644 index 000000000..b5cd23124 --- /dev/null +++ b/module-client/src/main/java/org/apache/http/conn/ConnectionKeepAliveStrategy.java @@ -0,0 +1,76 @@ +/* + * $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.conn; + +import java.util.concurrent.TimeUnit; + +import org.apache.http.ConnectionReuseStrategy; +import org.apache.http.HttpResponse; +import org.apache.http.protocol.HttpContext; + +/** + * Interface for deciding how long a connection can remain + * idle before being reused. + * + * @author Sam Berlin + * + * + * @version $Revision: $ + * + * @since 4.0 + */ +public interface ConnectionKeepAliveStrategy { + + /** Returns the TimeUnit that this uses for specifying duration. */ + public TimeUnit getTimeUnit(); + + /** + * Returns the duration of time which this connection can be safely kept + * idle. If the connection is left idle for longer than this period of time, + * it MUST not reused. A value of 0 or less may be returned to indicate that + * there is no suitable suggestion. + * + * When coupled with a {@link ConnectionReuseStrategy}, if + * {@link ConnectionReuseStrategy#keepAlive(HttpResponse, HttpContext) + * returns true, this allows you to control how long the reuse will last. If + * keepAlive returns false, this should have no meaningful impact + * + * @param response + * The last response received over the connection. + * @param context + * the context in which the connection is being used. + * + * @return the duration which it is safe to keep the connection idle, + * or <=0 if no suggested duration. + */ + public long getKeepAliveDuration(HttpResponse response, HttpContext context); + +} diff --git a/module-client/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java b/module-client/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java index 5ef456b7f..bcac1a2c1 100644 --- a/module-client/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java +++ b/module-client/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java @@ -31,6 +31,7 @@ package org.apache.http.conn; import java.io.IOException; +import java.util.concurrent.TimeUnit; /** @@ -56,15 +57,22 @@ import java.io.IOException; public interface ConnectionReleaseTrigger { /** - * Releases the connection with the option of keep-alive. - * This is a "graceful" release and may cause IO operations - * for consuming the remainder of a response entity. - * Use {@link #abortConnection abortConnection} for a hard release. - * - * @throws IOException in case of an IO problem. - * The connection will be released anyway. + * Releases the connection with the option of keep-alive. This is a + * "graceful" release and may cause IO operations for consuming the + * remainder of a response entity. Use + * {@link #abortConnection abortConnection} for a hard release. The + * connection may be reused as specified by the duration. + * + * @param validDuration + * The duration of time this connection is valid to be reused. + * @param timeUnit + * The time unit the duration is measured in. + * + * @throws IOException + * in case of an IO problem. The connection will be released + * anyway. */ - void releaseConnection() + void releaseConnection(long validDuration, TimeUnit timeUnit) throws IOException ; diff --git a/module-client/src/main/java/org/apache/http/conn/EofSensorInputStream.java b/module-client/src/main/java/org/apache/http/conn/EofSensorInputStream.java index bb37510b9..91901de96 100644 --- a/module-client/src/main/java/org/apache/http/conn/EofSensorInputStream.java +++ b/module-client/src/main/java/org/apache/http/conn/EofSensorInputStream.java @@ -32,6 +32,7 @@ package org.apache.http.conn; import java.io.InputStream; import java.io.IOException; +import java.util.concurrent.TimeUnit; /** @@ -305,7 +306,7 @@ public class EofSensorInputStream extends InputStream /** * Same as {@link #close close()}. */ - public void releaseConnection() throws IOException { + public void releaseConnection(long validDuration, TimeUnit timeUnit) throws IOException { this.close(); } diff --git a/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java b/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java index fa4641661..080ff23b3 100644 --- a/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java +++ b/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java @@ -54,6 +54,7 @@ import org.apache.http.client.UserTokenHandler; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.protocol.ClientContext; import org.apache.http.conn.ClientConnectionManager; +import org.apache.http.conn.ConnectionKeepAliveStrategy; import org.apache.http.conn.routing.HttpRoutePlanner; import org.apache.http.cookie.CookieSpecRegistry; import org.apache.http.params.HttpParams; @@ -87,6 +88,9 @@ public abstract class AbstractHttpClient implements HttpClient { /** The connection re-use strategy. */ private ConnectionReuseStrategy reuseStrategy; + + /** The connection keep-alive strategy. */ + private ConnectionKeepAliveStrategy keepAliveStrategy; /** The cookie spec registry. */ private CookieSpecRegistry supportedCookieSpecs; @@ -155,6 +159,7 @@ public abstract class AbstractHttpClient implements HttpClient { protected abstract ConnectionReuseStrategy createConnectionReuseStrategy(); + protected abstract ConnectionKeepAliveStrategy createConnectionKeepAliveStrategy(); protected abstract BasicHttpProcessor createHttpProcessor(); @@ -257,6 +262,18 @@ public abstract class AbstractHttpClient implements HttpClient { this.reuseStrategy = reuseStrategy; } + + public synchronized final ConnectionKeepAliveStrategy getConnectionKeepAliveStrategy() { + if (keepAliveStrategy == null) { + keepAliveStrategy = createConnectionKeepAliveStrategy(); + } + return keepAliveStrategy; + } + + public synchronized void setKeepAliveStrategy(final ConnectionKeepAliveStrategy keepAliveStrategy) { + this.keepAliveStrategy = keepAliveStrategy; + } + public synchronized final HttpRequestRetryHandler getHttpRequestRetryHandler() { if (retryHandler == null) { @@ -525,6 +542,7 @@ public abstract class AbstractHttpClient implements HttpClient { getRequestExecutor(), getConnectionManager(), getConnectionReuseStrategy(), + getConnectionKeepAliveStrategy(), getRoutePlanner(), getHttpProcessor().copy(), getHttpRequestRetryHandler(), @@ -547,6 +565,7 @@ public abstract class AbstractHttpClient implements HttpClient { final HttpRequestExecutor requestExec, final ClientConnectionManager conman, final ConnectionReuseStrategy reustrat, + final ConnectionKeepAliveStrategy kastrat, final HttpRoutePlanner rouplan, final HttpProcessor httpProcessor, final HttpRequestRetryHandler retryHandler, @@ -559,6 +578,7 @@ public abstract class AbstractHttpClient implements HttpClient { requestExec, conman, reustrat, + kastrat, rouplan, httpProcessor, retryHandler, 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 3a83c8bd8..70960f334 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 @@ -74,6 +74,7 @@ import org.apache.http.client.utils.URIUtils; import org.apache.http.conn.BasicManagedEntity; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.ClientConnectionRequest; +import org.apache.http.conn.ConnectionKeepAliveStrategy; import org.apache.http.conn.ManagedClientConnection; import org.apache.http.conn.params.ConnManagerParams; import org.apache.http.conn.routing.BasicRouteDirector; @@ -118,6 +119,9 @@ public class DefaultClientRequestDirector /** The connection re-use strategy. */ protected final ConnectionReuseStrategy reuseStrategy; + + /** The keep-alive duration strategy. */ + protected final ConnectionKeepAliveStrategy keepAliveStrategy; /** The request executor. */ protected final HttpRequestExecutor requestExec; @@ -158,6 +162,7 @@ public class DefaultClientRequestDirector final HttpRequestExecutor requestExec, final ClientConnectionManager conman, final ConnectionReuseStrategy reustrat, + final ConnectionKeepAliveStrategy kastrat, final HttpRoutePlanner rouplan, final HttpProcessor httpProcessor, final HttpRequestRetryHandler retryHandler, @@ -179,6 +184,10 @@ public class DefaultClientRequestDirector throw new IllegalArgumentException ("Connection reuse strategy may not be null."); } + if (kastrat == null) { + throw new IllegalArgumentException + ("Connection keep alive strategy may not be null."); + } if (rouplan == null) { throw new IllegalArgumentException ("Route planner may not be null."); @@ -214,6 +223,7 @@ public class DefaultClientRequestDirector this.requestExec = requestExec; this.connManager = conman; this.reuseStrategy = reustrat; + this.keepAliveStrategy = kastrat; this.routePlanner = rouplan; this.httpProcessor = httpProcessor; this.retryHandler = retryHandler; @@ -336,7 +346,7 @@ public class DefaultClientRequestDirector // Reopen connection if needed if (!managedConn.isOpen()) { managedConn.open(route, context, params); - } + } try { establishRoute(route, context); @@ -455,9 +465,7 @@ public class DefaultClientRequestDirector } // check if we can use the same connection for the followup if (!followup.getRoute().equals(roureq.getRoute())) { - // the followup has a different route, release conn - connManager.releaseConnection(managedConn); - managedConn = null; + releaseConnection(response, context); } roureq = followup; } @@ -478,12 +486,13 @@ public class DefaultClientRequestDirector // connection not needed and (assumed to be) in re-usable state if (reuse) managedConn.markReusable(); - connManager.releaseConnection(managedConn); - managedConn = null; + releaseConnection(response, context); } else { // install an auto-release entity HttpEntity entity = response.getEntity(); - entity = new BasicManagedEntity(entity, managedConn, reuse); + long duration = keepAliveStrategy.getKeepAliveDuration(response, context); + TimeUnit unit = keepAliveStrategy.getTimeUnit(); + entity = new BasicManagedEntity(entity, managedConn, reuse, duration, unit); response.setEntity(entity); } @@ -501,6 +510,17 @@ public class DefaultClientRequestDirector } } // execute + /** + * Returns the connection back to the connection manager + * and prepares for retrieving a new connection during + * the next request. + */ + protected void releaseConnection(HttpResponse response, HttpContext context) { + long duration = keepAliveStrategy.getKeepAliveDuration(response, context); + TimeUnit unit = keepAliveStrategy.getTimeUnit(); + connManager.releaseConnection(managedConn, duration, unit); + managedConn = null; + } /** * Determines the route for a request. @@ -977,7 +997,7 @@ public class DefaultClientRequestDirector } } // ensure the connection manager properly releases this connection - connManager.releaseConnection(mcc); + connManager.releaseConnection(mcc, -1, null); } } // abortConnection diff --git a/module-client/src/main/java/org/apache/http/impl/client/DefaultConnectionKeepAliveStrategy.java b/module-client/src/main/java/org/apache/http/impl/client/DefaultConnectionKeepAliveStrategy.java new file mode 100644 index 000000000..f18ffb275 --- /dev/null +++ b/module-client/src/main/java/org/apache/http/impl/client/DefaultConnectionKeepAliveStrategy.java @@ -0,0 +1,131 @@ +/* + * $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.client; + +import java.util.Locale; +import java.util.StringTokenizer; +import java.util.concurrent.TimeUnit; + +import org.apache.http.Header; +import org.apache.http.HeaderIterator; +import org.apache.http.HttpResponse; +import org.apache.http.TokenIterator; +import org.apache.http.conn.ConnectionKeepAliveStrategy; +import org.apache.http.message.BasicTokenIterator; +import org.apache.http.protocol.HTTP; +import org.apache.http.protocol.HttpContext; + +/** + * Default implementation of a strategy deciding duration + * that a connection can remain idle. + * + * The default implementation looks solely at the 'Keep-Alive' + * header's timeout token. + * + * @author Sam Berlin + * + * @version $Revision: $ + * + * @since 4.0 + */ +public class DefaultConnectionKeepAliveStrategy implements ConnectionKeepAliveStrategy { + + public long getKeepAliveDuration(HttpResponse response, HttpContext context) { + long duration = -1; + HeaderIterator hit = response.headerIterator(HTTP.CONN_KEEP_ALIVE); + + while(hit.hasNext() && duration==-1) { + Header header = hit.nextHeader(); + if(header.getValue() == null) + continue; + StringTokenizer tokenizer = new StringTokenizer(header.getValue(), ","); + while(tokenizer.hasMoreTokens()) { + String token = tokenizer.nextToken().trim(); + if(token.toLowerCase(Locale.US).startsWith("timeout=")) { + duration = parseTimeout(token); + break; + } + } + } + + // TODO: I'd prefer to do it this way, but BasicTokenIterator + // freaks out on an '=' character. +// if(hit.hasNext()) { +// try { +// TokenIterator it = createTokenIterator(hit); +// while(it.hasNext()) { +// String token = it.nextToken(); +// if(token.toLowerCase(Locale.US).startsWith("timeout=")) { +// duration = parseTimeout(token); +// break; +// } +// } +// } catch(ParseException pe) { +// // Stop trying to find it and just fall-through. +// } +// } + + return duration; + } + + /** + * Parses the # out of the 'timeout=#' token. + */ + protected long parseTimeout(String token) { + // Make sure the length is valid. + if(token.length() == "timeout=".length()) + return -1; + + try { + return Long.parseLong(token.substring("timeout=".length())); + } catch(NumberFormatException nfe) { + return -1; + } + } + + public TimeUnit getTimeUnit() { + return TimeUnit.SECONDS; + } + + /** + * Creates a token iterator from a header iterator. + * This method can be overridden to replace the implementation of + * the token iterator. + * + * @param hit the header iterator + * + * @return the token iterator + */ + protected TokenIterator createTokenIterator(HeaderIterator hit) { + return new BasicTokenIterator(hit); + } + +} diff --git a/module-client/src/main/java/org/apache/http/impl/client/DefaultHttpClient.java b/module-client/src/main/java/org/apache/http/impl/client/DefaultHttpClient.java index bf793b74c..f4c3eba1b 100644 --- a/module-client/src/main/java/org/apache/http/impl/client/DefaultHttpClient.java +++ b/module-client/src/main/java/org/apache/http/impl/client/DefaultHttpClient.java @@ -50,6 +50,7 @@ import org.apache.http.client.protocol.RequestTargetAuthentication; import org.apache.http.client.protocol.ResponseProcessCookies; import org.apache.http.conn.ClientConnectionManager; import org.apache.http.conn.ClientConnectionManagerFactory; +import org.apache.http.conn.ConnectionKeepAliveStrategy; import org.apache.http.conn.routing.HttpRoutePlanner; import org.apache.http.conn.scheme.PlainSocketFactory; import org.apache.http.conn.scheme.Scheme; @@ -204,6 +205,11 @@ public class DefaultHttpClient extends AbstractHttpClient { return new DefaultConnectionReuseStrategy(); } + @Override + protected ConnectionKeepAliveStrategy createConnectionKeepAliveStrategy() { + return new DefaultConnectionKeepAliveStrategy(); + } + @Override protected AuthSchemeRegistry createAuthSchemeRegistry() { diff --git a/module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java b/module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java index dcabdfed5..a498135b8 100644 --- a/module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java +++ b/module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java @@ -35,6 +35,8 @@ import java.io.IOException; import java.io.InterruptedIOException; import java.net.InetAddress; import java.net.Socket; +import java.util.concurrent.TimeUnit; + import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSession; @@ -347,9 +349,9 @@ public abstract class AbstractClientConnAdapter } // non-javadoc, see interface ConnectionReleaseTrigger - public void releaseConnection() { + public void releaseConnection(long validDuration, TimeUnit timeUnit) { if (connManager != null) - connManager.releaseConnection(this); + connManager.releaseConnection(this, validDuration, timeUnit); } // non-javadoc, see interface ConnectionReleaseTrigger @@ -377,7 +379,7 @@ public abstract class AbstractClientConnAdapter // manager if #abortConnection() is called from the main execution // thread while there is no blocking I/O operation. if (executionThread.equals(Thread.currentThread())) { - releaseConnection(); + releaseConnection(-1, null); } } diff --git a/module-client/src/main/java/org/apache/http/impl/conn/IdleConnectionHandler.java b/module-client/src/main/java/org/apache/http/impl/conn/IdleConnectionHandler.java index 48c8e325d..2aca12d7c 100644 --- a/module-client/src/main/java/org/apache/http/impl/conn/IdleConnectionHandler.java +++ b/module-client/src/main/java/org/apache/http/impl/conn/IdleConnectionHandler.java @@ -33,6 +33,7 @@ import java.io.IOException; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.concurrent.TimeUnit; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -53,12 +54,12 @@ public class IdleConnectionHandler { private final Log LOG = LogFactory.getLog(IdleConnectionHandler.class); /** Holds connections and the time they were added. */ - private final Map connectionToAdded; + private final Map connectionToTimes; public IdleConnectionHandler() { super(); - connectionToAdded = new HashMap(); + connectionToTimes = new HashMap(); } /** @@ -69,7 +70,7 @@ public class IdleConnectionHandler { * * @see #remove */ - public void add(HttpConnection connection) { + public void add(HttpConnection connection, long validDuration, TimeUnit unit) { Long timeAdded = Long.valueOf(System.currentTimeMillis()); @@ -77,22 +78,32 @@ public class IdleConnectionHandler { LOG.debug("Adding connection at: " + timeAdded); } - connectionToAdded.put(connection, timeAdded); + connectionToTimes.put(connection, new TimeValues(timeAdded, validDuration, unit)); } /** * Removes the given connection from the list of connections to be closed when idle. + * This will return true if the connection is still valid, and false + * if the connection should be considered expired and not used. + * * @param connection + * @return True if the connection is still valid. */ - public void remove(HttpConnection connection) { - connectionToAdded.remove(connection); + public boolean remove(HttpConnection connection) { + TimeValues times = connectionToTimes.remove(connection); + if(times == null) { + LOG.warn("Removing a connection that never existed!"); + return true; + } else { + return System.currentTimeMillis() <= times.timeExpires; + } } /** * Removes all connections referenced by this handler. */ public void removeAll() { - this.connectionToAdded.clear(); + this.connectionToTimes.clear(); } /** @@ -111,11 +122,12 @@ public class IdleConnectionHandler { } Iterator connectionIter = - connectionToAdded.keySet().iterator(); + connectionToTimes.keySet().iterator(); while (connectionIter.hasNext()) { HttpConnection conn = connectionIter.next(); - Long connectionTime = connectionToAdded.get(conn); + TimeValues times = connectionToTimes.get(conn); + Long connectionTime = times.timeAdded; if (connectionTime.longValue() <= idleTimeout) { if (LOG.isDebugEnabled()) { LOG.debug("Closing connection, connection time: " + connectionTime); @@ -129,4 +141,50 @@ public class IdleConnectionHandler { } } } + + + public void closeExpiredConnections() { + long now = System.currentTimeMillis(); + if (LOG.isDebugEnabled()) { + LOG.debug("Checking for expired connections, now: " + now); + } + + Iterator connectionIter = + connectionToTimes.keySet().iterator(); + + while (connectionIter.hasNext()) { + HttpConnection conn = connectionIter.next(); + TimeValues times = connectionToTimes.get(conn); + if(times.timeExpires <= now) { + if (LOG.isDebugEnabled()) { + LOG.debug("Closing connection, expired @: " + times.timeExpires); + } + connectionIter.remove(); + try { + conn.close(); + } catch (IOException ex) { + LOG.debug("I/O error closing connection", ex); + } + } + } + } + + private static class TimeValues { + private final long timeAdded; + private final long timeExpires; + + /** + * @param now The current time in milliseconds + * @param validDuration The duration this connection is valid for + * @param validUnit The unit of time the duration is specified in. + */ + TimeValues(long now, long validDuration, TimeUnit validUnit) { + this.timeAdded = now; + if(validDuration > 0) { + this.timeExpires = now + validUnit.toMillis(validDuration); + } else { + this.timeExpires = Long.MAX_VALUE; + } + } + } } 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 3126be415..80637a4c7 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 @@ -92,6 +92,9 @@ public class SingleClientConnManager implements ClientConnectionManager { /** The time of the last connection release, or -1. */ protected long lastReleaseTime; + + /** The time the last released connection expires and shouldn't be reused. */ + protected long connectionExpiresTime; /** Whether the connection should be shut down on release. */ protected boolean alwaysShutDown; @@ -219,6 +222,9 @@ public class SingleClientConnManager implements ClientConnectionManager { boolean recreate = false; boolean shutdown = false; + // Kill the connection if it expired. + closeExpiredConnections(); + if (uniquePoolEntry.connection.isOpen()) { RouteTracker tracker = uniquePoolEntry.tracker; shutdown = (tracker == null || // can happen if method is aborted @@ -251,7 +257,7 @@ public class SingleClientConnManager implements ClientConnectionManager { // non-javadoc, see interface ClientConnectionManager - public void releaseConnection(ManagedClientConnection conn) { + public void releaseConnection(ManagedClientConnection conn, long validDuration, TimeUnit timeUnit) { assertStillUp(); if (!(conn instanceof ConnAdapter)) { @@ -297,8 +303,18 @@ public class SingleClientConnManager implements ClientConnectionManager { sca.detach(); managedConn = null; lastReleaseTime = System.currentTimeMillis(); + if(validDuration > 0) + connectionExpiresTime = timeUnit.toMillis(validDuration) + lastReleaseTime; + else + connectionExpiresTime = Long.MAX_VALUE; } } // releaseConnection + + public void closeExpiredConnections() { + if(System.currentTimeMillis() >= connectionExpiresTime) { + closeIdleConnections(0, TimeUnit.MILLISECONDS); + } + } // non-javadoc, see interface ClientConnectionManager 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 3df138e35..9ce5e4c31 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 @@ -183,8 +183,10 @@ public abstract class AbstractConnPool implements RefQueueHandler { * @param entry the entry for the connection to release * @param reusable true if the entry is deemed * reusable, false otherwise. + * @param validDuration The duration that the entry should remain free and reusable. + * @param timeUnit The unit of time the duration is measured in. */ - public abstract void freeEntry(BasicPoolEntry entry, boolean reusable) + public abstract void freeEntry(BasicPoolEntry entry, boolean reusable, long validDuration, TimeUnit timeUnit) ; @@ -248,6 +250,16 @@ public abstract class AbstractConnPool implements RefQueueHandler { poolLock.unlock(); } } + + public void closeExpiredConnections() { + poolLock.lock(); + try { + idleConnHandler.closeExpiredConnections(); + } finally { + poolLock.unlock(); + } + } + //@@@ revise this cleanup stuff (closeIdle+deleteClosed), it's not good @@ -314,5 +326,7 @@ public abstract class AbstractConnPool implements RefQueueHandler { + + } // class AbstractConnPool 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 f68254bd9..cf3d572b0 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 @@ -367,7 +367,7 @@ public class ConnPoolByRoute extends AbstractConnPool { // non-javadoc, see base class AbstractConnPool @Override - public void freeEntry(BasicPoolEntry entry, boolean reusable) { + public void freeEntry(BasicPoolEntry entry, boolean reusable, long validDuration, TimeUnit timeUnit) { HttpRoute route = entry.getPlannedRoute(); if (LOG.isDebugEnabled()) { @@ -392,7 +392,7 @@ public class ConnPoolByRoute extends AbstractConnPool { if (reusable) { rospl.freeEntry(entry); freeConnections.add(entry); - idleConnHandler.add(entry.getConnection()); + idleConnHandler.add(entry.getConnection(), validDuration, timeUnit); } else { rospl.dropEntry(); numConnections--; @@ -421,27 +421,40 @@ public class ConnPoolByRoute extends AbstractConnPool { BasicPoolEntry entry = null; poolLock.lock(); try { - - entry = rospl.allocEntry(state); - - if (entry != null) { - if (LOG.isDebugEnabled()) { - LOG.debug("Getting free connection" - + " [" + rospl.getRoute() + "][" + state + "]"); - - } - freeConnections.remove(entry); - idleConnHandler.remove(entry.getConnection());// no longer idle - - issuedConnections.add(entry.getWeakRef()); - - } else { - if (LOG.isDebugEnabled()) { - LOG.debug("No free connections" - + " [" + rospl.getRoute() + "][" + state + "]"); + boolean done = false; + while(!done) { + entry = rospl.allocEntry(state); + + if (entry != null) { + if (LOG.isDebugEnabled()) { + LOG.debug("Getting free connection" + + " [" + rospl.getRoute() + "][" + state + "]"); + + } + freeConnections.remove(entry); + boolean valid = idleConnHandler.remove(entry.getConnection()); + if(!valid) { + // If the free entry isn't valid anymore, get rid of it + // and loop to find another one that might be valid. + if(LOG.isDebugEnabled()) + LOG.debug("Closing expired free connection" + + " [" + rospl.getRoute() + "][" + state + "]"); + closeConnection(entry.getConnection()); + rospl.deleteEntry(entry); + numConnections--; + } else { + issuedConnections.add(entry.getWeakRef()); + done = true; + } + + } else { + done = true; + if (LOG.isDebugEnabled()) { + LOG.debug("No free connections" + + " [" + rospl.getRoute() + "][" + state + "]"); + } } } - } finally { poolLock.unlock(); } 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 0b276b387..bea3cfdf7 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 @@ -185,7 +185,7 @@ public class ThreadSafeClientConnManager // non-javadoc, see interface ClientConnectionManager - public void releaseConnection(ManagedClientConnection conn) { + public void releaseConnection(ManagedClientConnection conn, long validDuration, TimeUnit timeUnit) { if (!(conn instanceof BasicPooledConnAdapter)) { throw new IllegalArgumentException @@ -225,7 +225,7 @@ public class ThreadSafeClientConnManager boolean reusable = hca.isMarkedReusable(); hca.detach(); if (entry != null) { - connectionPool.freeEntry(entry, reusable); + connectionPool.freeEntry(entry, reusable, validDuration, timeUnit); } } } @@ -274,6 +274,11 @@ public class ThreadSafeClientConnManager connectionPool.closeIdleConnections(idleTimeout, tunit); connectionPool.deleteClosedConnections(); } + + public void closeExpiredConnections() { + connectionPool.closeExpiredConnections(); + connectionPool.deleteClosedConnections(); + } } // class ThreadSafeClientConnManager diff --git a/module-client/src/test/java/org/apache/http/conn/TestConnectionAutoRelease.java b/module-client/src/test/java/org/apache/http/conn/TestConnectionAutoRelease.java index 102d77463..1d83234f9 100644 --- a/module-client/src/test/java/org/apache/http/conn/TestConnectionAutoRelease.java +++ b/module-client/src/test/java/org/apache/http/conn/TestConnectionAutoRelease.java @@ -122,7 +122,7 @@ public class TestConnectionAutoRelease extends ServerTestBase { connreq = mgr.requestConnection(new HttpRoute(target), null); ManagedClientConnection conn = connreq.getConnection(250, TimeUnit.MILLISECONDS); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); mgr.shutdown(); } @@ -165,7 +165,7 @@ public class TestConnectionAutoRelease extends ServerTestBase { connreq = mgr.requestConnection(new HttpRoute(target), null); ManagedClientConnection conn = connreq.getConnection(250, TimeUnit.MILLISECONDS); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); mgr.shutdown(); } @@ -207,7 +207,7 @@ public class TestConnectionAutoRelease extends ServerTestBase { connreq = mgr.requestConnection(new HttpRoute(target), null); ManagedClientConnection conn = connreq.getConnection(250, TimeUnit.MILLISECONDS); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); mgr.shutdown(); } @@ -288,7 +288,7 @@ public class TestConnectionAutoRelease extends ServerTestBase { connreq = mgr.requestConnection(new HttpRoute(target), null); ManagedClientConnection conn = connreq.getConnection(250, TimeUnit.MILLISECONDS); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); mgr.shutdown(); } diff --git a/module-client/src/test/java/org/apache/http/conn/TestConnectionReuse.java b/module-client/src/test/java/org/apache/http/conn/TestConnectionReuse.java index 62d67d9ec..88b38ef25 100644 --- a/module-client/src/test/java/org/apache/http/conn/TestConnectionReuse.java +++ b/module-client/src/test/java/org/apache/http/conn/TestConnectionReuse.java @@ -39,6 +39,7 @@ import junit.framework.Test; import junit.framework.TestCase; import junit.framework.TestSuite; +import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpException; import org.apache.http.HttpHost; @@ -47,8 +48,8 @@ import org.apache.http.HttpResponseInterceptor; import org.apache.http.HttpVersion; import org.apache.http.client.HttpClient; import org.apache.http.client.methods.HttpGet; -import org.apache.http.conn.params.ConnPerRouteBean; import org.apache.http.conn.params.ConnManagerParams; +import org.apache.http.conn.params.ConnPerRouteBean; import org.apache.http.conn.scheme.PlainSocketFactory; import org.apache.http.conn.scheme.Scheme; import org.apache.http.conn.scheme.SchemeRegistry; @@ -289,6 +290,77 @@ public class TestConnectionReuse extends TestCase { mgr.shutdown(); } + + public void testKeepAliveHeaderRespected() throws Exception { + BasicHttpProcessor httpproc = new BasicHttpProcessor(); + httpproc.addInterceptor(new ResponseDate()); + httpproc.addInterceptor(new ResponseServer()); + httpproc.addInterceptor(new ResponseContent()); + httpproc.addInterceptor(new ResponseConnControl()); + httpproc.addInterceptor(new ResponseKeepAlive()); + + this.localServer = new LocalTestServer(httpproc, null); + this.localServer.register("/random/*", new RandomHandler()); + this.localServer.start(); + + InetSocketAddress saddress = (InetSocketAddress) this.localServer.getServiceAddress(); + + HttpParams params = new BasicHttpParams(); + HttpProtocolParams.setVersion(params, HttpVersion.HTTP_1_1); + HttpProtocolParams.setContentCharset(params, "UTF-8"); + HttpProtocolParams.setUserAgent(params, "TestAgent/1.1"); + HttpProtocolParams.setUseExpectContinue(params, false); + HttpConnectionParams.setStaleCheckingEnabled(params, false); + ConnManagerParams.setMaxTotalConnections(params, 5); + ConnManagerParams.setMaxConnectionsPerRoute(params, + new ConnPerRouteBean(5)); + + SchemeRegistry supportedSchemes = new SchemeRegistry(); + SocketFactory sf = PlainSocketFactory.getSocketFactory(); + supportedSchemes.register(new Scheme("http", sf, 80)); + + ThreadSafeClientConnManager mgr = new ThreadSafeClientConnManager( + params, supportedSchemes); + + DefaultHttpClient client = new DefaultHttpClient(mgr, params); + HttpHost target = new HttpHost(saddress.getHostName(), saddress.getPort(), "http"); + + HttpResponse response = client.execute(target, new HttpGet("/random/2000")); + if(response.getEntity() != null) + response.getEntity().consumeContent(); + + assertEquals(1, mgr.getConnectionsInPool()); + assertEquals(1, localServer.getAcceptedConnectionCount()); + + response = client.execute(target, new HttpGet("/random/2000")); + if(response.getEntity() != null) + response.getEntity().consumeContent(); + + assertEquals(1, mgr.getConnectionsInPool()); + assertEquals(1, localServer.getAcceptedConnectionCount()); + + // Now sleep for 1.1 seconds and let the timeout do its work + Thread.sleep(1100); + response = client.execute(target, new HttpGet("/random/2000")); + if(response.getEntity() != null) + response.getEntity().consumeContent(); + + assertEquals(1, mgr.getConnectionsInPool()); + assertEquals(2, localServer.getAcceptedConnectionCount()); + + // Do another request just under the 1 second limit & make + // sure we reuse that connection. + Thread.sleep(500); + response = client.execute(target, new HttpGet("/random/2000")); + if(response.getEntity() != null) + response.getEntity().consumeContent(); + + assertEquals(1, mgr.getConnectionsInPool()); + assertEquals(2, localServer.getAcceptedConnectionCount()); + + + mgr.shutdown(); + } private static class WorkerThread extends Thread { @@ -342,4 +414,18 @@ public class TestConnectionReuse extends TestCase { } + // A very basic keep-alive header interceptor, to add Keep-Alive: timeout=1 + // if there is no Connection: close header. + private static class ResponseKeepAlive implements HttpResponseInterceptor { + public void process(HttpResponse response, HttpContext context) + throws HttpException, IOException { + Header connection = response.getFirstHeader(HTTP.CONN_DIRECTIVE); + if(connection != null) { + if(!connection.getValue().equalsIgnoreCase("Close")) { + response.addHeader(HTTP.CONN_KEEP_ALIVE, "timeout=1"); + } + } + } + } + } 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 4d87d3689..32812faaa 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 @@ -404,9 +404,9 @@ public class TestDefaultClientRequestDirector extends ServerTestBase { } @Override - public void releaseConnection(ManagedClientConnection conn) { + public void releaseConnection(ManagedClientConnection conn, long validDuration, TimeUnit timeUnit) { releasedConnection = conn; - super.releaseConnection(conn); + super.releaseConnection(conn, validDuration, timeUnit); } @@ -423,6 +423,10 @@ public class TestDefaultClientRequestDirector extends ServerTestBase { public void closeIdleConnections(long idletime, TimeUnit tunit) { throw new UnsupportedOperationException("just a mockup"); } + + public void closeExpiredConnections() { + throw new UnsupportedOperationException("just a mockup"); + } public ManagedClientConnection getConnection(HttpRoute route) throws InterruptedException { @@ -470,7 +474,7 @@ public class TestDefaultClientRequestDirector extends ServerTestBase { return registry; } - public void releaseConnection(ManagedClientConnection conn) { + public void releaseConnection(ManagedClientConnection conn, long validDuration, TimeUnit timeUnit) { this.releasedConnection = conn; } @@ -491,6 +495,10 @@ public class TestDefaultClientRequestDirector extends ServerTestBase { public void closeIdleConnections(long idletime, TimeUnit tunit) { throw new UnsupportedOperationException("just a mockup"); } + + public void closeExpiredConnections() { + throw new UnsupportedOperationException("just a mockup"); + } public ManagedClientConnection getConnection(HttpRoute route) throws InterruptedException { @@ -542,7 +550,7 @@ public class TestDefaultClientRequestDirector extends ServerTestBase { return registry; } - public void releaseConnection(ManagedClientConnection conn) { + public void releaseConnection(ManagedClientConnection conn, long validDuration, TimeUnit timeUnit) { 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 091b98318..e210d61f4 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 @@ -122,7 +122,7 @@ public class ExecReqThread extends GetConnThread { exception = dart; } finally { - conn_manager.releaseConnection(connection); + conn_manager.releaseConnection(connection, -1, null); } } diff --git a/module-client/src/test/java/org/apache/http/impl/conn/TestSCMWithServer.java b/module-client/src/test/java/org/apache/http/impl/conn/TestSCMWithServer.java index a0a9f8483..8fab62d98 100644 --- a/module-client/src/test/java/org/apache/http/impl/conn/TestSCMWithServer.java +++ b/module-client/src/test/java/org/apache/http/impl/conn/TestSCMWithServer.java @@ -30,16 +30,25 @@ package org.apache.http.impl.conn; +import java.util.concurrent.TimeUnit; + import junit.framework.Test; import junit.framework.TestSuite; import org.apache.http.HttpHost; +import org.apache.http.HttpRequest; +import org.apache.http.HttpResponse; +import org.apache.http.HttpStatus; +import org.apache.http.HttpVersion; import org.apache.http.conn.ManagedClientConnection; import org.apache.http.conn.params.ConnManagerParams; import org.apache.http.conn.routing.HttpRoute; import org.apache.http.conn.scheme.SchemeRegistry; import org.apache.http.localserver.ServerTestBase; +import org.apache.http.message.BasicHttpRequest; import org.apache.http.params.HttpParams; +import org.apache.http.protocol.ExecutionContext; +import org.apache.http.util.EntityUtils; public class TestSCMWithServer extends ServerTestBase { @@ -97,7 +106,134 @@ public class TestSCMWithServer extends ServerTestBase { assertFalse("connection should have been closed", conn.isOpen()); conn.open(route, httpContext, defaultParams); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); mgr.shutdown(); } + + /** + * Tests releasing with time limits. + */ + public void testReleaseConnectionWithTimeLimits() throws Exception { + + HttpParams mgrpar = defaultParams.copy(); + ConnManagerParams.setMaxTotalConnections(mgrpar, 1); + + SingleClientConnManager mgr = createSCCM(mgrpar, null); + + final HttpHost target = getServerHttp(); + final HttpRoute route = new HttpRoute(target, null, false); + final int rsplen = 8; + final String uri = "/random/" + rsplen; + + HttpRequest request = + new BasicHttpRequest("GET", uri, HttpVersion.HTTP_1_1); + + ManagedClientConnection conn = mgr.getConnection(route, null); + conn.open(route, httpContext, defaultParams); + + // a new context is created for each testcase, no need to reset + HttpResponse response = Helper.execute( + request, conn, target, + httpExecutor, httpProcessor, defaultParams, httpContext); + + assertEquals("wrong status in first response", + HttpStatus.SC_OK, + response.getStatusLine().getStatusCode()); + byte[] data = EntityUtils.toByteArray(response.getEntity()); + assertEquals("wrong length of first response entity", + rsplen, data.length); + // ignore data, but it must be read + + // release connection without marking for re-use + // expect the next connection obtained to be closed + mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS); + conn = mgr.getConnection(route, null); + assertFalse("connection should have been closed", conn.isOpen()); + + // repeat the communication, no need to prepare the request again + conn.open(route, httpContext, defaultParams); + httpContext.setAttribute(ExecutionContext.HTTP_CONNECTION, conn); + response = httpExecutor.execute(request, conn, httpContext); + httpExecutor.postProcess(response, httpProcessor, httpContext); + + assertEquals("wrong status in second response", + HttpStatus.SC_OK, + response.getStatusLine().getStatusCode()); + data = EntityUtils.toByteArray(response.getEntity()); + assertEquals("wrong length of second response entity", + rsplen, data.length); + // ignore data, but it must be read + + // release connection after marking it for re-use + // expect the next connection obtained to be open + conn.markReusable(); + mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS); + conn = mgr.getConnection(route, null); + assertTrue("connection should have been open", conn.isOpen()); + + // repeat the communication, no need to prepare the request again + httpContext.setAttribute(ExecutionContext.HTTP_CONNECTION, conn); + response = httpExecutor.execute(request, conn, httpContext); + httpExecutor.postProcess(response, httpProcessor, httpContext); + + assertEquals("wrong status in third response", + HttpStatus.SC_OK, + response.getStatusLine().getStatusCode()); + data = EntityUtils.toByteArray(response.getEntity()); + assertEquals("wrong length of third response entity", + rsplen, data.length); + // ignore data, but it must be read + + conn.markReusable(); + mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS); + Thread.sleep(150); + conn = mgr.getConnection(route, null); + assertTrue("connection should have been closed", !conn.isOpen()); + + // repeat the communication, no need to prepare the request again + conn.open(route, httpContext, defaultParams); + httpContext.setAttribute(ExecutionContext.HTTP_CONNECTION, conn); + response = httpExecutor.execute(request, conn, httpContext); + httpExecutor.postProcess(response, httpProcessor, httpContext); + + assertEquals("wrong status in third response", + HttpStatus.SC_OK, + response.getStatusLine().getStatusCode()); + data = EntityUtils.toByteArray(response.getEntity()); + assertEquals("wrong length of fourth response entity", + rsplen, data.length); + // ignore data, but it must be read + + mgr.shutdown(); + } + + + public void testCloseExpiredConnections() throws Exception { + + HttpParams mgrpar = defaultParams.copy(); + ConnManagerParams.setMaxTotalConnections(mgrpar, 1); + + SingleClientConnManager mgr = createSCCM(mgrpar, null); + + final HttpHost target = getServerHttp(); + final HttpRoute route = new HttpRoute(target, null, false); + + ManagedClientConnection conn = mgr.getConnection(route, null); + conn.open(route, httpContext, defaultParams); + mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS); + + mgr.closeExpiredConnections(); + + conn = mgr.getConnection(route, null); + assertTrue(conn.isOpen()); + mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS); + + Thread.sleep(150); + mgr.closeExpiredConnections(); + conn = mgr.getConnection(route, null); + assertFalse(conn.isOpen()); + + mgr.shutdown(); + } + } 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 d29dc44c7..02c24b720 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 @@ -182,7 +182,7 @@ public class TestTSCCMNoServer extends TestCase { assertNull(conn.getRoute()); assertFalse(conn.isOpen()); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); try { conn = getConnection(mgr, null); @@ -226,7 +226,7 @@ public class TestTSCCMNoServer extends TestCase { } // release one of the connections - mgr.releaseConnection(conn2); + mgr.releaseConnection(conn2, -1, null); conn2 = null; // there should be a connection available now @@ -303,13 +303,13 @@ public class TestTSCCMNoServer extends TestCase { // check releaseConnection with invalid arguments try { - mgr.releaseConnection(null); + mgr.releaseConnection(null, -1, null); fail("null connection adapter not detected"); } catch (IllegalArgumentException iax) { // expected } try { - mgr.releaseConnection(new ClientConnAdapterMockup()); + mgr.releaseConnection(new ClientConnAdapterMockup(), -1, null); fail("foreign connection adapter not detected"); } catch (IllegalArgumentException iax) { // expected @@ -367,7 +367,7 @@ public class TestTSCCMNoServer extends TestCase { } // now release one and check that exactly that one can be obtained then - mgr.releaseConnection(conn2); + mgr.releaseConnection(conn2, -1, null); conn2 = null; try { getConnection(mgr, route1, 10L, TimeUnit.MILLISECONDS); @@ -403,7 +403,7 @@ public class TestTSCCMNoServer extends TestCase { mgr.getConnectionsInPool(), 1); assertEquals("connectionsInPool(host)", mgr.getConnectionsInPool(route), 1); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); assertEquals("connectionsInPool", mgr.getConnectionsInPool(), 1); @@ -421,7 +421,6 @@ public class TestTSCCMNoServer extends TestCase { mgr.shutdown(); } - public void testShutdown() throws Exception { // 3.x: TestHttpConnectionManager.testShutdown @@ -449,7 +448,7 @@ public class TestTSCCMNoServer extends TestCase { // First release the connection. If the manager keeps working // despite the shutdown, this will deblock the extra thread. // The release itself should turn into a no-op, without exception. - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); gct.join(10000); @@ -508,7 +507,7 @@ public class TestTSCCMNoServer extends TestCase { // expected } - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); // this time: no exception conn = getConnection(mgr, route, 10L, TimeUnit.MILLISECONDS); assertNotNull("should have gotten a connection", conn); @@ -547,7 +546,7 @@ public class TestTSCCMNoServer extends TestCase { // releasing the connection for route1 should deblock thread1 // the other thread gets a timeout - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); gct1.join(10000); gct2.join(10000); @@ -593,7 +592,7 @@ public class TestTSCCMNoServer extends TestCase { // expected } - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); // this time: no exception conn = getConnection(mgr, route, 10L, TimeUnit.MILLISECONDS); assertNotNull("should have gotten a connection", conn); @@ -635,7 +634,7 @@ public class TestTSCCMNoServer extends TestCase { // expected } - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); // this time: no exception conn = getConnection(mgr, route, 10L, TimeUnit.MILLISECONDS); assertNotNull("should have gotten a connection", conn); 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 6d9aeb9cc..81d6d3621 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 @@ -252,7 +252,7 @@ public class TestTSCCMWithServer extends ServerTestBase { // release connection without marking for re-use // expect the next connection obtained to be closed - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); conn = getConnection(mgr, route); assertFalse("connection should have been closed", conn.isOpen()); @@ -273,7 +273,7 @@ public class TestTSCCMWithServer extends ServerTestBase { // release connection after marking it for re-use // expect the next connection obtained to be open conn.markReusable(); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); conn = getConnection(mgr, route); assertTrue("connection should have been open", conn.isOpen()); @@ -290,10 +290,155 @@ public class TestTSCCMWithServer extends ServerTestBase { rsplen, data.length); // ignore data, but it must be read - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); mgr.shutdown(); } + /** + * Tests releasing with time limits. + */ + public void testReleaseConnectionWithTimeLimits() throws Exception { + + HttpParams mgrpar = defaultParams.copy(); + ConnManagerParams.setMaxTotalConnections(mgrpar, 1); + + ThreadSafeClientConnManager mgr = createTSCCM(mgrpar, null); + + final HttpHost target = getServerHttp(); + final HttpRoute route = new HttpRoute(target, null, false); + final int rsplen = 8; + final String uri = "/random/" + rsplen; + + HttpRequest request = + new BasicHttpRequest("GET", uri, HttpVersion.HTTP_1_1); + + ManagedClientConnection conn = getConnection(mgr, route); + conn.open(route, httpContext, defaultParams); + + // a new context is created for each testcase, no need to reset + HttpResponse response = Helper.execute( + request, conn, target, + httpExecutor, httpProcessor, defaultParams, httpContext); + + assertEquals("wrong status in first response", + HttpStatus.SC_OK, + response.getStatusLine().getStatusCode()); + byte[] data = EntityUtils.toByteArray(response.getEntity()); + assertEquals("wrong length of first response entity", + rsplen, data.length); + // ignore data, but it must be read + + // check that there is no auto-release by default + try { + // this should fail quickly, connection has not been released + getConnection(mgr, route, 10L, TimeUnit.MILLISECONDS); + fail("ConnectionPoolTimeoutException should have been thrown"); + } catch (ConnectionPoolTimeoutException e) { + // expected + } + + // release connection without marking for re-use + // expect the next connection obtained to be closed + mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS); + conn = getConnection(mgr, route); + assertFalse("connection should have been closed", conn.isOpen()); + + // repeat the communication, no need to prepare the request again + conn.open(route, httpContext, defaultParams); + httpContext.setAttribute(ExecutionContext.HTTP_CONNECTION, conn); + response = httpExecutor.execute(request, conn, httpContext); + httpExecutor.postProcess(response, httpProcessor, httpContext); + + assertEquals("wrong status in second response", + HttpStatus.SC_OK, + response.getStatusLine().getStatusCode()); + data = EntityUtils.toByteArray(response.getEntity()); + assertEquals("wrong length of second response entity", + rsplen, data.length); + // ignore data, but it must be read + + // release connection after marking it for re-use + // expect the next connection obtained to be open + conn.markReusable(); + mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS); + conn = getConnection(mgr, route); + assertTrue("connection should have been open", conn.isOpen()); + + // repeat the communication, no need to prepare the request again + httpContext.setAttribute(ExecutionContext.HTTP_CONNECTION, conn); + response = httpExecutor.execute(request, conn, httpContext); + httpExecutor.postProcess(response, httpProcessor, httpContext); + + assertEquals("wrong status in third response", + HttpStatus.SC_OK, + response.getStatusLine().getStatusCode()); + data = EntityUtils.toByteArray(response.getEntity()); + assertEquals("wrong length of third response entity", + rsplen, data.length); + // ignore data, but it must be read + + conn.markReusable(); + mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS); + Thread.sleep(150); + conn = getConnection(mgr, route); + assertTrue("connection should have been closed", !conn.isOpen()); + + // repeat the communication, no need to prepare the request again + conn.open(route, httpContext, defaultParams); + httpContext.setAttribute(ExecutionContext.HTTP_CONNECTION, conn); + response = httpExecutor.execute(request, conn, httpContext); + httpExecutor.postProcess(response, httpProcessor, httpContext); + + assertEquals("wrong status in third response", + HttpStatus.SC_OK, + response.getStatusLine().getStatusCode()); + data = EntityUtils.toByteArray(response.getEntity()); + assertEquals("wrong length of fourth response entity", + rsplen, data.length); + // ignore data, but it must be read + + mgr.shutdown(); + } + + + public void testCloseExpiredConnections() throws Exception { + + HttpParams mgrpar = defaultParams.copy(); + ConnManagerParams.setMaxTotalConnections(mgrpar, 1); + + ThreadSafeClientConnManager mgr = createTSCCM(mgrpar, null); + + final HttpHost target = getServerHttp(); + final HttpRoute route = new HttpRoute(target, null, false); + + ManagedClientConnection conn = getConnection(mgr, route); + conn.open(route, httpContext, defaultParams); + + assertEquals("connectionsInPool", 1, mgr.getConnectionsInPool()); + assertEquals("connectionsInPool(host)", 1, mgr.getConnectionsInPool(route)); + mgr.releaseConnection(conn, 100, TimeUnit.MILLISECONDS); + + // Released, still active. + assertEquals("connectionsInPool", 1, mgr.getConnectionsInPool()); + assertEquals("connectionsInPool(host)", 1, mgr.getConnectionsInPool(route)); + + mgr.closeExpiredConnections(); + + // Time has not expired yet. + assertEquals("connectionsInPool", 1, mgr.getConnectionsInPool()); + assertEquals("connectionsInPool(host)", 1, mgr.getConnectionsInPool(route)); + + Thread.sleep(150); + + mgr.closeExpiredConnections(); + + // Time expired now, connections are destroyed. + assertEquals("connectionsInPool", 0, mgr.getConnectionsInPool()); + assertEquals("connectionsInPool(host)", 0, mgr.getConnectionsInPool(route)); + + mgr.shutdown(); + } + /** * Tests releasing connection from #abort method called from the @@ -343,7 +488,7 @@ public class TestTSCCMWithServer extends ServerTestBase { conn = getConnection(mgr, route, 5L, TimeUnit.SECONDS); assertFalse("connection should have been closed", conn.isOpen()); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); mgr.shutdown(); } @@ -432,7 +577,7 @@ public class TestTSCCMWithServer extends ServerTestBase { // release connection after marking it for re-use conn.markReusable(); - mgr.releaseConnection(conn); + mgr.releaseConnection(conn, -1, null); // We now have a manager with an open connection in it's pool. // We drop all potential hard reference to the manager and check @@ -509,13 +654,13 @@ public class TestTSCCMWithServer extends ServerTestBase { } // return it back to the manager - ((AbstractClientConnAdapter) conn).releaseConnection(); + ((AbstractClientConnAdapter) conn).releaseConnection(-1, null); // the connection is expected to be released back to the manager ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS); assertFalse("connection should have been closed", conn2.isOpen()); - mgr.releaseConnection(conn2); + mgr.releaseConnection(conn2, -1, null); mgr.shutdown(); } @@ -575,13 +720,13 @@ public class TestTSCCMWithServer extends ServerTestBase { } // return it back to the manager - ((AbstractClientConnAdapter) conn).releaseConnection(); + ((AbstractClientConnAdapter) conn).releaseConnection(-1, null); // the connection is expected to be released back to the manager ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS); assertFalse("connection should have been closed", conn2.isOpen()); - mgr.releaseConnection(conn2); + mgr.releaseConnection(conn2, -1, null); mgr.shutdown(); } @@ -646,13 +791,13 @@ public class TestTSCCMWithServer extends ServerTestBase { } // return it back to the manager - ((AbstractClientConnAdapter) conn).releaseConnection(); + ((AbstractClientConnAdapter) conn).releaseConnection(-1, null); // the connection is expected to be released back to the manager ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS); assertFalse("connection should have been closed", conn2.isOpen()); - mgr.releaseConnection(conn2); + mgr.releaseConnection(conn2, -1, null); mgr.shutdown(); } @@ -724,13 +869,13 @@ public class TestTSCCMWithServer extends ServerTestBase { } // return it back to the manager - ((AbstractClientConnAdapter) conn).releaseConnection(); + ((AbstractClientConnAdapter) conn).releaseConnection(-1, null); // the connection is expected to be released back to the manager ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS); assertFalse("connection should have been closed", conn2.isOpen()); - mgr.releaseConnection(conn2); + mgr.releaseConnection(conn2, -1, null); mgr.shutdown(); } diff --git a/module-client/src/test/java/org/apache/http/impl/conn/tsccm/TestConnPoolByRoute.java b/module-client/src/test/java/org/apache/http/impl/conn/tsccm/TestConnPoolByRoute.java index eca767b41..0827c3dce 100644 --- a/module-client/src/test/java/org/apache/http/impl/conn/tsccm/TestConnPoolByRoute.java +++ b/module-client/src/test/java/org/apache/http/impl/conn/tsccm/TestConnPoolByRoute.java @@ -95,7 +95,7 @@ public class TestConnPoolByRoute extends ServerTestBase { } // Free one - connPool.freeEntry(e3, true); + connPool.freeEntry(e3, true, -1, null); // This time the request should succeed PoolEntryRequest r5 = connPool.requestPoolEntry(route, null); @@ -136,9 +136,9 @@ public class TestConnPoolByRoute extends ServerTestBase { e3.setState(Integer.valueOf(3)); // Release entries - connPool.freeEntry(e1, true); - connPool.freeEntry(e2, true); - connPool.freeEntry(e3, true); + connPool.freeEntry(e1, true, -1, null); + connPool.freeEntry(e2, true, -1, null); + connPool.freeEntry(e3, true, -1, null); // Request statefull entries PoolEntryRequest r4 = connPool.requestPoolEntry(route, Integer.valueOf(2)); @@ -160,9 +160,9 @@ public class TestConnPoolByRoute extends ServerTestBase { assertTrue(e6 == e1); // Release entries again - connPool.freeEntry(e4, true); - connPool.freeEntry(e5, true); - connPool.freeEntry(e6, true); + connPool.freeEntry(e4, true, -1, null); + connPool.freeEntry(e5, true, -1, null); + connPool.freeEntry(e6, true, -1, null); // Request an entry with a state not avaialable in the pool PoolEntryRequest r7 = connPool.requestPoolEntry(route, Integer.valueOf(4));