another test case ported, trying to get rid of hard references to TSCCM

git-svn-id: https://svn.apache.org/repos/asf/jakarta/httpcomponents/httpclient/trunk@539772 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/http/impl/conn/AbstractClientConnAdapter.java

@@ -75,8 +75,12 @@
 public abstract class AbstractClientConnAdapter
     implements ManagedClientConnection {
 
-    /** The connection manager, if any. */
+    /**
-    protected final ClientConnectionManager connManager;
+     * The connection manager, if any.
+     * This attribute MUST NOT be final, so the adapter can be detached
+     * from the connection manager without keeping a hard reference there.
+     */
+    protected ClientConnectionManager connManager;
 
     /** The wrapped connection. */
     protected OperatedClientConnection wrappedConnection;
@@ -253,11 +257,8 @@ public boolean isMarkedReusable() {
 
     // non-javadoc, see interface ConnectionReleaseTrigger
     public void releaseConnection() {
-        if (connManager == null)
+        if (connManager != null)
-            throw new IllegalStateException
+            connManager.releaseConnection(this);
-                ("No connection manager to release to.");
-
-        connManager.releaseConnection(this);
     }
 
     // non-javadoc, see interface ConnectionReleaseTrigger
@@ -265,11 +266,8 @@ public void abortConnection() {
 
         unmarkReusable();
 
-        if (connManager == null)
+        if (connManager != null)
-            throw new IllegalStateException
+            connManager.releaseConnection(this);
-                ("No connection manager to release to.");
-
-        connManager.releaseConnection(this);
     }
 
 } // class AbstractClientConnAdapter

src/java/org/apache/http/impl/conn/AbstractPooledConnAdapter.java

@@ -99,6 +99,7 @@ protected final void assertAttached() {
     protected void detach() {
         wrappedConnection = null;
         poolEntry = null;
+        connManager = null; // base class attribute
     }
 
 

src/test/org/apache/http/impl/conn/Helper.java

@@ -0,0 +1,76 @@
+/*
+ * $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
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.http.impl.conn;
+
+import org.apache.http.HttpHost;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpClientConnection;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpProcessor;
+import org.apache.http.protocol.HttpRequestExecutor;
+import org.apache.http.protocol.HttpExecutionContext;
+
+
+/**
+ * Static helper methods.
+ */
+public final class Helper {
+
+    /** Disabled default constructor. */
+    private Helper() {
+        // no body
+    }
+
+
+    /**
+     * Executes a request.
+     */
+    public static HttpResponse execute(HttpRequest req,
+                                       HttpClientConnection conn,
+                                       HttpHost target,
+                                       HttpRequestExecutor exec,
+                                       HttpProcessor proc,
+                                       HttpContext ctxt)
+        throws Exception {
+
+        ctxt.setAttribute(HttpExecutionContext.HTTP_CONNECTION, conn);
+        ctxt.setAttribute(HttpExecutionContext.HTTP_TARGET_HOST, target);
+        ctxt.setAttribute(HttpExecutionContext.HTTP_REQUEST, req);
+
+        exec.preProcess(req, proc, ctxt);
+        HttpResponse rsp = exec.execute(req, conn, ctxt);
+        exec.postProcess(rsp, proc, ctxt);
+
+        return rsp;
+    }
+
+}

src/test/org/apache/http/impl/conn/TestTSCCMWithServer.java

@@ -30,6 +30,9 @@
 
 package org.apache.http.impl.conn;
 
+
+import java.lang.ref.WeakReference;
+
 import junit.framework.Test;
 import junit.framework.TestSuite;
 
@@ -112,7 +115,6 @@ public HttpParams createManagerParams() {
      * Tests releasing and re-using a connection after a response is read.
      */
     public void testReleaseConnection() throws Exception {
-        //public void testSkeleton() throws Exception {
 
         HttpParams mgrpar = createManagerParams();
         HttpConnectionManagerParams.setMaxTotalConnections(mgrpar, 1);
@@ -131,19 +133,8 @@ public void testReleaseConnection() throws Exception {
         conn.open(route, httpContext, defaultParams);
 
         // a new context is created for each testcase, no need to reset
-        httpContext.setAttribute(
+        HttpResponse response = Helper.execute
-            HttpExecutionContext.HTTP_CONNECTION, conn);
+            (request, conn, target, httpExecutor, httpProcessor, httpContext);
-        httpContext.setAttribute(
-            HttpExecutionContext.HTTP_TARGET_HOST, target);
-        httpContext.setAttribute(
-            HttpExecutionContext.HTTP_REQUEST, request);
-
-        httpExecutor.preProcess
-            (request, httpProcessor, httpContext);
-        HttpResponse response =
-            httpExecutor.execute(request, conn, httpContext);
-        httpExecutor.postProcess
-            (response, httpProcessor, httpContext);
 
         assertEquals("wrong status in first response",
                      HttpStatus.SC_OK,
@@ -207,17 +198,81 @@ public void testReleaseConnection() throws Exception {
     }
 
 
+    /**
+     * Tests GC of an unreferenced connection manager.
+     */
+    public void testConnectionManagerGC() throws Exception {
+        // 3.x: TestHttpConnectionManager.testDroppedThread
+
+        ThreadSafeClientConnManager mgr = createTSCCM(null, 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 = mgr.getConnection(route);
+        conn.open(route, httpContext, defaultParams);
+
+        // a new context is created for each testcase, no need to reset
+        HttpResponse response = Helper.execute
+            (request, conn, target, httpExecutor, httpProcessor, httpContext);
+        byte[] data = EntityUtils.toByteArray(response.getEntity());
+
+        // release connection after marking it for re-use
+        conn.markReusable();
+        mgr.releaseConnection(conn);
+
+        // We now have a manager with an open connection. We drop all
+        // potential hard reference to it and check whether it is GCed.
+        // Note that the connection keeps a reference even if detached.
+        // Internal references might prevent that if set up incorrectly.
+
+        WeakReference wref = new WeakReference(mgr);
+
+        request = null;
+        response = null;
+        mgr = null;
+
+        //@@@ the connection currently prevents the manager from being GCed
+        conn = null;
+        httpContext = null; // holds the connection and request
+
+        // 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("TSCCM not garbage collected", wref.get());
+    }
+
+
     // List of server-based tests in 3.x TestHttpConnectionManager
     // The execution framework (HttpClient) used by some of them
     // can probably be replaced by hand-coded request execution
     //
-    // testConnectMethodFailureRelease
+    // + testReleaseConnection
-    // testDroppedThread
+    // + testDroppedThread
-    // testWriteRequestReleaseConnection, depends on execution framework
-    // + testReleaseConnection, depends on execution framework
-    // testResponseAutoRelease
-    // testMaxConnectionsPerServer - what's the server used/needed for?
     // testReclaimUnusedConnection, depends on execution framework
     // testGetFromMultipleThreads, depends on execution framework
 
+
+    // Server-based tests not ported from 3.x TestHttpConnectionManager
+    //
+    // testWriteRequestReleaseConnection
+    //      This tests auto-release in case of an I/O error while writing.
+    //      It's a test of the execution framework, not of the manager.
+    // testConnectMethodFailureRelease
+    //      This tests method.fakeResponse() and auto-release. It's a
+    //      test of a 3.x specific hack and the execution framework.
+    // testResponseAutoRelease
+    //      Auto-release is not part of the connection manager.
+    // testMaxConnectionsPerServer
+    //      Connection limits are already tested without a server.
+
+
 } // class TestTSCCMWithServer