HTTPCLIENT-636: towards getting rid of static maps

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

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

@@ -96,8 +96,8 @@
 proxy can be tunnelled, and a secure protocol (TLS/SSL)
 might be put on top of the tunnel.
 The {@link org.apache.http.conn.RouteTracker RouteTracker}
-helps in tracking the steps for establishing a route, while a
+helps in tracking the steps for establishing a route, while an
-{@link org.apache.http.conn.RouteDirector RouteDirector}
+{@link org.apache.http.conn.HttpRouteDirector HttpRouteDirector}
 determines the next step to take.
 </p>
 

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

@@ -71,9 +71,11 @@ public abstract class AbstractPoolEntry {
     /** The underlying connection being pooled or used. */
     protected OperatedClientConnection connection;
 
-    //@@@ keep the planned route as HttpRoute when TSHCM is restructured
+    /** The route for which this entry gets allocated. */
-    //@@@ /* * The route for which this entry gets allocated. */
+    //@@@ currently accessed from connection manager(s) as attribute
-    //@@@ private HostConfiguration plannedRoute;
+    //@@@ avoid that, derived classes should decide whether update is allowed
+    //@@@ SCCM: yes, TSCCM: no
+    protected HttpRoute plannedRoute;
 
     /** The tracked route, or <code>null</code> before tracking starts. */
     protected RouteTracker tracker;
@@ -82,10 +84,14 @@ public abstract class AbstractPoolEntry {
     /**
      * Creates a new pool entry.
      *
-     * @param occ   the underlying connection for this entry
+     * @param occ     the underlying connection for this entry
+     * @param route   the planned route for the connection,
+     *                or <code>null</code>
      */
-    protected AbstractPoolEntry(OperatedClientConnection occ) {
+    protected AbstractPoolEntry(OperatedClientConnection occ,
+                                HttpRoute route) {
         this.connection = occ;
+        this.plannedRoute = route;
         this.tracker = null;
     }
 

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

@@ -346,18 +346,13 @@ protected void revokeConnection() {
      */
     protected class PoolEntry extends AbstractPoolEntry {
 
-        //@@@ move to base class when TSCCM is cleaned up
-        /** The route for which this entry gets allocated. */
-        protected HttpRoute plannedRoute;
-
-            
         /**
          * Creates a new pool entry.
          *
          * @param occ   the underlying connection for this entry
          */
         protected PoolEntry(OperatedClientConnection occ) {
-            super(occ);
+            super(occ, null);
         }
 
 

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

@@ -35,7 +35,9 @@
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
+import java.util.Set;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.Map;
@@ -559,10 +561,6 @@ void shutdownCheckedOutConnections(ConnectionPool connectionPool) {
         for (Iterator i = connectionsToClose.iterator(); i.hasNext();) {
             TrackingPoolEntry entry = (TrackingPoolEntry) i.next();
             closeConnection(entry.connection);
-            // remove the reference to the connection manager. this ensures
-            // that the we don't accidentally end up here again
-            //@@@ connection.setHttpConnectionManager(null);
-
             entry.manager.releasePoolEntry(entry);
         }
     }
@@ -677,8 +675,21 @@ private class ConnectionPool {
         private LinkedList waitingThreads = new LinkedList();
 
         /**
-         * Map where keys are {@link HttpRoute}s and values are
+         * References to issued connections.
-         * {@link RouteConnPool}s
+         * Objects in this set are of class {@link PoolEntryRef PoolEntryRef},
+         * and point to the pool entry for the issued connection.
+         * GCed connections are detected by the missing pool entries.
+         */
+        private Set issuedConnections = new HashSet();
+
+        /** A reference queue to track loss of pool entries to GC. */
+        //@@@ this should be a pool-specific reference queue
+        private ReferenceQueue refQueue = REFERENCE_QUEUE; //@@@
+
+        /**
+         * Map of route-specific pools.
+         * Keys are of class {@link HttpRoute}, and
+         * values are of class {@link RouteConnPool}.
          */
         private final Map mapRoutes = new HashMap();
 
@@ -701,7 +712,17 @@ public synchronized void shutdown() {
             }
             
             // close all connections that have been checked out
-            shutdownCheckedOutConnections(this);
+            iter = issuedConnections.iterator();
+            while (iter.hasNext()) {
+                PoolEntryRef per = (PoolEntryRef) iter.next();
+                iter.remove();
+                TrackingPoolEntry entry = (TrackingPoolEntry) per.get();
+                if (entry != null) {
+                    closeConnection(entry.connection);
+                }
+            }
+            //@@@ while the static map exists, call there to clean it up
+            shutdownCheckedOutConnections(this); //@@@
             
             // interrupt all waiting threads
             iter = waitingThreads.iterator();
@@ -737,14 +758,16 @@ TrackingPoolEntry createEntry(HttpRoute route,
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Allocating new connection, route=" + route);
             }
-            TrackingPoolEntry entry = new TrackingPoolEntry(conn);
+            TrackingPoolEntry entry = new TrackingPoolEntry
-            entry.plannedRoute = route;
+                (ThreadSafeClientConnManager.this, conn, route, refQueue);
             numConnections++;
             routePool.numConnections++;
     
             // store a reference to this entry so that it can be cleaned up
             // in the event it is not correctly released
-            storeReferenceToConnection(entry, route, this);
+            storeReferenceToConnection(entry, route, this); //@@@
+            issuedConnections.add(entry.reference);
+
             return entry;
         }
 
@@ -811,7 +834,8 @@ public synchronized TrackingPoolEntry getFreeConnection(HttpRoute route) {
 
                 // store a reference to this entry so that it can be cleaned up
                 // in the event it is not correctly released
-                storeReferenceToConnection(entry, route, this);
+                storeReferenceToConnection(entry, route, this); //@@@
+                issuedConnections.add(entry.reference);
                 if (LOG.isDebugEnabled()) {
                     LOG.debug("Getting free connection, route=" + route);
                 }
@@ -978,7 +1002,7 @@ private void freeConnection(TrackingPoolEntry entry) {
                 // and notify a waiter
                 routePool.freeConnections.add(entry);
                 if (routePool.numConnections == 0) {
-                    // for some reason this pool didn't already exist
+                    // for some reason the route pool didn't already exist
                     LOG.error("Route connection pool not found. " + route);
                     routePool.numConnections = 1;
                 }
@@ -987,10 +1011,11 @@ private void freeConnection(TrackingPoolEntry entry) {
                 // We can remove the reference to this connection as we have
                 // control over it again. This also ensures that the connection
                 // manager can be GCed.
-                removeReferenceToConnection(entry);
+                removeReferenceToConnection(entry); //@@@
+                issuedConnections.remove(entry.reference); //@@@ move up
                 if (numConnections == 0) {
                     // for some reason this pool didn't already exist
-                    LOG.error("Route connection pool not found. " + route);
+                    LOG.error("Master connection pool not found. " + route);
                     numConnections = 1;
                 }
 
@@ -1004,10 +1029,12 @@ private void freeConnection(TrackingPoolEntry entry) {
 
 
     private static void closeConnection(final OperatedClientConnection conn) {
-        try {
+        if (conn != null) {
-            conn.close();
+            try {
-        } catch (IOException ex) {
+                conn.close();
-            LOG.debug("I/O error closing connection", ex);
+            } catch (IOException ex) {
+                LOG.debug("I/O error closing connection", ex);
+            }
         }
     }
 
@@ -1016,36 +1043,36 @@ private static void closeConnection(final OperatedClientConnection conn) {
      * a connection's resources when claimed by the garbage collector.
      */
     private static class ConnectionSource {
-        
+
         /** The connection pool that created the connection */
         public ConnectionPool connectionPool;
 
         /** The connection's planned route. */
         public HttpRoute route;
     }
-    
+
     /**
      * A simple struct-like class to combine the connection list and the count
      * of created connections.
      */
     private static class RouteConnPool {
 
-        /** The route this pool is for */
+        /** The route this pool is for. */
         public HttpRoute route;
-        
+
-        /** The list of free connections */
+        /** The list of free connections. */
         public LinkedList freeConnections = new LinkedList();
-        
+
         /** The list of WaitingThreads for this pool. */
         public LinkedList waitingThreads = new LinkedList();
 
-        /** The number of created connections */
+        /** The number of created connections. */
         public int numConnections = 0;
     }
-    
+
+
     /**
-     * A simple struct-like class to combine the waiting thread and the connection 
+     * A thread and the pool in which it is waiting.
-     * pool it is waiting on.
      */
     private static class WaitingThread {
         /** The thread that is waiting for a connection */
@@ -1131,47 +1158,111 @@ public void run() {
 
     } // class ReferenceQueueThread
 
+
+    /**
+     * A weak reference to a pool entry.
+     * Needed for connection GC.
+     * Instances of this class can be kept as static references.
+     * If an issued connection is lost to garbage collection,
+     * it's pool entry is GC'd and the reference becomes invalid.
+     */
+    private static class PoolEntryRef extends WeakReference {
+
+        /** The planned route of the entry. */
+        private final HttpRoute route;
+
+
+        /**
+         * Creates a new reference to a pool entry.
+         *
+         * @param entry   the pool entry, must not be <code>null</code>
+         * @param queue   the reference queue, or <code>null</code>
+         */
+        public PoolEntryRef(AbstractPoolEntry entry, ReferenceQueue queue) {
+            super(entry, queue);
+            if (entry == null) {
+                throw new IllegalArgumentException
+                    ("Pool entry must not be null.");
+            }
+            route = entry.plannedRoute;
+        }
+
+
+        /**
+         * Indicates whether this reference is still valid.
+         *
+         * @return <code>true</code> if the pool entry is still referenced,
+         *         <code>false</code> otherwise
+         */
+        public final boolean isValid() {
+            return (super.get() != null);
+        }
+
+
+        /**
+         * Obtain the planned route for the referenced entry.
+         * The planned route is still available, even if the entry is gone.
+         *
+         * @return      the planned route
+         */
+        public final HttpRoute getRoute() {
+            return this.route;
+        }
+        
+    } // class PoolEntryRef
+
     
     /**
      * A pool entry representing a connection that tracks it's route.
      * For historical reasons, these entries are sometimes referred to
      * as <i>connections</i> throughout the code.
      */
-    private class TrackingPoolEntry extends AbstractPoolEntry {
+    private static class TrackingPoolEntry extends AbstractPoolEntry {
-
-        //@@@ move to base class
-        /** The route for which this entry gets allocated. */
-        private HttpRoute plannedRoute;
 
         /** The connection manager. */
         private ThreadSafeClientConnManager manager;
 
 
         /**
-         * A weak reference used to detect GCed pool entries.
+         * A weak reference to <code>this</code> used to detect GC of entries.
          * Of course, pool entries can only be GCed when they are allocated
          * and therefore not referenced with a hard link in the manager.
          */
-        private WeakReference reference;
+        private PoolEntryRef reference;
 
             
         /**
          * Creates a new pool entry.
          *
-         * @param occ   the underlying connection for this entry
+         * @param tsccm   the connection manager
+         * @param occ     the underlying connection for this entry
+         * @param route   the planned route for the connection
+         * @param queue   the reference queue for tracking GC of this entry,
+         *                or <code>null</code>
          */
-        private TrackingPoolEntry(OperatedClientConnection occ) {
+        private TrackingPoolEntry(ThreadSafeClientConnManager tsccm,
-            super(occ);
+                                  OperatedClientConnection occ,
-            //@@@ pass planned route to the constructor?
+                                  HttpRoute route,
-            //@@@ or update when the adapter is created?
+                                  ReferenceQueue queue) {
-            this.manager = ThreadSafeClientConnManager.this;
+            super(occ, route);
-            this.reference = new WeakReference(this, REFERENCE_QUEUE);
+            if (tsccm == null) {
+                throw new IllegalArgumentException
+                    ("Connection manager must not be null.");
+            }
+            if (route == null) {
+                throw new IllegalArgumentException
+                    ("Planned route must not be null.");
+            }
+            this.manager = tsccm;
+            this.reference = new PoolEntryRef(this, queue);
         }
 
 
         // non-javadoc, see base AbstractPoolEntry
         protected ClientConnectionOperator getOperator() {
-            return ThreadSafeClientConnManager.this.connOperator;
+            //@@@ if the operator is passed explicitly to the constructor,
+            //@@@ this class can be factored out
+            return manager.connOperator;
         }