ApacheHttpClient4Executor doesn't close connections, Part 2

If a call to org.jboss.resteasy.client.ClientRequest.get() or any of the other HTTP verb methods returns an org.jboss.resteasy.client.ClientResponse, and if ClientResponse.getEntity() is not called, then it is necessary to call ClientResponse.releaseConnection() to "release the underlying connection". I put "release the underlying connection" in quotes because its semantics are somewhat fuzzy.

For example, in HttpClient (4.x), SingleClientConnManager uses SingleClientConnManager$ConnAdapter to reference an instance of SingleClientConnManager$PoolEntry, which references an instance of org.apache.http.impl.conn.DefaultClientConnection, which references an instance of java.net.Socket. SingleClientConnManager.getConnection() will return a given instance of SingleClientConnManager$ConnAdapter only a single time, and if SingleClientConnManager$ConnAdapter.getConnection() is called while an instance of SingleClientConnManager$ConnAdapter exists, it will throw an IllegalStateException with the message "Invalid use of SingleClientConnManager: connection still allocated.\n" + "Make sure to release the connection before allocating another one." Calling ClientResponse.releaseConnection() leads to the following sequence:

ClientResponse.releaseConnection()
  -> ApacheHttpClient4Executor.performReleaseConnection()
  -> org.jboss.resteasy.client.core.SelfExpandingBufferredInputStream.close()
  -> org.apache.http.conn.EofSensorInputStream.close()
  -> org.apache.http.conn.BasicManagedEntity.streamClosed()
  -> org.apache.http.conn.BasicManagedEntity.releaseManagedConnection()
  -> org.apache.http.impl.conn.SingleClientConnManager$ConnAdapter.releaseConnection()
  -> org.apache.http.impl.conn.SingleClientConnManager.releaseConnection()

where SingleClientConnManger.releaseConnection() throws away the reference to the existing SingleClientConnManager$ConnAdapter so that a subsequent call to SingleClientConnManager.getConnection() will succeed.

However, even though SingleClientConnManager throws away its reference to the SingleClientConnManager$ConnAdapter, it RETAINS its reference to the SingleClientConnManager$PoolEntry, which it can use to supply a subsquently created instance of SingleClientConnManager$ConnAdapter with an EXISTING Socket. So, when using HttpClient (4.x), "release the underlying connection" means "make an existing Socket available for reuse". It DOES NOT MEAN "close an existing Socket".

A phenomenon that can be observed in the RESTEasy test suite occurs when too many separate ClientRequests have been created and, even though ClientResponse.releaseConnection() has been called on the related ClientResponse, the underlying Sockets remain open. Each Socket is connected to a Socket on the server side, and the server side Socket is managed by a worker thread in TJWS, the embedded servlet server used in the test suite. Once TJWS runs out of worker threads, new ClientRequests have to wait until an existing Socket times out and a worker thread is made available.

A workaround is to replace the call to ClientResponse.releaseConnection() with

ApacheHttpClient4Executor executor = (ApacheHttpClient4Executor) request.getExecutor();
executor.getHttpClient().getConnectionManager().shutdown();

which will close the Socket created by SingleClientConnManager.

Bill suggests

I think putting a finalize() method in ApacheHttpClient4Executor is the way to go and call the shutdown() there if the Executor created HttpClient object.

void finalize() {
   if (wasCreatedByMe) { httpClient.getConnectionManager().shutdown(); }
} 

and, more generally,

We should also probably add a shutdown() method to the ClientExecutor interface and put a finalize method in anything that manages ClientExecutor instances.

The question that remains is: Should we add something like ClientRequest.shutdown() to the RESTEasy client framework API with the semantics of closing any Sockets opened for the ClientRequest. Also, what about client proxies?

For now, we will wait for the JAX-RS 2.0 client framework before making any changes.