You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2008/04/17 12:00:46 UTC

svn commit: r649035 - in /httpcomponents/httpclient/trunk/module-client/src: main/java/org/apache/http/impl/conn/ main/java/org/apache/http/impl/conn/tsccm/ test/java/org/apache/http/impl/conn/tsccm/

Author: olegk
Date: Thu Apr 17 03:00:45 2008
New Revision: 649035

URL: http://svn.apache.org/viewvc?rev=649035&view=rev
Log:
Refactoring of the TSCCM code:
* Removed code maintaining a weak reference to the parent connection manager in the AbstractConnPool. The weak reference to the connection manager is currently used to shut down the pool when the connection manager gets GC-ed. The same net result can be achieved by calling #shutdown from #finalize() of the connection manager class.
* Shut down connection managers in #finalize().
* There is no point passing ClientConnectionOperator around. It can be assigned to the connection pool once at the construction time.

Modified:
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/PoolEntryRequest.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java
    httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/tsccm/TestSpuriousWakeup.java

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java?rev=649035&r1=649034&r2=649035&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/SingleClientConnManager.java Thu Apr 17 03:00:45 2008
@@ -127,6 +127,13 @@
     } // <constructor>
 
 
+    @Override
+    protected void finalize() throws Throwable {
+        shutdown();
+        super.finalize();
+    }
+
+
     // non-javadoc, see interface ClientConnectionManager
     public SchemeRegistry getSchemeRegistry() {
         return this.schemeRegistry;

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java?rev=649035&r1=649034&r2=649035&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java Thu Apr 17 03:00:45 2008
@@ -33,7 +33,6 @@
 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;
@@ -43,15 +42,12 @@
 
 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.OperatedClientConnection;
 import org.apache.http.conn.routing.HttpRoute;
 import org.apache.http.impl.conn.IdleConnectionHandler;
 
 
-
 /**
  * An abstract connection pool.
  * It is used by the {@link ThreadSafeClientConnManager}.
@@ -86,15 +82,6 @@
     protected int numConnections;
 
     /**
-     * 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.
      * The same queue is used to track loss of the connection manager,
      * so we cannot specialize the type.
@@ -108,39 +95,17 @@
     /** Indicates whether this pool is shut down. */
     protected volatile boolean isShutDown;
 
-
-    /**
-     * A weak reference to the connection manager, to detect GC.
-     */
-    private static class ConnMgrRef
-        extends WeakReference<ClientConnectionManager> {
-
-        /**
-         * Creates a new reference.
-         *
-         * @param ccmgr   the connection manager
-         * @param queue   the reference queue, or <code>null</code>
-         */
-        public ConnMgrRef(ClientConnectionManager ccmgr,
-                          ReferenceQueue<Object> queue) {
-            super(ccmgr, queue);
-        }
-    }
-
-
     /**
      * Creates a new connection pool.
      *
      * @param mgr   the connection manager
      */
-    protected AbstractConnPool(ClientConnectionManager mgr) {
+    protected AbstractConnPool() {
         issuedConnections = new HashSet<BasicPoolEntryRef>();
         idleConnHandler = new IdleConnectionHandler();
 
         boolean fair = false; //@@@ check parameters to decide
         poolLock = new ReentrantLock(fair);
-
-        connManager = new ConnMgrRef(mgr, null);
     }
 
 
@@ -176,8 +141,6 @@
         t.setDaemon(true);
         t.setName("RefQueueWorker@" + this);
         t.start();
-
-        connManager = new ConnMgrRef(connManager.get(), refQueue);
     }
 
 
@@ -203,10 +166,9 @@
                 HttpRoute route, 
                 Object state,
                 long timeout, 
-                TimeUnit tunit,
-                ClientConnectionOperator operator)
+                TimeUnit tunit)
                     throws ConnectionPoolTimeoutException, InterruptedException {
-        return newPoolEntryRequest().getPoolEntry(route, state, timeout, tunit, operator);
+        return newPoolEntryRequest().getPoolEntry(route, state, timeout, tunit);
     }
     
     /**
@@ -248,11 +210,6 @@
                     }
                     handleLostEntry(route);
                 }
-            } else if (ref instanceof ConnMgrRef) {
-                if (LOG.isDebugEnabled()) {
-                    LOG.debug("Connection manager garbage collected.");
-                }
-                shutdown();
             }
 
         } finally {

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java?rev=649035&r1=649034&r2=649035&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java Thu Apr 17 03:00:45 2008
@@ -42,7 +42,6 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.http.conn.routing.HttpRoute;
-import org.apache.http.conn.ClientConnectionManager;
 import org.apache.http.conn.ClientConnectionOperator;
 import org.apache.http.conn.ConnectionPoolTimeoutException;
 import org.apache.http.conn.params.ConnPerRoute;
@@ -50,7 +49,6 @@
 import org.apache.http.params.HttpParams;
 
 
-
 /**
  * A connection pool that maintains connections by route.
  * This class is derived from <code>MultiThreadedHttpConnectionManager</code>
@@ -73,7 +71,9 @@
     //@@@ use a protected LOG in the base class?
     private final Log LOG = LogFactory.getLog(ConnPoolByRoute.class);
 
-
+    /** Connection operator for this pool */
+    protected final ClientConnectionOperator operator;
+    
     /** The list of free connections */
     protected Queue<BasicPoolEntry> freeConnections;
 
@@ -96,8 +96,13 @@
      *
      * @param mgr   the connection manager
      */
-    public ConnPoolByRoute(final ClientConnectionManager mgr, final HttpParams params) {
-        super(mgr);
+    public ConnPoolByRoute(final ClientConnectionOperator operator, final HttpParams params) {
+        super();
+        if (operator == null) {
+            throw new IllegalArgumentException("Connection operator may not be null");
+        }
+        this.operator = operator;
+        
         freeConnections = createFreeConnQueue();
         waitingThreads  = createWaitingThreadQueue();
         routeToPool     = createRouteToPoolMap();
@@ -231,10 +236,9 @@
                     HttpRoute route,
                     Object state,
                     long timeout,
-                    TimeUnit tunit, 
-                    ClientConnectionOperator operator)
+                    TimeUnit tunit)
                         throws InterruptedException, ConnectionPoolTimeoutException {
-                return getEntryBlocking(route, state, timeout, tunit, operator, aborter);
+                return getEntryBlocking(route, state, timeout, tunit, aborter);
             }
             
         };
@@ -263,7 +267,6 @@
     protected BasicPoolEntry getEntryBlocking(
                                    HttpRoute route, Object state,
                                    long timeout, TimeUnit tunit,
-                                   ClientConnectionOperator operator,
                                    Aborter aborter)
         throws ConnectionPoolTimeoutException, InterruptedException {
 

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/PoolEntryRequest.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/PoolEntryRequest.java?rev=649035&r1=649034&r2=649035&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/PoolEntryRequest.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/PoolEntryRequest.java Thu Apr 17 03:00:45 2008
@@ -50,8 +50,6 @@
      * @param timeout   the timeout, 0 or negative for no timeout
      * @param tunit     the unit for the <code>timeout</code>,
      *                  may be <code>null</code> only if there is no timeout
-     * @param operator  the connection operator, in case
-     *                  a connection has to be created
      *
      * @return  pool entry holding a connection for the route
      *
@@ -64,9 +62,7 @@
             HttpRoute route, 
             Object state,
             long timeout, 
-            TimeUnit unit,
-            ClientConnectionOperator operator) 
-                throws InterruptedException, ConnectionPoolTimeoutException;
+            TimeUnit unit) throws InterruptedException, ConnectionPoolTimeoutException;
 
     /**
      * Aborts the active or next call to

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java?rev=649035&r1=649034&r2=649035&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java Thu Apr 17 03:00:45 2008
@@ -96,11 +96,18 @@
             throw new IllegalArgumentException("HTTP parameters may not be null");
         }
         this.schemeRegistry = schreg;
-        this.connectionPool = createConnectionPool(params);
         this.connOperator   = createConnectionOperator(schreg);
+        this.connectionPool = createConnectionPool(params);
 
     } // <constructor>
 
+    
+    @Override
+    protected void finalize() throws Throwable {
+        shutdown();
+        super.finalize();
+    }
+
 
     /**
      * Hook for creating the connection pool.
@@ -109,7 +116,7 @@
      */
     protected AbstractConnPool createConnectionPool(final HttpParams params) {
 
-        AbstractConnPool acp = new ConnPoolByRoute(this, params);
+        AbstractConnPool acp = new ConnPoolByRoute(connOperator, params);
         boolean conngc = true; //@@@ check parameters to decide
         if (conngc) {
             acp.enableConnectionGC();
@@ -166,7 +173,7 @@
                 }
 
                 final BasicPoolEntry entry = poolRequest.getPoolEntry(
-                        route, state, timeout, tunit, connOperator);
+                        route, state, timeout, tunit);
 
                 return new BasicPooledConnAdapter(ThreadSafeClientConnManager.this, entry);
             }

Modified: httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/tsccm/TestSpuriousWakeup.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/tsccm/TestSpuriousWakeup.java?rev=649035&r1=649034&r2=649035&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/tsccm/TestSpuriousWakeup.java (original)
+++ httpcomponents/httpclient/trunk/module-client/src/test/java/org/apache/http/impl/conn/tsccm/TestSpuriousWakeup.java Thu Apr 17 03:00:45 2008
@@ -39,9 +39,9 @@
 
 import org.apache.http.HttpHost;
 import org.apache.http.HttpVersion;
+import org.apache.http.conn.ClientConnectionOperator;
 import org.apache.http.conn.ClientConnectionRequest;
 import org.apache.http.conn.ManagedClientConnection;
-import org.apache.http.conn.ClientConnectionManager;
 import org.apache.http.conn.routing.HttpRoute;
 import org.apache.http.conn.scheme.PlainSocketFactory;
 import org.apache.http.conn.scheme.Scheme;
@@ -97,8 +97,8 @@
         protected WaitingThread newestWT;
 
 
-        public XConnPoolByRoute(ClientConnectionManager mgr, HttpParams params) {
-            super(mgr, params);
+        public XConnPoolByRoute(ClientConnectionOperator operator, HttpParams params) {
+            super(operator, params);
         }
 
         protected synchronized
@@ -126,7 +126,7 @@
         }
 
         protected AbstractConnPool createConnectionPool(HttpParams params) {
-            extendedCPBR = new XConnPoolByRoute(this, params);
+            extendedCPBR = new XConnPoolByRoute(connOperator, params);
             // no connection GC required
             return extendedCPBR;
         }