[prev in list] [next in list] [prev in thread] [next in thread] 

List:       httpcomponents-commits
Subject:    svn commit: r560288 - in /jakarta/httpcomponents/httpclient/trunk: ./
From:       rolandw () apache ! org
Date:       2007-07-27 15:49:46
Message-ID: 20070727154946.CFC6D1A981A () eris ! apache ! org
[Download RAW message or body]

Author: rolandw
Date: Fri Jul 27 08:49:45 2007
New Revision: 560288

URL: http://svn.apache.org/viewvc?view=rev&rev=560288
Log:
HTTPCLIENT-636: BadStaticMaps are gone

Removed:
    jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/BadStaticMaps.java
 Modified:
    jakarta/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
    jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java
  jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java
  jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
  jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java


Modified: jakarta/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?view=diff&rev=560288&r1=560287&r2=560288
 ==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ jakarta/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Fri Jul 27 08:49:45 \
2007 @@ -1,5 +1,8 @@
 Changes since release 4.0 Alpha 1
 
+* [HTTPCLIENT-636] refactor ThreadSafeClientConnManager in separate package
+  Contributed by Roland Weber <rolandw at apache.org>
+
 * [HTTPCLIENT-669] new HttpRoutePlanner interface and implementation
   Contributed by Andrea Selva <selva.andre at gmail.com>
 

Modified: jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java
                
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/module-clien \
t/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java?view=diff&rev=560288&r1=560287&r2=560288
 ==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java \
                (original)
+++ jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java \
Fri Jul 27 08:49:45 2007 @@ -33,6 +33,7 @@
 import java.io.IOException;
 import java.lang.ref.Reference;
 import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
 import java.util.Set;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -40,6 +41,7 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.http.conn.ClientConnectionOperator;
+import org.apache.http.conn.ClientConnectionManager;
 import org.apache.http.conn.ConnectionPoolTimeoutException;
 import org.apache.http.conn.HttpRoute;
 import org.apache.http.conn.OperatedClientConnection;
@@ -78,15 +80,17 @@
     protected HttpParams params;
 
 
-    /** The connection manager. */
-    //@@@ replace with a weak reference to allow for GC
-    //@@@ is it necessary to have the manager in the pool entry?
-    protected ThreadSafeClientConnManager connManager;
+    /**
+     * The connection manager.
+     * This weak reference is used only to detect garbage collection
+     * of the manager. The connection pool MUST NOT keep a hard reference
+     * to the manager, or else the manager might never be GCed.
+     */
+    protected ConnMgrRef connManager;
 
 
     /** A reference queue to track loss of pool entries to GC. */
-    //@@@ this should be a pool-specific reference queue
-    protected ReferenceQueue refQueue = BadStaticMaps.REFERENCE_QUEUE; //@@@
+    protected ReferenceQueue refQueue;
 
     /** A worker (thread) to track loss of pool entries to GC. */
     private RefQueueWorker refWorker;
@@ -96,6 +100,23 @@
     protected volatile boolean isShutDown;
 
 
+    /**
+     * A weak reference to the connection manager, to detect GC.
+     */
+    private static class ConnMgrRef extends WeakReference {
+
+        /**
+         * Creates a new reference.
+         *
+         * @param ccmgr   the connection manager
+         * @param queue   the reference queue, or <code>null</code>
+         */
+        public ConnMgrRef(ClientConnectionManager ccmgr,
+                          ReferenceQueue queue) {
+            super(ccmgr, queue);
+        }
+    }
+
 
     /**
      * Creates a new connection pool.
@@ -104,7 +125,6 @@
      */
     protected AbstractConnPool(ThreadSafeClientConnManager tsccm) {
 
-        connManager = tsccm;
         params = tsccm.getParams();
 
         issuedConnections = new HashSet();
@@ -112,7 +132,7 @@
 
         //@@@ currently must be false, otherwise the TSCCM
         //@@@ will not be garbage collected in the unit test...
-        boolean conngc = false; //@@@ check parameters to decide
+        boolean conngc = true; //@@@ check parameters to decide
         if (conngc) {
             refQueue = new ReferenceQueue();
             refWorker = new RefQueueWorker(refQueue, this);
@@ -121,6 +141,8 @@
             t.setName("RefQueueWorker@" + this);
             t.start();
         }
+
+        connManager = new ConnMgrRef(tsccm, refQueue);
     }
 
 
@@ -172,8 +194,12 @@
                 }
                 handleLostEntry(route);
             }
+        } else if (ref instanceof ConnMgrRef) {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Connection manager garbage collected. ");
+            }
+            shutdown();
         }
-        //@@@ else check if the connection manager was GCed
     }
 
 
@@ -213,7 +239,8 @@
      */
     public synchronized void shutdown() {
 
-        isShutDown = true;
+        if (isShutDown)
+            return;
 
         // no point in monitoring GC anymore
         if (refWorker != null)
@@ -229,12 +256,12 @@
                 closeConnection(entry.getConnection());
             }
         }
-        //@@@ while the static map exists, call there to clean it up
-        BadStaticMaps.shutdownCheckedOutConnections(this); //@@@
 
         // remove all references to connections
         //@@@ use this for shutting them down instead?
         idleConnHandler.removeAll();
+
+        isShutDown = true;
     }
 
 

Modified: jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java
                
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/module-clien \
t/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java?view=diff&rev=560288&r1=560287&r2=560288
 ==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java \
                (original)
+++ jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java \
Fri Jul 27 08:49:45 2007 @@ -46,10 +46,12 @@
  */
 public class BasicPoolEntry extends AbstractPoolEntry {
 
-    /** The connection manager. */
-    private ThreadSafeClientConnManager manager;
-    //@@@ do we actually need the manager, or can we live with the pool alone?
-    //@@@ references to the manager will prevent it's GC
+    /** The connection pool. */
+    private AbstractConnPool connPool;
+
+    /** The connection operator. */
+    //@@@ move to base class, drop getOperator()?
+    private ClientConnectionOperator connOperator;
 
 
     /**
@@ -63,36 +65,36 @@
     /**
      * Creates a new pool entry.
      *
-     * @param tsccm   the connection manager
-     * @param occ     the underlying connection for this entry
+     * @param pool    the connection pool
+     * @param op      the connection operator
      * @param route   the planned route for the connection
      * @param queue   the reference queue for tracking GC of this entry,
      *                or <code>null</code>
      */
-    public BasicPoolEntry(ThreadSafeClientConnManager tsccm,
-                          OperatedClientConnection occ,
+    public BasicPoolEntry(AbstractConnPool pool,
+                          ClientConnectionOperator op,
                           HttpRoute route,
                           ReferenceQueue queue) {
-        super(occ, route);
-        if (tsccm == null) {
+        //@@@ create connection in base? or delay creation until needed?
+        super(op.createConnection(), route);
+        if (pool == null) {
             throw new IllegalArgumentException
-                ("Connection manager must not be null.");
+                ("Connection pool must not be null.");
         }
         if (route == null) {
             throw new IllegalArgumentException
                 ("Planned route must not be null.");
         }
 
-        this.manager = tsccm;
+        this.connPool = pool;
+        this.connOperator = op;
         this.reference = new BasicPoolEntryRef(this, queue);
     }
 
 
     // non-javadoc, see base AbstractPoolEntry
     protected ClientConnectionOperator getOperator() {
-        //@@@ pass the operator explicitly to the constructor
-        //@@@ or provide getter in the connection manager
-        return manager.connOperator;
+        return this.connOperator;
     }
 
 
@@ -108,8 +110,8 @@
         return this.reference;
     }
 
-    protected final ThreadSafeClientConnManager getManager() {
-        return this.manager;
+    protected final AbstractConnPool getConnPool() {
+        return this.connPool;
     }
 
 

Modified: jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
                
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/module-clien \
t/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java?view=diff&rev=560288&r1=560287&r2=560288
 ==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java \
                (original)
+++ jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java \
Fri Jul 27 08:49:45 2007 @@ -41,7 +41,6 @@
 import org.apache.http.conn.ClientConnectionOperator;
 import org.apache.http.conn.ConnectionPoolTimeoutException;
 import org.apache.http.conn.HttpRoute;
-import org.apache.http.conn.OperatedClientConnection;
 import org.apache.http.conn.params.HttpConnectionManagerParams;
 
 
@@ -192,8 +191,6 @@
 
 
     // non-javadoc, see base class AbstractConnPool
-    //@@@ can we keep the operator out of here and simply return a
-    //@@@ pool entry without a connection, to be filled in by the manager?
     public synchronized
         BasicPoolEntry getEntry(HttpRoute route, long timeout,
                                 ClientConnectionOperator operator)
@@ -323,10 +320,12 @@
             return;
         }
 
+        // no longer issued, we keep a hard reference now
+        issuedConnections.remove(entry.getWeakRef());
+
         RouteConnPool routePool = getRoutePool(route);
 
-        // Put the connection back in the available list
-        // and notify a waiter
+        // put the connection back in the available list and notify a waiter
         routePool.freeConnections.add(entry);
         if (routePool.numConnections == 0) {
             // for some reason the route pool didn't already exist
@@ -335,11 +334,6 @@
         }
         freeConnections.add(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.
-        BadStaticMaps.removeReferenceToConnection(entry); //@@@
-        issuedConnections.remove(entry.getWeakRef()); //@@@ move above
         if (numConnections == 0) {
             // for some reason this pool didn't already exist
             LOG.error("Master connection pool not found. " + route);
@@ -369,18 +363,15 @@
         RouteConnPool routePool = getRoutePool(route);
 
         if (routePool.freeConnections.size() > 0) {
-            entry = (BasicPoolEntry) routePool.freeConnections.removeLast();
-            freeConnections.remove(entry);
-
-            // store a reference to this entry so that it can be cleaned up
-            // in the event it is not correctly released
-            BadStaticMaps.storeReferenceToConnection(entry, route, this); //@@@
-            issuedConnections.add(entry.getWeakRef());
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Getting free connection. " + route);
             }
+            entry = (BasicPoolEntry) routePool.freeConnections.removeLast();
+            freeConnections.remove(entry);
             idleConnHandler.remove(entry.getConnection()); // no longer idle
 
+            issuedConnections.add(entry.getWeakRef());
+
         } else {
             if (LOG.isDebugEnabled()) {
                 LOG.debug("No free connections. " + route);
@@ -409,15 +400,11 @@
             LOG.debug("Creating new connection. " + route);
         }
 
-        OperatedClientConnection conn = op.createConnection();
-        BasicPoolEntry entry = new BasicPoolEntry
-            (connManager, conn, route, refQueue);
+        // the entry will create the connection when needed
+        BasicPoolEntry entry = new BasicPoolEntry(this, op, 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
-        BadStaticMaps.storeReferenceToConnection(entry, route, this); //@@@
         issuedConnections.add(entry.getWeakRef());
 
         return entry;

Modified: jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java
                
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/module-clien \
t/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java?view=diff&rev=560288&r1=560287&r2=560288
 ==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java \
                (original)
+++ jakarta/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java \
Fri Jul 27 08:49:45 2007 @@ -79,7 +79,8 @@
 
 
     /** The pool of connections being managed. */
-    private AbstractConnPool connectionPool;
+    //@@@ temporarily, used in BasicPoolEntry
+    /*private*/ AbstractConnPool connectionPool;
 
     /** The operator for opening and updating connections. */
     /*private*/ ClientConnectionOperator connOperator;
@@ -108,9 +109,9 @@
         this.connOperator = createConnectionOperator(schreg);
         this.isShutDown = false;
 
-        synchronized(BadStaticMaps.ALL_CONNECTION_MANAGERS) {
-            BadStaticMaps.ALL_CONNECTION_MANAGERS.put(this, null);
-        }
+        //@@@ synchronized(BadStaticMaps.ALL_CONNECTION_MANAGERS) {
+        //@@@    BadStaticMaps.ALL_CONNECTION_MANAGERS.put(this, null);
+        //@@@}
     } // <constructor>
 
 
@@ -234,8 +235,7 @@
      * @param entry     the pool entry for the connection to release,
      *                  or <code>null</code>
      */
-    //@@@ temporary default visibility, for BadStaticMaps
-    void /*default*/ releasePoolEntry(BasicPoolEntry entry) {
+    private void releasePoolEntry(BasicPoolEntry entry) {
 
         if (entry == null)
             return;
@@ -245,14 +245,15 @@
 
 
 
-    /**
+    /* *
      * Shuts down all instances of this class.
      *
      * @deprecated no replacement
-     */
+     * /
     public static void shutdownAll() {
-        BadStaticMaps.shutdownAll();
+        //@@@ BadStaticMaps.shutdownAll();
     }
+    */
 
 
     /**


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic