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.


git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@649035 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Oleg Kalnichevski 2008-04-17 10:00:45 +00:00
parent 5a08148d25
commit 2b91201275
6 changed files with 37 additions and 67 deletions

View File

@ -127,6 +127,13 @@ public class SingleClientConnManager implements ClientConnectionManager {
} // <constructor>
@Override
protected void finalize() throws Throwable {
shutdown();
super.finalize();
}
// non-javadoc, see interface ClientConnectionManager
public SchemeRegistry getSchemeRegistry() {
return this.schemeRegistry;

View File

@ -33,7 +33,6 @@ package org.apache.http.impl.conn.tsccm;
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 java.util.concurrent.locks.ReentrantLock;
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}.
@ -85,15 +81,6 @@ public abstract class AbstractConnPool implements RefQueueHandler {
/** The current total number of connections. */
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,
@ -108,39 +95,17 @@ public abstract class AbstractConnPool implements RefQueueHandler {
/** 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 @@ public abstract class AbstractConnPool implements RefQueueHandler {
t.setDaemon(true);
t.setName("RefQueueWorker@" + this);
t.start();
connManager = new ConnMgrRef(connManager.get(), refQueue);
}
@ -203,10 +166,9 @@ public abstract class AbstractConnPool implements RefQueueHandler {
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 @@ public abstract class AbstractConnPool implements RefQueueHandler {
}
handleLostEntry(route);
}
} else if (ref instanceof ConnMgrRef) {
if (LOG.isDebugEnabled()) {
LOG.debug("Connection manager garbage collected.");
}
shutdown();
}
} finally {

View File

@ -42,7 +42,6 @@ import java.util.concurrent.TimeUnit;
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.conn.params.HttpConnectionManagerParams;
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 @@ public class ConnPoolByRoute extends AbstractConnPool {
//@@@ 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 @@ public class ConnPoolByRoute extends AbstractConnPool {
*
* @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 @@ public class ConnPoolByRoute extends AbstractConnPool {
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 @@ public class ConnPoolByRoute extends AbstractConnPool {
protected BasicPoolEntry getEntryBlocking(
HttpRoute route, Object state,
long timeout, TimeUnit tunit,
ClientConnectionOperator operator,
Aborter aborter)
throws ConnectionPoolTimeoutException, InterruptedException {

View File

@ -50,8 +50,6 @@ public interface PoolEntryRequest {
* @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 @@ public interface PoolEntryRequest {
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

View File

@ -96,11 +96,18 @@ public class ThreadSafeClientConnManager
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 @@ public class ThreadSafeClientConnManager
*/
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 @@ public class ThreadSafeClientConnManager
}
final BasicPoolEntry entry = poolRequest.getPoolEntry(
route, state, timeout, tunit, connOperator);
route, state, timeout, tunit);
return new BasicPooledConnAdapter(ThreadSafeClientConnManager.this, entry);
}

View File

@ -39,9 +39,9 @@ import junit.framework.TestSuite;
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 @@ public class TestSpuriousWakeup extends TestCase {
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 @@ public class TestSpuriousWakeup extends TestCase {
}
protected AbstractConnPool createConnectionPool(HttpParams params) {
extendedCPBR = new XConnPoolByRoute(this, params);
extendedCPBR = new XConnPoolByRoute(connOperator, params);
// no connection GC required
return extendedCPBR;
}