[
https://issues.apache.org/jira/browse/HTTPCLIENT-2231?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Oleg Kalnichevski resolved HTTPCLIENT-2231.
-------------------------------------------
Resolution: Fixed
> Race condition starting request causes request to fail
> ------------------------------------------------------
>
> Key: HTTPCLIENT-2231
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-2231
> Project: HttpComponents HttpClient
> Issue Type: Bug
> Affects Versions: 5.1.3, 5.2-beta1
> Reporter: Malay Shah
> Priority: Major
> Fix For: 5.1.4, 5.2-beta2
>
> Attachments: TestRaceCondition.java
>
>
> 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]
