Index: httpclient/src/test/java/org/apache/http/impl/conn/tsccm/TestDumbHelpers.java =================================================================== --- httpclient/src/test/java/org/apache/http/impl/conn/tsccm/TestDumbHelpers.java (revision 781936) +++ httpclient/src/test/java/org/apache/http/impl/conn/tsccm/TestDumbHelpers.java (working copy) @@ -1,137 +0,0 @@ -/* - * $HeadURL$ - * $Revision$ - * $Date$ - * ==================================================================== - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * ==================================================================== - * - * This software consists of voluntary contributions made by many - * individuals on behalf of the Apache Software Foundation. For more - * information on the Apache Software Foundation, please see - * . - * - */ - -package org.apache.http.impl.conn.tsccm; - -/* -import java.util.Date; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.Condition; -import java.util.concurrent.locks.ReentrantLock; -*/ - -import junit.framework.Test; -import junit.framework.TestCase; -import junit.framework.TestSuite; - - -import org.apache.http.HttpHost; -import org.apache.http.conn.routing.HttpRoute; -import org.apache.http.conn.scheme.PlainSocketFactory; -import org.apache.http.conn.scheme.Scheme; -import org.apache.http.conn.scheme.SchemeRegistry; -import org.apache.http.conn.scheme.SocketFactory; -import org.apache.http.conn.ClientConnectionOperator; -import org.apache.http.impl.conn.DefaultClientConnectionOperator; - - - -/** - * Tests for simple helper classes without advanced functionality. - */ -public class TestDumbHelpers extends TestCase { - - public final static - HttpHost TARGET = new HttpHost("target.test.invalid"); - - - /** The default scheme registry. */ - private SchemeRegistry supportedSchemes; - - - - public TestDumbHelpers(String testName) { - super(testName); - } - - public static void main(String args[]) { - String[] testCaseName = { TestDumbHelpers.class.getName() }; - junit.textui.TestRunner.main(testCaseName); - } - - public static Test suite() { - return new TestSuite(TestDumbHelpers.class); - } - - - @Override - protected void setUp() { - supportedSchemes = new SchemeRegistry(); - SocketFactory sf = PlainSocketFactory.getSocketFactory(); - supportedSchemes.register(new Scheme("http", sf, 80)); - } - - - - - public void testBasicPoolEntry() { - HttpRoute route = new HttpRoute(TARGET); - ClientConnectionOperator ccop = - new DefaultClientConnectionOperator(supportedSchemes); - - BasicPoolEntry bpe = null; - try { - bpe = new BasicPoolEntry(null, null, null); - fail("null operator not detected"); - } catch (NullPointerException npx) { - // expected - } catch (IllegalArgumentException iax) { - // would be preferred - } - - try { - bpe = new BasicPoolEntry(ccop, null, null); - fail("null route not detected"); - } catch (IllegalArgumentException iax) { - // expected - } - - bpe = new BasicPoolEntry(ccop, route, null); - assertEquals ("wrong route", route, bpe.getPlannedRoute()); - assertNotNull("missing ref", bpe.getWeakRef()); - - assertEquals("bad weak ref", bpe, bpe.getWeakRef().get()); - assertEquals("bad ref route", route, bpe.getWeakRef().getRoute()); - } - - - public void testBasicPoolEntryRef() { - // the actual reference is tested implicitly with BasicPoolEntry - // but we need to cover the argument check in the constructor - try { - new BasicPoolEntryRef(null, null); - fail("null pool entry not detected"); - } catch (IllegalArgumentException iax) { - // expected - } - } - - -} // class TestDumbHelpers Index: httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java =================================================================== --- httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java (revision 781936) +++ httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java (working copy) @@ -493,65 +493,6 @@ } /** - * Tests GC of an unreferenced connection. - */ - public void testConnectionGC() throws Exception { - // 3.x: TestHttpConnectionManager.testReclaimUnusedConnection - - HttpParams mgrpar = defaultParams.copy(); - ConnManagerParams.setMaxTotalConnections(mgrpar, 1); - - ThreadSafeClientConnManager mgr = createTSCCM(mgrpar, null); - - final HttpHost target = getServerHttp(); - final HttpRoute route = new HttpRoute(target, null, false); - final int rsplen = 8; - final String uri = "/random/" + rsplen; - - HttpRequest request = - new BasicHttpRequest("GET", uri, HttpVersion.HTTP_1_1); - - ManagedClientConnection conn = getConnection(mgr, route); - conn.open(route, httpContext, defaultParams); - - // a new context is created for each testcase, no need to reset - Helper.execute(request, conn, target, - httpExecutor, httpProcessor, defaultParams, httpContext); - - // we leave the connection in mid-use - // it's not really re-usable, but it must be closed anyway - conn.markReusable(); - - // first check that we can't get another connection - try { - // this should fail quickly, connection has not been released - getConnection(mgr, route, 10L, TimeUnit.MILLISECONDS); - fail("ConnectionPoolTimeoutException should have been thrown"); - } catch (ConnectionPoolTimeoutException e) { - // expected - } - - // We now drop the hard references to the connection and trigger GC. - WeakReference wref = - new WeakReference(conn); - conn = null; - httpContext = null; // holds a reference to the connection - - // Java does not guarantee that this will trigger the GC, but - // it does in the test environment. GC is asynchronous, so we - // need to give the garbage collector some time afterwards. - System.gc(); - Thread.sleep(1000); - - assertNull("connection not garbage collected", wref.get()); - conn = getConnection(mgr, route, 10L, TimeUnit.MILLISECONDS); - assertFalse("GCed connection not closed", conn.isOpen()); - - mgr.shutdown(); - } - - - /** * Tests GC of an unreferenced connection manager. */ public void testConnectionManagerGC() throws Exception { Index: httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueHandler.java =================================================================== --- httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueHandler.java (revision 781936) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueHandler.java (working copy) @@ -1,50 +0,0 @@ -/* - * $HeadURL$ - * $Revision$ - * $Date$ - * - * ==================================================================== - * - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * ==================================================================== - * - * This software consists of voluntary contributions made by many - * individuals on behalf of the Apache Software Foundation. For more - * information on the Apache Software Foundation, please see - * . - * - */ - -package org.apache.http.impl.conn.tsccm; - -import java.lang.ref.Reference; - - -/** - * Callback handler for {@link RefQueueWorker RefQueueWorker}. - * - * @since 4.0 - */ -public interface RefQueueHandler { - - /** - * Invoked when a reference is found on the queue. - * - * @param ref the reference to handle - */ - public void handleReference(Reference ref) - ; -} Index: httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java =================================================================== --- httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java (revision 781936) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java (working copy) @@ -381,7 +381,7 @@ } // no longer issued, we keep a hard reference now - issuedConnections.remove(entry.getWeakRef()); + issuedConnections.remove(entry); RouteSpecificPool rospl = getRoutePool(route, true); @@ -448,7 +448,7 @@ rospl.dropEntry(); numConnections--; } else { - issuedConnections.add(entry.getWeakRef()); + issuedConnections.add(entry); done = true; } @@ -487,7 +487,7 @@ // the entry will create the connection when needed BasicPoolEntry entry = - new BasicPoolEntry(op, rospl.getRoute(), refQueue); + new BasicPoolEntry(op, rospl.getRoute()); poolLock.lock(); try { @@ -495,7 +495,7 @@ rospl.createdEntry(entry); numConnections++; - issuedConnections.add(entry.getWeakRef()); + issuedConnections.add(entry); } finally { poolLock.unlock(); Index: httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntryRef.java =================================================================== --- httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntryRef.java (revision 781936) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntryRef.java (working copy) @@ -1,85 +0,0 @@ -/* - * $HeadURL$ - * $Revision$ - * $Date$ - * - * ==================================================================== - * - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * ==================================================================== - * - * This software consists of voluntary contributions made by many - * individuals on behalf of the Apache Software Foundation. For more - * information on the Apache Software Foundation, please see - * . - * - */ - -package org.apache.http.impl.conn.tsccm; - - -import java.lang.ref.WeakReference; -import java.lang.ref.ReferenceQueue; - -import net.jcip.annotations.Immutable; - -import org.apache.http.conn.routing.HttpRoute; - - - -/** - * A weak reference to a {@link BasicPoolEntry BasicPoolEntry}. - * This reference explicitly keeps the planned route, so the connection - * can be reclaimed if it is lost to garbage collection. - * - * @since 4.0 - */ -@Immutable -public class BasicPoolEntryRef extends WeakReference { - - /** The planned route of the entry. */ - private final HttpRoute route; // HttpRoute is @Immutable - - - /** - * Creates a new reference to a pool entry. - * - * @param entry the pool entry, must not be null - * @param queue the reference queue, or null - */ - public BasicPoolEntryRef(BasicPoolEntry entry, - ReferenceQueue queue) { - super(entry, queue); - if (entry == null) { - throw new IllegalArgumentException - ("Pool entry must not be null."); - } - route = entry.getPlannedRoute(); - } - - - /** - * 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 BasicPoolEntryRef - Index: httpclient/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java =================================================================== --- httpclient/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java (revision 781936) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java (working copy) @@ -31,8 +31,6 @@ package org.apache.http.impl.conn.tsccm; import java.io.IOException; -import java.lang.ref.Reference; -import java.lang.ref.ReferenceQueue; import java.util.Set; import java.util.HashSet; import java.util.Iterator; @@ -41,7 +39,7 @@ import java.util.concurrent.locks.ReentrantLock; import net.jcip.annotations.GuardedBy; -import net.jcip.annotations.NotThreadSafe; +import net.jcip.annotations.ThreadSafe; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -50,7 +48,6 @@ 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}. @@ -60,8 +57,8 @@ * * @since 4.0 */ -@NotThreadSafe // unsynch access to refQueue, refWorker -public abstract class AbstractConnPool implements RefQueueHandler { +@ThreadSafe +public abstract class AbstractConnPool { private final Log log = LogFactory.getLog(getClass()); @@ -80,7 +77,7 @@ * Must hold poolLock when accessing. */ @GuardedBy("poolLock") - protected Set issuedConnections; + protected Set issuedConnections; /** The handler for idle connections. Must hold poolLock when accessing. */ @GuardedBy("poolLock") @@ -90,19 +87,6 @@ @GuardedBy("poolLock") protected int numConnections; - /** - * 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. - */ - // TODO - this needs to be synchronized, e.g. on Pool Lock - protected ReferenceQueue refQueue; - - /** A worker (thread) to track loss of pool entries to GC. */ - // TODO - this needs to be synchronized, e.g. on Pool Lock - private RefQueueWorker refWorker; - - /** Indicates whether this pool is shut down. */ protected volatile boolean isShutDown; @@ -110,50 +94,14 @@ * Creates a new connection pool. */ protected AbstractConnPool() { - issuedConnections = new HashSet(); + issuedConnections = new HashSet(); idleConnHandler = new IdleConnectionHandler(); boolean fair = false; //@@@ check parameters to decide poolLock = new ReentrantLock(fair); } - /** - * Enables connection garbage collection (GC). - * This method must be called immediately after creating the - * connection pool. It is not possible to enable connection GC - * after pool entries have been created. Neither is it possible - * to disable connection GC. - * - * @throws IllegalStateException - * if connection GC is already enabled, or if it cannot be - * enabled because there already are pool entries - */ - public void enableConnectionGC() - throws IllegalStateException { - - if (refQueue != null) { // TODO - this access is not guaranteed protected by the pool lock - throw new IllegalStateException("Connection GC already enabled."); - } - poolLock.lock(); - try { - if (numConnections > 0) { //@@@ is this check sufficient? - throw new IllegalStateException("Pool already in use."); - } - } finally { - poolLock.unlock(); - } - - refQueue = new ReferenceQueue(); - refWorker = new RefQueueWorker(refQueue, this); - Thread t = new Thread(refWorker); //@@@ use a thread factory - t.setDaemon(true); - t.setName("RefQueueWorker@" + this); - t.start(); - } - - - /** * Obtains a pool entry with a connection within the given timeout. * * @param route the route for which to get the connection @@ -200,35 +148,6 @@ public abstract void freeEntry(BasicPoolEntry entry, boolean reusable, long validDuration, TimeUnit timeUnit) ; - - - // non-javadoc, see interface RefQueueHandler - public void handleReference(Reference ref) { - - poolLock.lock(); - try { - - if (ref instanceof BasicPoolEntryRef) { - // check if the GCed pool entry was still in use - //@@@ find a way to detect this without lookup - //@@@ flag in the BasicPoolEntryRef, to be reset when freed? - final boolean lost = issuedConnections.remove(ref); - if (lost) { - final HttpRoute route = - ((BasicPoolEntryRef)ref).getRoute(); - if (log.isDebugEnabled()) { - log.debug("Connection garbage collected. " + route); - } - handleLostEntry(route); - } - } - - } finally { - poolLock.unlock(); - } - } - - /** * Handles cleaning up for a lost pool entry with the given route. * A lost pool entry corresponds to a connection that was @@ -291,16 +210,11 @@ if (isShutDown) return; - // no point in monitoring GC anymore - if (refWorker != null) - refWorker.shutdown(); - // close all connections that are issued to an application - Iterator iter = issuedConnections.iterator(); + Iterator iter = issuedConnections.iterator(); while (iter.hasNext()) { - BasicPoolEntryRef per = iter.next(); + BasicPoolEntry entry = iter.next(); iter.remove(); - BasicPoolEntry entry = per.get(); if (entry != null) { closeConnection(entry.getConnection()); } Index: httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueWorker.java =================================================================== --- httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueWorker.java (revision 781936) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueWorker.java (working copy) @@ -1,129 +0,0 @@ -/* - * $HeadURL$ - * $Revision$ - * $Date$ - * - * ==================================================================== - * - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * ==================================================================== - * - * This software consists of voluntary contributions made by many - * individuals on behalf of the Apache Software Foundation. For more - * information on the Apache Software Foundation, please see - * . - * - */ - -package org.apache.http.impl.conn.tsccm; - -import java.lang.ref.Reference; -import java.lang.ref.ReferenceQueue; - -/** - * A worker thread for processing queued references. - * {@link Reference Reference}s can be - * {@link ReferenceQueue queued} - * automatically by the garbage collector. - * If that feature is used, a daemon thread should be executing - * this worker. It will pick up the queued references and pass them - * on to a handler for appropriate processing. - * - * @since 4.0 - */ -public class RefQueueWorker implements Runnable { - - /** The reference queue to monitor. */ - protected final ReferenceQueue refQueue; - - /** The handler for the references found. */ - protected final RefQueueHandler refHandler; - - - /** - * The thread executing this handler. - * This attribute is also used as a shutdown indicator. - */ - protected volatile Thread workerThread; - - - /** - * Instantiates a new worker to listen for lost connections. - * - * @param queue the queue on which to wait for references - * @param handler the handler to pass the references to - */ - public RefQueueWorker(ReferenceQueue queue, RefQueueHandler handler) { - if (queue == null) { - throw new IllegalArgumentException("Queue must not be null."); - } - if (handler == null) { - throw new IllegalArgumentException("Handler must not be null."); - } - - refQueue = queue; - refHandler = handler; - } - - - /** - * The main loop of this worker. - * If initialization succeeds, this method will only return - * after {@link #shutdown shutdown()}. Only one thread can - * execute the main loop at any time. - */ - public void run() { - - if (this.workerThread == null) { - this.workerThread = Thread.currentThread(); - } - - while (this.workerThread == Thread.currentThread()) { - try { - // remove the next reference and process it - Reference ref = refQueue.remove(); - refHandler.handleReference(ref); - } catch (InterruptedException ignore) { - } - } - } - - - /** - * Shuts down this worker. - * It can be re-started afterwards by another call to {@link #run run()}. - */ - public void shutdown() { - Thread wt = this.workerThread; - if (wt != null) { - this.workerThread = null; // indicate shutdown - wt.interrupt(); - } - } - - - /** - * Obtains a description of this worker. - * - * @return a descriptive string for this worker - */ - @Override - public String toString() { - return "RefQueueWorker::" + this.workerThread; - } - -} // class RefQueueWorker - Index: httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java =================================================================== --- httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java (revision 781936) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java (working copy) @@ -30,16 +30,11 @@ package org.apache.http.impl.conn.tsccm; - -import java.lang.ref.ReferenceQueue; - import org.apache.http.conn.OperatedClientConnection; import org.apache.http.conn.ClientConnectionOperator; import org.apache.http.conn.routing.HttpRoute; import org.apache.http.impl.conn.AbstractPoolEntry; - - /** * Basic implementation of a connection pool entry. * @@ -48,13 +43,6 @@ public class BasicPoolEntry extends AbstractPoolEntry { /** - * A weak reference to this used to detect GC of entries. - * Pool entries can only be GCed when they are allocated by an application - * and therefore not referenced with a hard link in the manager. - */ - private final BasicPoolEntryRef reference; - - /** * Creates a new pool entry. * * @param op the connection operator @@ -63,13 +51,11 @@ * or null */ public BasicPoolEntry(ClientConnectionOperator op, - HttpRoute route, - ReferenceQueue queue) { + HttpRoute route) { super(op, route); if (route == null) { throw new IllegalArgumentException("HTTP route may not be null"); } - this.reference = new BasicPoolEntryRef(this, queue); } protected final OperatedClientConnection getConnection() { @@ -80,15 +66,11 @@ return super.route; } - protected final BasicPoolEntryRef getWeakRef() { - return this.reference; - } - @Override protected void shutdownEntry() { super.shutdownEntry(); } -} // class BasicPoolEntry +} Index: httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java =================================================================== --- httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java (revision 781936) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java (working copy) @@ -121,13 +121,7 @@ * @return the connection pool to use */ protected AbstractConnPool createConnectionPool(final HttpParams params) { - - AbstractConnPool acp = new ConnPoolByRoute(connOperator, params); - boolean conngc = true; //@@@ check parameters to decide - if (conngc) { - acp.enableConnectionGC(); - } - return acp; + return new ConnPoolByRoute(connOperator, params); } /**