433689 Evict idle HttpDestinations from client

3 files changed, 112 insertions, 19 deletions

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java
index 80f5536890..1d32bbf6d8 100644
--- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java
+++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java
@@ -21,11 +21,14 @@ package org.eclipse.jetty.client;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.UnknownHostException;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.Enumeration;
 import java.util.LinkedList;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+
 import javax.net.ssl.SSLContext;
 
 import org.eclipse.jetty.client.security.Authentication;
@@ -79,6 +82,7 @@ public class HttpClient extends AggregateLifeCycle implements HttpBuffers, Attri
     private int _connectorType = CONNECTOR_SELECT_CHANNEL;
     private boolean _useDirectBuffers = true;
     private boolean _connectBlocking = true;
+    private boolean _removeIdleDestinations=false;
     private int _maxConnectionsPerAddress = Integer.MAX_VALUE;
     private int _maxQueueSizePerAddress = Integer.MAX_VALUE;
     private ConcurrentMap<Address, HttpDestination> _destinations = new ConcurrentHashMap<Address, HttpDestination>();
@@ -157,6 +161,18 @@ public class HttpClient extends AggregateLifeCycle implements HttpBuffers, Attri
     }
 
     /* ------------------------------------------------------------------------------- */
+    public boolean isRemoveIdleDestinations()
+    {
+        return _removeIdleDestinations;
+    }
+
+    /* ------------------------------------------------------------------------------- */
+    public void setRemoveIdleDestinations(boolean removeIdleDestinations)
+    {
+        _removeIdleDestinations = removeIdleDestinations;
+    }
+
+    /* ------------------------------------------------------------------------------- */
     public void send(HttpExchange exchange) throws IOException
     {
         boolean ssl = HttpSchemes.HTTPS_BUFFER.equalsIgnoreCase(exchange.getScheme());
@@ -262,7 +278,19 @@ public class HttpClient extends AggregateLifeCycle implements HttpBuffers, Attri
         }
         return destination;
     }
-
+    
+    /* ------------------------------------------------------------------------------- */
+    public Collection<Address> getDestinations()
+    {
+        return Collections.unmodifiableCollection(_destinations.keySet());
+    }
+    
+    /* ------------------------------------------------------------------------------- */
+    public void removeDestination(HttpDestination destination)
+    {
+        _destinations.remove(destination.getAddress(),destination);
+    }
+    
     /* ------------------------------------------------------------ */
     public void schedule(Timeout.Task task)
     {
@@ -908,4 +936,5 @@ public class HttpClient extends AggregateLifeCycle implements HttpBuffers, Attri
     private static class LocalQueuedThreadPool extends QueuedThreadPool
     {
     }
+
 }
diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java
index 4e551cd51a..6c2d5474bf 100644
--- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java
+++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java
@@ -169,6 +169,15 @@ public class HttpDestination implements Dumpable
 
         // TODO query, remove and age methods
     }
+    
+    public void clearCookies()
+    {
+        synchronized (this)
+        {
+            _cookies.clear();
+        }
+
+    }
 
     /**
      * Get a connection. We either get an idle connection if one is available, or
@@ -442,15 +451,20 @@ public class HttpDestination implements Dumpable
         else
         {
             boolean startConnection = false;
+            boolean remove = false;
             synchronized (this)
             {
                 _connections.remove(connection);
-                if (!_exchanges.isEmpty())
+                if (_exchanges.isEmpty())
+                    remove=_client.isRemoveIdleDestinations() && ( _cookies==null || _cookies.isEmpty() ) &&_connections.isEmpty() && _idleConnections.isEmpty();
+                else if (_client.isStarted())
                     startConnection = true;
             }
 
             if (startConnection)
                 startNewConnection();
+            if (remove)
+                _client.removeDestination(this);
         }
     }
 
@@ -461,17 +475,22 @@ public class HttpDestination implements Dumpable
         connection.onIdleExpired(idleForMs);
 
         boolean startConnection = false;
+        boolean remove = false;
         synchronized (this)
         {
             _idleConnections.remove(connection);
             _connections.remove(connection);
 
-            if (!_exchanges.isEmpty() && _client.isStarted())
+            if (_exchanges.isEmpty())
+                remove=_client.isRemoveIdleDestinations() && ( _cookies==null || _cookies.isEmpty() ) &&_connections.isEmpty() && _idleConnections.isEmpty();
+            else if (_client.isStarted())
                 startConnection = true;
         }
 
         if (startConnection)
             startNewConnection();
+        if (remove)
+            _client.removeDestination(this);
     }
 
     public void send(HttpExchange ex) throws IOException
@@ -524,21 +543,24 @@ public class HttpDestination implements Dumpable
     {
         // add cookies
         // TODO handle max-age etc.
-        if (_cookies != null)
+        synchronized (this)
         {
-            StringBuilder buf = null;
-            for (HttpCookie cookie : _cookies)
+            if (_cookies != null)
             {
-                if (buf == null)
-                    buf = new StringBuilder();
-                else
-                    buf.append("; ");
-                buf.append(cookie.getName()); // TODO quotes
-                buf.append("=");
-                buf.append(cookie.getValue()); // TODO quotes
-            }
-            if (buf != null)
-                ex.addRequestHeader(HttpHeaders.COOKIE, buf.toString());
+                StringBuilder buf = null;
+                for (HttpCookie cookie : _cookies)
+                {
+                    if (buf == null)
+                        buf = new StringBuilder();
+                    else
+                        buf.append("; ");
+                    buf.append(cookie.getName()); // TODO quotes
+                    buf.append("=");
+                    buf.append(cookie.getValue()); // TODO quotes
+                }
+                if (buf != null)
+                    ex.addRequestHeader(HttpHeaders.COOKIE, buf.toString());
+            }  
         }
 
         // Add any known authorizations
diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ExpireTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ExpireTest.java
index 68e4b1952b..90c4476c89 100644
--- a/jetty-client/src/test/java/org/eclipse/jetty/client/ExpireTest.java
+++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ExpireTest.java
@@ -33,6 +33,7 @@ import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.eclipse.jetty.server.nio.SelectChannelConnector;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -58,10 +59,12 @@ public class ExpireTest
             public void handle(String target, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse)
                     throws IOException, ServletException
             {
+                httpResponse.setStatus(200);
                 request.setHandled(true);
                 try
                 {
-                    Thread.sleep(2000);
+                    if (request.getRequestURI().contains("/sleep"))
+                        Thread.sleep(2000);
                 }
                 catch (InterruptedException x)
                 {
@@ -76,7 +79,6 @@ public class ExpireTest
         client.setTimeout(200);
         client.setMaxRetries(0);
         client.setMaxConnectionsPerAddress(100);
-        client.start();
     }
 
     @After
@@ -90,7 +92,10 @@ public class ExpireTest
     @Test
     public void testExpire() throws Exception
     {
-        String baseUrl = "http://" + "localhost" + ":" + port + "/";
+        client.setIdleTimeout(5000);
+        client.start();
+        
+        String baseUrl = "http://" + "localhost" + ":" + port + "/sleep";
 
         int count = 200;
         final CountDownLatch expires = new CountDownLatch(count);
@@ -114,4 +119,41 @@ public class ExpireTest
         // Wait to be sure that all exchanges have expired
         assertTrue(expires.await(5, TimeUnit.SECONDS));
     }
+    
+    @Test
+    public void testRemoveIdleDestination() throws Exception
+    {
+        client.setIdleTimeout(200);
+        client.setRemoveIdleDestinations(true);
+        client.start();
+        
+        String baseUrl = "http://" + "localhost" + ":" + port + "/other";
+
+        int count = 5;
+        final CountDownLatch latch = new CountDownLatch(count);
+
+        for (int i=0;i<count;i++)
+        {
+            final ContentExchange exchange = new ContentExchange()
+            {
+                @Override
+                protected void onResponseComplete()
+                {
+                    latch.countDown();
+                }
+            };
+            exchange.setMethod("GET");
+            exchange.setURL(baseUrl);
+
+            client.send(exchange);
+        }
+
+        // Wait to be sure that all exchanges have expired
+        assertTrue(latch.await(1, TimeUnit.SECONDS));
+        
+        Assert.assertEquals(1,client.getDestinations().size());
+        Thread.sleep(500);
+        Assert.assertEquals(0,client.getDestinations().size());
+                
+    }
 }