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 782906) +++ 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 782906) +++ 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 782906) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueHandler.java (working copy) @@ -32,12 +32,12 @@ import java.lang.ref.Reference; - /** - * Callback handler for {@link RefQueueWorker RefQueueWorker}. + * @deprecated do not use * * @since 4.0 */ +@Deprecated public interface RefQueueHandler { /** 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 782906) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java (working copy) @@ -283,7 +283,7 @@ if (log.isDebugEnabled()) { log.debug("Total connections kept alive: " + freeConnections.size()); - log.debug("Total issued connections: " + issuedConnections.size()); + log.debug("Total issued connections: " + leasedConnections.size()); log.debug("Total allocated connection: " + numConnections + " out of " + maxTotalConnections); } @@ -381,7 +381,7 @@ } // no longer issued, we keep a hard reference now - issuedConnections.remove(entry.getWeakRef()); + leasedConnections.remove(entry); RouteSpecificPool rospl = getRoutePool(route, true); @@ -448,7 +448,7 @@ rospl.dropEntry(); numConnections--; } else { - issuedConnections.add(entry.getWeakRef()); + leasedConnections.add(entry); done = true; } @@ -486,17 +486,13 @@ } // the entry will create the connection when needed - BasicPoolEntry entry = - new BasicPoolEntry(op, rospl.getRoute(), refQueue); + BasicPoolEntry entry = new BasicPoolEntry(op, rospl.getRoute()); poolLock.lock(); try { - rospl.createdEntry(entry); numConnections++; - - issuedConnections.add(entry.getWeakRef()); - + leasedConnections.add(entry); } finally { poolLock.unlock(); } 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 782906) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java (working copy) @@ -41,7 +41,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 +50,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,7 +59,9 @@ * * @since 4.0 */ -@NotThreadSafe // unsynch access to refQueue, refWorker + +@ThreadSafe +@SuppressWarnings("deprecation") public abstract class AbstractConnPool implements RefQueueHandler { private final Log log = LogFactory.getLog(getClass()); @@ -70,17 +71,12 @@ */ protected final Lock poolLock; - /** * References to issued connections. - * Objects in this set are of class - * {@link BasicPoolEntryRef BasicPoolEntryRef}, - * and point to the pool entry for the issued connection. - * GCed connections are detected by the missing pool entries. * Must hold poolLock when accessing. */ @GuardedBy("poolLock") - protected Set issuedConnections; + protected Set leasedConnections; /** The handler for idle connections. Must hold poolLock when accessing. */ @GuardedBy("poolLock") @@ -90,66 +86,32 @@ @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; + @Deprecated + protected Set issuedConnections; + + @Deprecated + protected ReferenceQueue refQueue; + /** * Creates a new connection pool. */ protected AbstractConnPool() { - issuedConnections = new HashSet(); + leasedConnections = 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 + * @deprecated do not sue */ + @Deprecated 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(); } @@ -200,47 +162,14 @@ public abstract void freeEntry(BasicPoolEntry entry, boolean reusable, long validDuration, TimeUnit timeUnit) ; - - - // non-javadoc, see interface RefQueueHandler + @Deprecated 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(); - } } + @Deprecated + protected abstract void handleLostEntry(HttpRoute route); /** - * Handles cleaning up for a lost pool entry with the given route. - * A lost pool entry corresponds to a connection that was - * garbage collected instead of being properly released. - * - * @param route the route of the pool entry that was lost - */ - protected abstract void handleLostEntry(HttpRoute route) - ; - - - /** * Closes idle connections. * * @param idletime the time the connections should have been idle @@ -291,23 +220,13 @@ 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 = leasedConnections.iterator(); while (iter.hasNext()) { - BasicPoolEntryRef per = iter.next(); + BasicPoolEntry entry = iter.next(); iter.remove(); - BasicPoolEntry entry = per.get(); - if (entry != null) { - closeConnection(entry.getConnection()); - } + closeConnection(entry.getConnection()); } - - // remove all references to connections - //@@@ use this for shutting them down instead? idleConnHandler.removeAll(); isShutDown = true; 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 782906) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueWorker.java (working copy) @@ -42,8 +42,9 @@ * this worker. It will pick up the queued references and pass them * on to a handler for appropriate processing. * - * @since 4.0 + * @deprecated do not use */ +@Deprecated public class RefQueueWorker implements Runnable { /** The reference queue to monitor. */ 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 782906) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java (working copy) @@ -30,7 +30,6 @@ package org.apache.http.impl.conn.tsccm; - import java.lang.ref.ReferenceQueue; import org.apache.http.conn.OperatedClientConnection; @@ -38,8 +37,6 @@ import org.apache.http.conn.routing.HttpRoute; import org.apache.http.impl.conn.AbstractPoolEntry; - - /** * Basic implementation of a connection pool entry. * @@ -48,11 +45,17 @@ 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. + * @deprecated do not use */ - private final BasicPoolEntryRef reference; + @Deprecated + public BasicPoolEntry(ClientConnectionOperator op, + HttpRoute route, + ReferenceQueue queue) { + super(op, route); + if (route == null) { + throw new IllegalArgumentException("HTTP route may not be null"); + } + } /** * Creates a new pool entry. @@ -63,13 +66,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,8 +81,9 @@ return super.route; } + @Deprecated protected final BasicPoolEntryRef getWeakRef() { - return this.reference; + return null; } @Override @@ -89,6 +91,6 @@ 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 782906) +++ httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java (working copy) @@ -123,13 +123,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); } /**