Malay Shah created HTTPCORE-722:
-----------------------------------
Summary: Race condition starting request causes request to fail
Key: HTTPCORE-722
URL: https://issues.apache.org/jira/browse/HTTPCORE-722
Project: HttpComponents HttpCore
Issue Type: Bug
Reporter: Malay Shah
I believe we have found an infrequent race condition in the Http Client library
and can reproduce it about every few thousand requests. We are using version
5.1.3. What I believe is happening is that the request processing begins on the
IO threads before the {{execute}} call has finished configuring its data
structures to manage the request, causing an errant call to
{{{}AsyncResponseConsumer<T>::fail{}}}. In this situation, the response headers
have arrived and dispatched through a call to
{{{}AsyncResponseConsumer<T>::consumeResponse{}}}, and then the {{execute}}
thread gets to the final line in {{{}HttpAsyncMainClientExec::execute{}}}:
{{operation.setDependency(execRuntime.execute(exchangeId,
internalExchangeHandler, clientContext));}}
Here, {{operation}} is a {{ComplexFuture}} object that has already been
completed so it ends up cancelling the dependency:
{{@Override}}
{{public void setDependency(final Cancellable dependency) {}}
{{ Args.notNull(dependency, "dependency");}}
{{ if (isDone()) {}}
{{ dependency.cancel();}}
{{ } else {}}
{{ dependencyRef.set(dependency);}}
{{ }}}
{{}}}
Here's the call stack:
{{org.apache.hc.client5.http.impl.async.HttpAsyncMainClientExec$1.cancel(HttpAsyncMainClientExec.java:129)}}
{{org.apache.hc.client5.http.impl.async.InternalHttpAsyncExecRuntime$3.cancel(InternalHttpAsyncExecRuntime.java:267)}}
{{org.apache.hc.core5.concurrent.ComplexFuture.setDependency(ComplexFuture.java:55)}}
{{org.apache.hc.client5.http.impl.async.HttpAsyncMainClientExec.execute(HttpAsyncMainClientExec.java:250)}}
{{org.apache.hc.client5.http.impl.async.AsyncExecChainElement.execute(AsyncExecChainElement.java:54)}}
{{org.apache.hc.client5.http.impl.async.AsyncExecChainElement$1.proceed(AsyncExecChainElement.java:62)}}
{{org.apache.hc.client5.http.impl.async.AsyncConnectExec$1.completed(AsyncConnectExec.java:142)}}
{{org.apache.hc.client5.http.impl.async.AsyncConnectExec$1.completed(AsyncConnectExec.java:136)}}
{{org.apache.hc.client5.http.impl.async.InternalHttpAsyncExecRuntime$1.completed(InternalHttpAsyncExecRuntime.java:114)}}
{{org.apache.hc.client5.http.impl.async.InternalHttpAsyncExecRuntime$1.completed(InternalHttpAsyncExecRuntime.java:105)}}
{{org.apache.hc.core5.concurrent.BasicFuture.completed(BasicFuture.java:123)}}
{{org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager$1$1.leaseCompleted(PoolingAsyncClientConnectionManager.java:285)}}
{{org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager$1$1.completed(PoolingAsyncClientConnectionManager.java:270)}}
{{org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager$1$1.completed(PoolingAsyncClientConnectionManager.java:234)}}
{{org.apache.hc.core5.concurrent.BasicFuture.completed(BasicFuture.java:123)}}
{{org.apache.hc.core5.pool.StrictConnPool.fireCallbacks(StrictConnPool.java:399)}}
{{org.apache.hc.core5.pool.StrictConnPool.lease(StrictConnPool.java:215)}}
{{org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager$1.<init>(PoolingAsyncClientConnectionManager.java:231)}}
{{org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager.lease(PoolingAsyncClientConnectionManager.java:227)}}
{{org.apache.hc.client5.http.impl.async.InternalHttpAsyncExecRuntime.acquireEndpoint(InternalHttpAsyncExecRuntime.java:100)}}
{{org.apache.hc.client5.http.impl.async.AsyncConnectExec.execute(AsyncConnectExec.java:135)}}
{{org.apache.hc.client5.http.impl.async.AsyncExecChainElement.execute(AsyncExecChainElement.java:54)}}
{{org.apache.hc.client5.http.impl.async.AsyncExecChainElement$1.proceed(AsyncExecChainElement.java:62)}}
{{org.apache.hc.client5.http.impl.async.AsyncProtocolExec.internalExecute(AsyncProtocolExec.java:183)}}
{{org.apache.hc.client5.http.impl.async.AsyncProtocolExec.execute(AsyncProtocolExec.java:145)}}
{{org.apache.hc.client5.http.impl.async.AsyncExecChainElement.execute(AsyncExecChainElement.java:54)}}
{{org.apache.hc.client5.http.impl.async.AsyncExecChainElement$1.proceed(AsyncExecChainElement.java:62)}}
{{org.apache.hc.client5.http.impl.async.AsyncHttpRequestRetryExec.internalExecute(AsyncHttpRequestRetryExec.java:97)}}
{{org.apache.hc.client5.http.impl.async.AsyncHttpRequestRetryExec.execute(AsyncHttpRequestRetryExec.java:183)}}
{{org.apache.hc.client5.http.impl.async.AsyncExecChainElement.execute(AsyncExecChainElement.java:54)}}
{{org.apache.hc.client5.http.impl.async.AsyncExecChainElement$1.proceed(AsyncExecChainElement.java:62)}}
{{org.apache.hc.client5.http.impl.async.AsyncRedirectExec.internalExecute(AsyncRedirectExec.java:112)}}
{{org.apache.hc.client5.http.impl.async.AsyncRedirectExec.execute(AsyncRedirectExec.java:278)}}
{{org.apache.hc.client5.http.impl.async.AsyncExecChainElement.execute(AsyncExecChainElement.java:54)}}
{{org.apache.hc.client5.http.impl.async.InternalAbstractHttpAsyncClient.executeImmediate(InternalAbstractHttpAsyncClient.java:367)}}
{{org.apache.hc.client5.http.impl.async.InternalAbstractHttpAsyncClient$1.sendRequest(InternalAbstractHttpAsyncClient.java:223)}}
{{org.apache.hc.core5.http.nio.support.BasicRequestProducer.sendRequest(BasicRequestProducer.java:93)}}
{{org.apache.hc.client5.http.impl.async.InternalAbstractHttpAsyncClient.doExecute(InternalAbstractHttpAsyncClient.java:180)}}
{{org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient.execute(CloseableHttpAsyncClient.java:97)}}
{{org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient.execute(CloseableHttpAsyncClient.java:107)}}
Back to {{{}HttpAsyncMainClientExec::execute{}}}:
{{operation.setDependency(execRuntime.execute(exchangeId,
internalExchangeHandler, clientContext));}}
This calls {{InternalHttpAsyncExecRuntime::execute}} which calls
{{endpoint.execute(id, exchangeHandler, context);}} which I believe begins the
asynchronous processing of the HTTP exchange.{{
InternalHttpAsyncExecRuntime::execute}} then returns the {{Cancellable}} to be
set in the {{{}ComplexFuture{}}}.
This only happens when there is already an available connection to lease
because the the sequence above happens on the thread that calls execute; when
no connection is available, one is created and the setup of the exchange then
also happens on the HTTP dispatch thread so processing of bytes will not happen
until after the setup is complete (since they both must happen on the dispatch
thread).
I have two possible ideas for solutions:
Always do the setup on the HTTP dispatch thread. This seems like the safest
option, but I'm not sure if that will impact anything else.
The other idea is to configure the dependency callback first before letting the
HTTP exchange begin. This may solve this particular race condition but still
leaves it possible for other similar race conditions.
I or one of my colleagues may be able to implement a fix but we would need
guidance on how best to solve this.
I have a simple unit test that exhibits this issue that I'll post as a comment
below.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
