simplified keep-alive support. changed ConnectionKeepAliveStrategy to return time in ms, instead of time & timeunit in two different calls. reverted ConnectionReleaseTrigger.releaseConnection to take zero params & instead added a setIdleDuration method to ManagedClientConnection, along with converting direct uses of ClientConnectionManager.releaseConnection to instead go through ManagedClientConnection.releaseConnection (which now uses the idle time when giving itself to the manager).

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@672367 13f79535-47bb-0310-9956-ffa450edef68

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(-1, null);
+            managedConn.releaseConnection();
         }
         return false;
     }
@@ -104,7 +104,7 @@ public class BasicEofSensorWatcher implements EofSensorWatcher {
                 managedConn.markReusable();
             }
         } finally {
-            managedConn.releaseConnection(-1, null);
+            managedConn.releaseConnection();
         }
         return false;
     }

module-client/src/main/java/org/apache/http/conn/BasicManagedEntity.java

@@ -30,10 +30,9 @@
 
 package org.apache.http.conn;
 
-import java.io.InputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.concurrent.TimeUnit;
 
 import org.apache.http.HttpEntity;
 import org.apache.http.entity.HttpEntityWrapper;
@@ -61,12 +60,6 @@ public class BasicManagedEntity extends HttpEntityWrapper
 
     /** Whether to keep the connection alive. */
     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;
 
 
     /**
@@ -81,9 +74,7 @@ public class BasicManagedEntity extends HttpEntityWrapper
      */
     public BasicManagedEntity(HttpEntity entity,
                               ManagedClientConnection conn,
-                              boolean reuse,
+                              boolean reuse) {
-                              long validDuration,
-                              TimeUnit validUnit) {
         super(entity);
 
         if (conn == null)
@@ -92,8 +83,6 @@ public class BasicManagedEntity extends HttpEntityWrapper
 
         this.managedConn = conn;
         this.attemptReuse = reuse;
-        this.validDuration = validDuration;
-        this.validUnit = validUnit;
     }
 
 
@@ -140,7 +129,7 @@ public class BasicManagedEntity extends HttpEntityWrapper
 
 
     // non-javadoc, see interface ConnectionReleaseTrigger
-    public void releaseConnection(long validDuration, TimeUnit timeUnit)
+    public void releaseConnection()
         throws IOException {
 
         this.consumeContent();
@@ -221,11 +210,7 @@ public class BasicManagedEntity extends HttpEntityWrapper
 
         if (managedConn != null) {
             try {
-                // TODO: Should this be subtracting the elapsed time from
+                managedConn.releaseConnection();
-                //       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;
             }

module-client/src/main/java/org/apache/http/conn/ConnectionKeepAliveStrategy.java

@@ -30,8 +30,6 @@
  */
 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;
@@ -48,9 +46,6 @@ import org.apache.http.protocol.HttpContext;
  * @since 4.0
  */
 public interface ConnectionKeepAliveStrategy {
-
-    /** Returns the TimeUnit that this uses for specifying duration. */
-    TimeUnit getTimeUnit();
     
     /**
      * Returns the duration of time which this connection can be safely kept
@@ -68,8 +63,8 @@ public interface ConnectionKeepAliveStrategy {
      * @param context
      *            the context in which the connection is being used.
      * 
-     * @return the duration which it is safe to keep the connection idle,
+     * @return the duration in ms for which it is safe to keep the connection
-     *         or <=0 if no suggested duration.
+     *         idle, or <=0 if no suggested duration.
      */
     long getKeepAliveDuration(HttpResponse response, HttpContext context);
     

module-client/src/main/java/org/apache/http/conn/ConnectionReleaseTrigger.java

@@ -31,7 +31,6 @@
 package org.apache.http.conn;
 
 import java.io.IOException;
-import java.util.concurrent.TimeUnit;
 
 
 /**
@@ -63,16 +62,11 @@ public interface ConnectionReleaseTrigger {
      * {@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(long validDuration, TimeUnit timeUnit)
+    void releaseConnection()
         throws IOException
         ;
 

module-client/src/main/java/org/apache/http/conn/EofSensorInputStream.java

@@ -32,7 +32,6 @@ package org.apache.http.conn;
 
 import java.io.InputStream;
 import java.io.IOException;
-import java.util.concurrent.TimeUnit;
 
 
 /**
@@ -306,7 +305,7 @@ public class EofSensorInputStream extends InputStream
     /**
      * Same as {@link #close close()}.
      */
-    public void releaseConnection(long validDuration, TimeUnit timeUnit) throws IOException {
+    public void releaseConnection() throws IOException {
         this.close();
     }
 

module-client/src/main/java/org/apache/http/conn/ManagedClientConnection.java

@@ -32,6 +32,8 @@
 package org.apache.http.conn;
 
 import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+
 import javax.net.ssl.SSLSession;
 
 import org.apache.http.HttpClientConnection;
@@ -245,5 +247,12 @@ public interface ManagedClientConnection extends
      */
     Object getState()
         ;
+    
+    /**
+     * Sets the duration that this connection can remain idle
+     * before it is reused.
+     * The connection should not be used again if this time elapses.
+     */
+    void setIdleDuration(long duration, TimeUnit unit);
 
 } // interface ManagedClientConnection

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

@@ -298,6 +298,7 @@ public class DefaultClientRequestDirector
         
         int execCount = 0;
         
+        boolean reuse = false;
         HttpResponse response = null;
         boolean done = false;
         try {
@@ -445,11 +446,19 @@ public class DefaultClientRequestDirector
                 response.setParams(params);
                 requestExec.postProcess(response, httpProcessor, context);
                 
+
+                // The connection is in or can be brought to a re-usable state.
+                reuse = reuseStrategy.keepAlive(response, context);
+                if(reuse) {
+                    // Set the idle duration of this connection
+                    long duration = keepAliveStrategy.getKeepAliveDuration(response, context);
+                    managedConn.setIdleDuration(duration, TimeUnit.MILLISECONDS);
+                }
+                
                 RoutedRequest followup = handleResponse(roureq, response, context);
                 if (followup == null) {
                     done = true;
                 } else {
-                    boolean reuse = reuseStrategy.keepAlive(response, context);
                     if (reuse) {
                         LOG.debug("Connection kept alive");
                         // Make sure the response body is fully consumed, if present
@@ -465,7 +474,7 @@ public class DefaultClientRequestDirector
                     }
                     // check if we can use the same connection for the followup
                     if (!followup.getRoute().equals(roureq.getRoute())) {
-                        releaseConnection(response, context);
+                        releaseConnection();
                     }
                     roureq = followup;
                 }
@@ -477,8 +486,6 @@ public class DefaultClientRequestDirector
                 }
             } // while not done
 
-            // The connection is in or can be brought to a re-usable state.
-            boolean reuse = reuseStrategy.keepAlive(response, context);
 
             // check for entity, release connection if possible
             if ((response == null) || (response.getEntity() == null) ||
@@ -486,13 +493,11 @@ public class DefaultClientRequestDirector
                 // connection not needed and (assumed to be) in re-usable state
                 if (reuse)
                     managedConn.markReusable();
-                releaseConnection(response, context);
+                releaseConnection();
             } else {
                 // install an auto-release entity
                 HttpEntity entity = response.getEntity();
-                long duration = keepAliveStrategy.getKeepAliveDuration(response, context);
+                entity = new BasicManagedEntity(entity, managedConn, reuse);
-                TimeUnit unit = keepAliveStrategy.getTimeUnit();
-                entity = new BasicManagedEntity(entity, managedConn, reuse, duration, unit);
                 response.setEntity(entity);
             }
 
@@ -515,10 +520,15 @@ public class DefaultClientRequestDirector
      * and prepares for retrieving a new connection during
      * the next request.
      */
-    protected void releaseConnection(HttpResponse response, HttpContext context) {
+    protected void releaseConnection() {
-        long duration = keepAliveStrategy.getKeepAliveDuration(response, context);
+        // Release the connection through the ManagedConnection instead of the
-        TimeUnit unit = keepAliveStrategy.getTimeUnit();
+        // ConnectionManager directly.  This lets the connection control how
-        connManager.releaseConnection(managedConn, duration, unit);
+        // it is released.
+        try {
+            managedConn.releaseConnection();
+        } catch(IOException ignored) {
+            LOG.debug("IOException releasing connection", ignored);
+        }
         managedConn = null;
     }
 
@@ -980,10 +990,8 @@ public class DefaultClientRequestDirector
      * Shuts down the connection.
      * This method is called from a <code>catch</code> block in
      * {@link #execute execute} during exception handling.
-     *
-     * @throws IOException      in case of an IO problem
      */
-    private void abortConnection() throws IOException {
+    private void abortConnection() {
         ManagedClientConnection mcc = managedConn;
         if (mcc != null) {
             // we got here as the result of an exception
@@ -997,7 +1005,11 @@ public class DefaultClientRequestDirector
                 }
             }
             // ensure the connection manager properly releases this connection
-            connManager.releaseConnection(mcc, -1, null);
+            try {
+                mcc.releaseConnection();
+            } catch(IOException ignored) {
+                LOG.debug("Error releasing connection", ignored);
+            }
         }
     } // abortConnection
 

module-client/src/main/java/org/apache/http/impl/client/DefaultConnectionKeepAliveStrategy.java

@@ -30,8 +30,6 @@
  */
 package org.apache.http.impl.client;
 
-import java.util.concurrent.TimeUnit;
-
 import org.apache.http.HeaderElement;
 import org.apache.http.HeaderElementIterator;
 import org.apache.http.HttpResponse;
@@ -67,16 +65,12 @@ public class DefaultConnectionKeepAliveStrategy implements ConnectionKeepAliveSt
             String value = he.getValue();
             if (value != null && param.equalsIgnoreCase("timeout")) {
                 try {
-                    return Long.parseLong(value);
+                    return Long.parseLong(value) * 1000;
                 } catch(NumberFormatException ignore) {
                 }
             }
         }
         return -1;
     }
-    
-    public TimeUnit getTimeUnit() {
-        return TimeUnit.SECONDS;
-    }
 
 }

module-client/src/main/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java

@@ -98,6 +98,12 @@ public abstract class AbstractClientConnAdapter
 
     /** True if the connection has been aborted. */
     private volatile boolean aborted;
+    
+    /** The last time this processed incoming headers. */
+    private volatile long lastHeadersRead;
+    
+    /** The duration this is valid for while idle (in ms). */
+    private volatile long duration;
 
     /**
      * Creates a new connection adapter.
@@ -115,7 +121,7 @@ public abstract class AbstractClientConnAdapter
         wrappedConnection = conn;
         markedReusable = false;
         aborted = false;
-
+        duration = Long.MAX_VALUE;
     } // <constructor>
 
 
@@ -126,6 +132,7 @@ public abstract class AbstractClientConnAdapter
     protected void detach() {
         wrappedConnection = null;
         connManager = null; // base class attribute
+        duration = Long.MAX_VALUE;
     }
 
     protected OperatedClientConnection getWrappedConnection() {
@@ -252,6 +259,8 @@ public abstract class AbstractClientConnAdapter
         assertValid(conn);
 
         unmarkReusable();
+        duration = Long.MAX_VALUE; // Reset duration per request.
+        lastHeadersRead = System.currentTimeMillis();
         return conn.receiveResponseHeader();
     }
 
@@ -347,11 +356,45 @@ public abstract class AbstractClientConnAdapter
     public boolean isMarkedReusable() {
         return markedReusable;
     }
+    
+    public void setIdleDuration(long duration, TimeUnit unit) {
+        if(duration > 0) {
+            this.duration = unit.toMillis(duration);
+        } else {
+            this.duration = Long.MAX_VALUE;
+        }
+    }
+    
+    /**
+     * Returns the amount of time remaining that this connection
+     * can be reused.  If negative, the connection should not
+     * be reused.  If Long.MAX_VALUE, there is no suggested
+     * duration.
+     * 
+     * The remaining time is the elapsed time between now,
+     * the time the last headers were processed, and the duration
+     * given from {@link #setIdleDuration(long, TimeUnit)}.
+     */
+    protected long getRemainingIdleDuration() {
+        if(duration == Long.MAX_VALUE) {
+            return Long.MAX_VALUE;
+        } else {
+            long elapsedAlready = System.currentTimeMillis() - lastHeadersRead;
+            return duration - elapsedAlready;
+        }
+    }
 
     // non-javadoc, see interface ConnectionReleaseTrigger
-    public void releaseConnection(long validDuration, TimeUnit timeUnit) {
+    public void releaseConnection() {
-        if (connManager != null)
+        if (connManager != null) {
-            connManager.releaseConnection(this, validDuration, timeUnit);
+            long remainingTime = getRemainingIdleDuration();
+            if(remainingTime <= 0) {
+                unmarkReusable();
+            } else if(remainingTime == Long.MAX_VALUE) {
+                remainingTime = -1;
+            }
+            connManager.releaseConnection(this, remainingTime, TimeUnit.MILLISECONDS);
+        }
     }
 
     // non-javadoc, see interface ConnectionReleaseTrigger
@@ -379,7 +422,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(-1, null);
+            releaseConnection();
         }
     }
 

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

@@ -379,7 +379,7 @@ public class TestDefaultClientRequestDirector extends ServerTestBase {
                         if(!awaitLatch.await(timeout, tunit))
                             throw new ConnectionPoolTimeoutException();
                         
-                        return new ClientConnAdapterMockup();
+                        return new ClientConnAdapterMockup(ConnMan4.this);
                     }
                 };  
             } else {
@@ -452,7 +452,7 @@ public class TestDefaultClientRequestDirector extends ServerTestBase {
                         long timeout, TimeUnit unit)
                         throws InterruptedException,
                         ConnectionPoolTimeoutException {
-                    allocatedConnection = new ClientConnAdapterMockup() {
+                    allocatedConnection = new ClientConnAdapterMockup(ConnMan2.this) {
                         @Override
                         public void open(HttpRoute route, HttpContext context,
                                 HttpParams params) throws IOException {
@@ -535,7 +535,7 @@ public class TestDefaultClientRequestDirector extends ServerTestBase {
                     if(!awaitLatch.await(timeout, tunit))
                         throw new ConnectionPoolTimeoutException();
                     
-                    return new ClientConnAdapterMockup();
+                    return new ClientConnAdapterMockup(ConMan.this);
                 }
             };
         }

module-client/src/test/java/org/apache/http/impl/client/TestDefaultConnKeepAliveStrategy.java

@@ -1,7 +1,7 @@
 /*
- * $HeadURL: $
+ * $HeadURL$
- * $Revision: $
+ * $Revision$
- * $Date: $
+ * $Date$
  * ====================================================================
  *
  *  Licensed to the Apache Software Foundation (ASF) under one or more
@@ -115,7 +115,7 @@ public class TestDefaultConnKeepAliveStrategy extends TestCase {
         response.addHeader("Keep-Alive", "timeout=300, max=20");
         ConnectionKeepAliveStrategy keepAliveStrat = new DefaultConnectionKeepAliveStrategy();
         long d = keepAliveStrat.getKeepAliveDuration(response, context);
-        assertEquals(300, d);
+        assertEquals(300000, d);
     }
 
 }

module-client/src/test/java/org/apache/http/impl/conn/ClientConnAdapterMockup.java

@@ -33,6 +33,7 @@ package org.apache.http.impl.conn;
 import java.io.IOException;
 
 import org.apache.http.HttpHost;
+import org.apache.http.conn.ClientConnectionManager;
 import org.apache.http.conn.routing.HttpRoute;
 import org.apache.http.params.HttpParams;
 import org.apache.http.protocol.HttpContext;
@@ -43,8 +44,8 @@ import org.apache.http.protocol.HttpContext;
  */
 public class ClientConnAdapterMockup extends AbstractClientConnAdapter {
 
-    public ClientConnAdapterMockup() {
+    public ClientConnAdapterMockup(ClientConnectionManager mgr) {
-        super(null, null);
+        super(mgr, null);
     }
 
     public void close() {

module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMNoServer.java

@@ -309,7 +309,7 @@ public class TestTSCCMNoServer extends TestCase {
             // expected
         }
         try {
-            mgr.releaseConnection(new ClientConnAdapterMockup(), -1, null);
+            mgr.releaseConnection(new ClientConnAdapterMockup(null), -1, null);
             fail("foreign connection adapter not detected");
         } catch (IllegalArgumentException iax) {
             // expected

module-client/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java

@@ -654,7 +654,7 @@ public class TestTSCCMWithServer extends ServerTestBase {
         }
 
         // return it back to the manager
-        ((AbstractClientConnAdapter) conn).releaseConnection(-1, null);
+        ((AbstractClientConnAdapter) conn).releaseConnection();
         
         // the connection is expected to be released back to the manager
         ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS);
@@ -720,7 +720,7 @@ public class TestTSCCMWithServer extends ServerTestBase {
         }
 
         // return it back to the manager
-        ((AbstractClientConnAdapter) conn).releaseConnection(-1, null);
+        ((AbstractClientConnAdapter) conn).releaseConnection();
         
         // the connection is expected to be released back to the manager
         ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS);
@@ -791,7 +791,7 @@ public class TestTSCCMWithServer extends ServerTestBase {
         }
 
         // return it back to the manager
-        ((AbstractClientConnAdapter) conn).releaseConnection(-1, null);
+        ((AbstractClientConnAdapter) conn).releaseConnection();
         
         // the connection is expected to be released back to the manager
         ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS);
@@ -869,7 +869,7 @@ public class TestTSCCMWithServer extends ServerTestBase {
         }
 
         // return it back to the manager
-        ((AbstractClientConnAdapter) conn).releaseConnection(-1, null);
+        ((AbstractClientConnAdapter) conn).releaseConnection();
         
         // the connection is expected to be released back to the manager
         ManagedClientConnection conn2 = getConnection(mgr, route, 5L, TimeUnit.SECONDS);