Return-Path: Delivered-To: apmail-jakarta-httpcomponents-commits-archive@www.apache.org Received: (qmail 22717 invoked from network); 27 Jul 2007 15:50:08 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 27 Jul 2007 15:50:08 -0000 Received: (qmail 50208 invoked by uid 500); 27 Jul 2007 15:50:08 -0000 Delivered-To: apmail-jakarta-httpcomponents-commits-archive@jakarta.apache.org Received: (qmail 50190 invoked by uid 500); 27 Jul 2007 15:50:08 -0000 Mailing-List: contact httpcomponents-commits-help@jakarta.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: httpcomponents-dev@jakarta.apache.org Delivered-To: mailing list httpcomponents-commits@jakarta.apache.org Received: (qmail 50180 invoked by uid 99); 27 Jul 2007 15:50:08 -0000 Received: from Unknown (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 27 Jul 2007 08:50:08 -0700 X-ASF-Spam-Status: No, hits=-100.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO eris.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 27 Jul 2007 15:50:07 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id CFC6D1A981A; Fri, 27 Jul 2007 08:49:46 -0700 (PDT) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r560288 - in /jakarta/httpcomponents/httpclient/trunk: ./ module-client/src/main/java/org/apache/http/impl/conn/tsccm/ Date: Fri, 27 Jul 2007 15:49:46 -0000 To: httpcomponents-commits@jakarta.apache.org From: rolandw@apache.org X-Mailer: svnmailer-1.1.0 Message-Id: <20070727154946.CFC6D1A981A@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 + * [HTTPCLIENT-669] new HttpRoutePlanner interface and implementation Contributed by Andrea Selva 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-client/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 null + */ + 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-client/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 null */ - 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-client/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-client/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); + //@@@} } // @@ -234,8 +235,7 @@ * @param entry the pool entry for the connection to release, * or null */ - //@@@ 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(); } + */ /**