PR classpath/26082

	* gnu/java/net/protocol/http/HTTPConnection.java (pool): Changed to
	type Pool.
	(Pool): New inner class.
	(timeLastUsed): New field.
	(setPool): Changed parameter type to Pool.
	(release): Moved pool management logic to new class Pool.
	* gnu/java/net/protocol/http/HTTPURLConnection.java (connectionPool):
	Removed.
	(maxConnections) : Removed.
	(GetHTTPPropertiesAction.run): Don't initialize maxConnections.
	(getConnection):  Moved pool management logic to HTTPConnection.Pool.

3 files changed, 248 insertions, 57 deletions

diff --git a/ChangeLog b/ChangeLog
index 881722073..da49d44e5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2006-02-24  David Daney  <ddaney@avtrex.com>
+
+	PR classpath/26082
+	* gnu/java/net/protocol/http/HTTPConnection.java (pool): Changed to
+	type Pool.
+	(Pool): New inner class.
+	(timeLastUsed): New field.
+	(setPool): Changed parameter type to Pool.
+	(release): Moved pool management logic to new class Pool.
+	* gnu/java/net/protocol/http/HTTPURLConnection.java (connectionPool):
+	Removed.
+	(maxConnections) : Removed.
+	(GetHTTPPropertiesAction.run): Don't initialize maxConnections.
+	(getConnection):  Moved pool management logic to HTTPConnection.Pool.
+
 2006-02-24  Lillian Angel  <langel@redhat.com>
 
 	* java/awt/Container.java:
diff --git a/gnu/java/net/protocol/http/HTTPConnection.java b/gnu/java/net/protocol/http/HTTPConnection.java
index 573a7918d..4ad87f8c0 100644
--- a/gnu/java/net/protocol/http/HTTPConnection.java
+++ b/gnu/java/net/protocol/http/HTTPConnection.java
@@ -1,5 +1,5 @@
 /* HTTPConnection.java --
-   Copyright (C) 2004, 2005  Free Software Foundation, Inc.
+   Copyright (C) 2004, 2005, 2006  Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
 
@@ -53,7 +53,8 @@ import java.security.GeneralSecurityException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
-import java.util.LinkedHashMap;
+import java.util.LinkedList;
+import java.util.ListIterator;
 import java.util.List;
 import java.util.Map;
 
@@ -164,7 +165,7 @@ public class HTTPConnection
   /**
    * The pool that this connection is a member of (if any).
    */
-  private LinkedHashMap pool;
+  private Pool pool;
 
   /**
    * Creates a new HTTP connection.
@@ -331,27 +332,226 @@ public class HTTPConnection
   }
 
   /**
+   * Manages a pool of HTTPConections.  The pool will have a maximum
+   * size determined by the value of the maxConn parameter passed to
+   * the {@link #get} method.  This value inevitably comes from the
+   * http.maxConnections system property.  If the
+   * classpath.net.http.keepAliveTTL system property is set, that will
+   * be the maximum time (in seconds) that an idle connection will be
+   * maintained.
+   */
+  static class Pool
+  {
+    /**
+     * Singleton instance of the pool.
+     */
+    static Pool instance = new Pool();
+
+    /**
+     * The pool
+     */
+    final LinkedList connectionPool = new LinkedList();
+
+    /**
+     * Maximum size of the pool.
+     */
+    int maxConnections;
+
+    /**
+     * If greater than zero, the maximum time a connection will remain
+     * int the pool.
+     */
+    int connectionTTL;
+
+    /**
+     * A thread that removes connections older than connectionTTL.
+     */
+    class Reaper
+      implements Runnable
+    {
+      public void run()
+      {
+        synchronized (Pool.this)
+          {
+            try
+              {
+                do
+                  {
+                    while (connectionPool.size() > 0)
+                      {
+                        long currentTime = System.currentTimeMillis();
+
+                        HTTPConnection c =
+                          (HTTPConnection)connectionPool.getFirst();
+
+                        long waitTime = c.timeLastUsed
+                          + connectionTTL - currentTime;
+
+                        if (waitTime <= 0)
+                          removeOldest();
+                        else
+                          try
+                            {
+                              Pool.this.wait(waitTime);
+                            }
+                          catch (InterruptedException _)
+                            {
+                              // Ignore the interrupt.
+                            }
+                      }
+                    // After the pool is empty, wait TTL to see if it
+                    // is used again.  This is because in the
+                    // situation where a single thread is making HTTP
+                    // requests to the same server it can remove the
+                    // connection from the pool before the Reaper has
+                    // a chance to start.  This would cause the Reaper
+                    // to exit if it were not for this extra delay.
+                    // The result would be starting a Reaper thread
+                    // for each HTTP request.  With the delay we get
+                    // at most one Reaper created each TTL.
+                    try
+                      {
+                        Pool.this.wait(connectionTTL);
+                      }
+                    catch (InterruptedException _)
+                      {
+                        // Ignore the interrupt.
+                      }
+                  }
+                while (connectionPool.size() > 0);
+              }
+            finally
+              {
+                reaper = null;
+              }
+          }
+      }
+    }
+
+    Reaper reaper;
+
+    /**
+     * Private constructor to ensure singleton.
+     */
+    private Pool()
+    {
+    }
+
+    /**
+     * Tests for a matching connection.
+     *
+     * @param c connection to match.
+     * @param h the host name.
+     * @param p the port.
+     * @param sec true if using https.
+     *
+     * @return true if c matches h, p, and sec.
+     */
+    private static boolean matches(HTTPConnection c,
+                                   String h, int p, boolean sec)
+    {
+      return h.equals(c.hostname) && (p == c.port) && (sec == c.secure);
+    }
+
+    /**
+     * Get a pooled HTTPConnection.  If there is an existing idle
+     * connection to the requested server it is returned.  Otherwise a
+     * new connection is created.
+     *
+     * @param hostname the name of the host to connect to
+     * @param port the port on the host to connect to
+     * @param secure whether to use a secure connection
+     *
+     * @return the HTTPConnection.
+     */
+    synchronized HTTPConnection get(String host,
+                                    int port,
+                                    boolean secure)
+    {
+      String ttl =
+        SystemProperties.getProperty("classpath.net.http.keepAliveTTL");
+      connectionTTL = (ttl != null && ttl.length() > 0) ?
+        1000 * Math.max(1, Integer.parseInt(ttl)) : 10000;
+
+      String mc = SystemProperties.getProperty("http.maxConnections");
+      maxConnections = (mc != null && mc.length() > 0) ?
+        Math.max(Integer.parseInt(mc), 1) : 5;
+      if (maxConnections < 1)
+        maxConnections =  1;
+
+      HTTPConnection c = null;
+      
+      ListIterator it = connectionPool.listIterator(0);
+      while (it.hasNext())
+        {
+          HTTPConnection cc = (HTTPConnection)it.next();
+          if (matches(cc, host, port, secure))
+            {
+              c = cc;
+              break;
+            }
+        }
+      if (c == null)
+        {
+          c = new HTTPConnection(host, port, secure);
+          c.setPool(this);
+        }
+      return c;
+    }
+
+    /**
+     * Put an idle HTTPConnection back into the pool.  If this causes
+     * the pool to be come too large, the oldest connection is removed
+     * and closed.
+     *
+     */
+    synchronized void put(HTTPConnection c)
+    {
+      c.timeLastUsed = System.currentTimeMillis();
+      connectionPool.addLast(c);
+
+      // maxConnections must always be >= 1
+      while (connectionPool.size() >= maxConnections)
+        removeOldest();
+
+      if (connectionTTL > 0 && null == reaper) {
+        // If there is a connectionTTL, then the reaper has removed
+        // any stale connections, so we don't have to check for stale
+        // now.  We do have to start a reaper though, as there is not
+        // one running now.
+        reaper = new Reaper();
+        Thread t = new Thread(reaper, "HTTPConnection.Reaper");
+        t.setDaemon(true);
+        t.start();
+      }
+    }
+
+    /**
+     * Remove the oldest connection from the pool and close it.
+     */
+    void removeOldest()
+    {
+      HTTPConnection cx = (HTTPConnection)connectionPool.removeFirst();
+      try
+        {
+          cx.closeConnection();
+        }
+      catch (IOException ioe)
+        {
+          // Ignore it.  We are just cleaning up.
+        }
+    }
+  }
+  
+  /**
    * The number of times this HTTPConnection has be used via keep-alive.
    */
   int useCount;
 
   /**
-   * Generates a key for connections in the connection pool.
-   *
-   * @param h the host name.
-   * @param p the port.
-   * @param sec true if using https.
-   *
-   * @return the key.
+   * If this HTTPConnection is in the pool, the time it was put there.
    */
-  static Object getPoolKey(String h, int p, boolean sec)
-  {
-    StringBuilder buf = new StringBuilder(sec ? "https://" : "http://");
-    buf.append(h);
-    buf.append(':');
-    buf.append(p);
-    return buf.toString();
-  }
+  long timeLastUsed;
 
   /**
    * Set the connection pool that this HTTPConnection is a member of.
@@ -360,7 +560,7 @@ public class HTTPConnection
    *
    * @param p the pool.
    */
-  void setPool(LinkedHashMap p)
+  void setPool(Pool p)
   {
     pool = p;
   }
@@ -374,25 +574,20 @@ public class HTTPConnection
   {
     if (pool != null)
       {
-        synchronized (pool)
+        useCount++;
+        pool.put(this);
+        
+      }
+    else
+      {
+        // If there is no pool, just close.
+        try
           {
-            useCount++;
-            Object key = HTTPConnection.getPoolKey(hostname, port, secure);
-            pool.put(key, this);
-            while (pool.size() >= HTTPURLConnection.maxConnections)
-              {
-                // maxConnections must always be >= 1
-                Object lru = pool.keySet().iterator().next();
-                HTTPConnection c = (HTTPConnection)pool.remove(lru);
-                try
-                  {
-                    c.closeConnection();
-                  }
-                catch (IOException ioe)
-                  {
-                      // Ignore it.  We are just cleaning up.
-                  }
-              }
+            closeConnection();
+          }
+        catch (IOException ioe)
+          {
+            // Ignore it.  We are just cleaning up.
           }
       }
   }
diff --git a/gnu/java/net/protocol/http/HTTPURLConnection.java b/gnu/java/net/protocol/http/HTTPURLConnection.java
index c598f443b..b28c3809d 100644
--- a/gnu/java/net/protocol/http/HTTPURLConnection.java
+++ b/gnu/java/net/protocol/http/HTTPURLConnection.java
@@ -70,13 +70,6 @@ public class HTTPURLConnection
   extends HttpsURLConnection
   implements HandshakeCompletedListener
 {
-
-  /**
-   * Pool of reusable connections, used if keepAlive is true.
-   */
-  private static final LinkedHashMap connectionPool = new LinkedHashMap();
-  static int maxConnections;
-
   /*
    * The underlying connection.
    */
@@ -134,9 +127,6 @@ public class HTTPURLConnection
       agent = System.getProperty("http.agent");
       String ka = System.getProperty("http.keepAlive");
       keepAlive = !(ka != null && "false".equals(ka));
-      String mc = System.getProperty("http.maxConnections");
-      maxConnections = (mc != null && mc.length() > 0) ?
-        Math.max(Integer.parseInt(mc), 1) : 5;
       return null;
     }
 
@@ -366,16 +356,7 @@ public class HTTPURLConnection
     HTTPConnection connection;
     if (keepAlive)
       {
-        Object key = HTTPConnection.getPoolKey(host, port, secure);
-        synchronized (connectionPool)
-          {
-            connection = (HTTPConnection) connectionPool.remove(key);
-            if (connection == null)
-              {
-                connection = new HTTPConnection(host, port, secure);
-                connection.setPool(connectionPool);
-              }
-          }
+        connection = HTTPConnection.Pool.instance.get(host, port, secure);
       }
     else
       {