HTTPCLIENT-763: AbstractClientConnAdapter#abortConnection() does not release the connection if called from the main execution thread while there is no blocking I/O operation.
AbstractClientConnAdapter#abortConnection() is usually expected to be called from a helper thread in order to unblock the main execution thread blocked in an I/O operation. It may be unsafe to call AbstractClientConnAdapter#releaseConnection() from the helper thread, so we have to rely on an IOException thrown by the closed socket on the main thread to trigger the release of the connection back to the connection manager. However, if this method is called from the main execution thread it should be safe to release the connection immediately. Besides, this also helps ensure the connection gets released back to the manager if AbstractClientConnAdapter#abortConnection() is called from the main execution thread while there is no blocking I/O operation. git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@650223 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
parent
ef548b4620
commit
e2eb355878
|
@ -78,6 +78,9 @@ import org.apache.http.conn.ClientConnectionManager;
|
|||
public abstract class AbstractClientConnAdapter
|
||||
implements ManagedClientConnection {
|
||||
|
||||
/** Thread that requested this connection. */
|
||||
private final Thread executionThread;
|
||||
|
||||
/**
|
||||
* The connection manager, if any.
|
||||
* This attribute MUST NOT be final, so the adapter can be detached
|
||||
|
@ -103,7 +106,8 @@ public abstract class AbstractClientConnAdapter
|
|||
*/
|
||||
protected AbstractClientConnAdapter(ClientConnectionManager mgr,
|
||||
OperatedClientConnection conn) {
|
||||
|
||||
super();
|
||||
executionThread = Thread.currentThread();
|
||||
connManager = mgr;
|
||||
wrappedConnection = conn;
|
||||
markedReusable = false;
|
||||
|
@ -362,6 +366,22 @@ public abstract class AbstractClientConnAdapter
|
|||
conn.shutdown();
|
||||
} catch (IOException ignore) {
|
||||
}
|
||||
// Usually #abortConnection() is expected to be called from
|
||||
// a helper thread in order to unblock the main execution thread
|
||||
// blocked in an I/O operation. It may be unsafe to call
|
||||
// #releaseConnection() from the helper thread, so we have to rely
|
||||
// on an IOException thrown by the closed socket on the main thread
|
||||
// to trigger the release of the connection back to the
|
||||
// connection manager.
|
||||
//
|
||||
// However, if this method is called from the main execution thread
|
||||
// it should be safe to release the connection immediately. Besides,
|
||||
// this also helps ensure the connection gets released back to the
|
||||
// manager if #abortConnection() is called from the main execution
|
||||
// thread while there is no blocking I/O operation.
|
||||
if (executionThread.equals(Thread.currentThread())) {
|
||||
releaseConnection();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -208,7 +208,7 @@ public class SingleClientConnManager implements ClientConnectionManager {
|
|||
assertStillUp();
|
||||
|
||||
if (LOG.isDebugEnabled()) {
|
||||
LOG.debug("SingleClientConnManager.getConnection: " + route);
|
||||
LOG.debug("Get connection for route " + route);
|
||||
}
|
||||
|
||||
if (managedConn != null)
|
||||
|
@ -246,6 +246,11 @@ public class SingleClientConnManager implements ClientConnectionManager {
|
|||
("Connection class mismatch, " +
|
||||
"connection not obtained from this manager.");
|
||||
}
|
||||
|
||||
if (LOG.isDebugEnabled()) {
|
||||
LOG.debug("Releasing connection " + conn);
|
||||
}
|
||||
|
||||
ConnAdapter sca = (ConnAdapter) conn;
|
||||
if (sca.getManager() != this) {
|
||||
throw new IllegalArgumentException
|
||||
|
@ -277,6 +282,7 @@ public class SingleClientConnManager implements ClientConnectionManager {
|
|||
} finally {
|
||||
sca.detach();
|
||||
managedConn = null;
|
||||
uniquePoolEntry.tracker = null;
|
||||
lastReleaseTime = System.currentTimeMillis();
|
||||
}
|
||||
} // releaseConnection
|
||||
|
@ -334,11 +340,7 @@ public class SingleClientConnManager implements ClientConnectionManager {
|
|||
if (managedConn == null)
|
||||
return;
|
||||
|
||||
// Generate a stack trace, it might help debugging.
|
||||
// Do NOT throw the exception, just log it!
|
||||
IllegalStateException isx = new IllegalStateException
|
||||
("Revoking connection to " + managedConn.getRoute());
|
||||
LOG.warn(MISUSE_MESSAGE, isx);
|
||||
LOG.warn(MISUSE_MESSAGE);
|
||||
|
||||
if (managedConn != null)
|
||||
managedConn.detach();
|
||||
|
|
|
@ -281,6 +281,58 @@ public class TestTSCCMWithServer extends ServerTestBase {
|
|||
}
|
||||
|
||||
|
||||
/**
|
||||
* Tests releasing connection from #abort method called from the
|
||||
* main execution thread while there is no blocking I/O operation.
|
||||
*/
|
||||
public void testReleaseConnectionOnAbort() throws Exception {
|
||||
|
||||
HttpParams mgrpar = defaultParams.copy();
|
||||
HttpConnectionManagerParams.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
|
||||
HttpResponse response = Helper.execute(
|
||||
request, conn, target,
|
||||
httpExecutor, httpProcessor, defaultParams, httpContext);
|
||||
|
||||
assertEquals("wrong status in first response",
|
||||
HttpStatus.SC_OK,
|
||||
response.getStatusLine().getStatusCode());
|
||||
|
||||
// check that there are no connections available
|
||||
try {
|
||||
// this should fail quickly, connection has not been released
|
||||
getConnection(mgr, route, 100L, TimeUnit.MILLISECONDS);
|
||||
fail("ConnectionPoolTimeoutException should have been thrown");
|
||||
} catch (ConnectionPoolTimeoutException e) {
|
||||
// expected
|
||||
}
|
||||
|
||||
// abort the connection
|
||||
assertTrue(conn instanceof AbstractClientConnAdapter);
|
||||
((AbstractClientConnAdapter) conn).abortConnection();
|
||||
|
||||
// the connection is expected to be released back to the manager
|
||||
conn = getConnection(mgr, route, 5L, TimeUnit.SECONDS);
|
||||
assertFalse("connection should have been closed", conn.isOpen());
|
||||
|
||||
mgr.releaseConnection(conn);
|
||||
mgr.shutdown();
|
||||
}
|
||||
|
||||
/**
|
||||
* Tests GC of an unreferenced connection.
|
||||
*/
|
||||
|
|
Loading…
Reference in New Issue