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 2009/06/13 12:13:37 UTC
svn commit: r784361 - in /httpcomponents/httpclient/trunk: ./
httpclient/src/main/java/org/apache/http/impl/conn/tsccm/
httpclient/src/test/java/org/apache/http/impl/conn/
httpclient/src/test/java/org/apache/http/impl/conn/tsccm/
Author: olegk
Date: Sat Jun 13 10:13:36 2009
New Revision: 784361
URL: http://svn.apache.org/viewvc?rev=784361&view=rev
Log:
HTTPCLIENT-841: removed connection garbage collection due to a memory leak
Removed:
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/tsccm/TestDumbHelpers.java
Modified:
httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueHandler.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueWorker.java
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java
Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=784361&r1=784360&r2=784361&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Sat Jun 13 10:13:36 2009
@@ -1,6 +1,10 @@
Changes since 4.0 beta 2
-------------------
+* [HTTPCLIENT-841] Removed automatic connection release using garbage collection
+ due to a memory leak
+ Contributed by Oleg Kalnichevski <olegk at apache.org>
+
* [HTTPCLIENT-853] Fixed bug causing invalid cookie origin port to be selected
when the target is accessed on the default port and the connection is
established via a proxy.
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java?rev=784361&r1=784360&r2=784361&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/AbstractConnPool.java Sat Jun 13 10:13:36 2009
@@ -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<BasicPoolEntryRef> issuedConnections;
+ protected Set<BasicPoolEntry> 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<Object> 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<BasicPoolEntryRef> issuedConnections;
+
+ @Deprecated
+ protected ReferenceQueue<Object> refQueue;
+
/**
* Creates a new connection pool.
*/
protected AbstractConnPool() {
- issuedConnections = new HashSet<BasicPoolEntryRef>();
+ leasedConnections = new HashSet<BasicPoolEntry>();
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<Object>();
- refWorker = new RefQueueWorker(refQueue, this);
- Thread t = new Thread(refWorker); //@@@ use a thread factory
- t.setDaemon(true);
- t.setName("RefQueueWorker@" + this);
- t.start();
}
@@ -200,45 +162,12 @@
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();
- }
}
-
- /**
- * 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)
- ;
-
+ @Deprecated
+ protected abstract void handleLostEntry(HttpRoute route);
/**
* Closes idle connections.
@@ -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<BasicPoolEntryRef> iter = issuedConnections.iterator();
+ Iterator<BasicPoolEntry> 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;
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java?rev=784361&r1=784360&r2=784361&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/BasicPoolEntry.java Sat Jun 13 10:13:36 2009
@@ -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 <code>this</code> 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<Object> 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 <code>null</code>
*/
public BasicPoolEntry(ClientConnectionOperator op,
- HttpRoute route,
- ReferenceQueue<Object> 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
+}
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java?rev=784361&r1=784360&r2=784361&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java Sat Jun 13 10:13:36 2009
@@ -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();
}
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueHandler.java?rev=784361&r1=784360&r2=784361&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueHandler.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueHandler.java Sat Jun 13 10:13:36 2009
@@ -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 {
/**
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueWorker.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueWorker.java?rev=784361&r1=784360&r2=784361&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueWorker.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/RefQueueWorker.java Sat Jun 13 10:13:36 2009
@@ -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. */
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java?rev=784361&r1=784360&r2=784361&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/tsccm/ThreadSafeClientConnManager.java Sat Jun 13 10:13:36 2009
@@ -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);
}
/**
Modified: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java?rev=784361&r1=784360&r2=784361&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/conn/TestTSCCMWithServer.java Sat Jun 13 10:13:36 2009
@@ -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<ManagedClientConnection> wref =
- new WeakReference<ManagedClientConnection>(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 {