[HTTPCORE-584] Iterating a LaxConnPool is not thread-safe

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 5.0-beta8
• Fix Version/s: 5.0-beta9
• Component/s: HttpCore
• Assignee: Unassigned
• Reporter: Linton Miller
• Created: 23/Jul/19 21:54
• Updated: 27/Jul/19 21:42
• Resolved: 27/Jul/19 21:42

Description

Iterating over connections in a LaxConnPool can cause unexpected connection exceptions; for example, calling the closeIdle or closeExpired methods at the same time as threads are requesting connections from the pool.

This is demonstrated by the attached test class, which creates a LaxConnPool for a PoolingHttpClientConnectionManager and then uses that to execute a number of concurrent requests, sleeps a bit, then re-executes concurrent requests at the same time as closing idle connections in the pool. Being a threading bug, it's not a guaranteed fail every time, but within a few runs, the test code will normally get at least one request thread that fails. e.g

Thread req-000000AB request failed!
org.apache.hc.core5.http.ConnectionClosedException: Connection is closed
        at org.apache.hc.core5.http.impl.io.BHttpConnectionBase.ensureOpen(BHttpConnectionBase.java:98)
        at org.apache.hc.core5.http.impl.io.DefaultBHttpClientConnection.receiveResponseHeader(DefaultBHttpClientConnection.java:186)
        at org.apache.hc.core5.http.impl.io.HttpRequestExecutor.execute(HttpRequestExecutor.java:181)
        at org.apache.hc.core5.http.impl.io.HttpRequestExecutor.execute(HttpRequestExecutor.java:224)
        at org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager$InternalConnectionEndpoint.execute(PoolingHttpClientConnectionManager.java:596)
        at org.apache.hc.client5.http.impl.classic.InternalExecRuntime.execute(InternalExecRuntime.java:220)
        at org.apache.hc.client5.http.impl.classic.InternalHttpClient.doExecute(InternalHttpClient.java:175)
...
        at org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(CloseableHttpClient.java:77)
        at org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(CloseableHttpClient.java:102)
        at LaxConnPoolThreadingTest$2.executeReq(LaxConnPoolThreadingTest.java:69)
        at LaxConnPoolThreadingTest$2.run(LaxConnPoolThreadingTest.java:58)

or

 

Thread req-00000179 request failed!
java.lang.IllegalStateException: Endpoint is not connected
        at org.apache.hc.core5.util.Asserts.check(Asserts.java:38)
        at org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager$InternalConnectionEndpoint.getValidatedPoolEntry(PoolingHttpClientConnectionManager.java:548)
        at org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager$InternalConnectionEndpoint.execute(PoolingHttpClientConnectionManager.java:592)
...

There may also be recoverable I/O exceptions logged:

2019-07-24 09:17:27,488 INFO  [org.apache.hc.client5.http.impl.classic.RetryExec] Recoverable I/O exception (java.net.SocketException) caught when processing request to {}->http://httpbin.org:80

 

This appear to be because the iteration methods of LaxConnPool are not thread-safe. Though their access to the internal queue structures are protected by the queues being ConcurrentLinkedDeque, the iteration provides the means for the same pool entry to be accessed on multiple threads at once because there is no locking or concurrency control on the individual pool entries as the callback is executed through enumAvailable or enumLeased.