You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Oleg Kalnichevski (Jira)" <ji...@apache.org> on 2022/08/17 16:37:00 UTC

[jira] [Updated] (HTTPCLIENT-2231) Race condition starting request causes request to fail

     [ https://issues.apache.org/jira/browse/HTTPCLIENT-2231?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated HTTPCLIENT-2231:
------------------------------------------
        Fix Version/s: 5.1.4
                       5.2-beta2
    Affects Version/s: 5.2-beta1
                       5.1.3

> 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org