hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hirthwork <...@git.apache.org>
Subject [GitHub] httpcore pull request: Fix race condition in org.apache.http.nio.p...
Date Fri, 05 Feb 2016 19:14:03 GMT
GitHub user hirthwork opened a pull request:

    https://github.com/apache/httpcore/pull/21

    Fix race condition in org.apache.http.nio.protocol.HttpAsyncRequestExecutor.endOfInput
and org.apache.http.impl.nio.client.AbstractClientExchangeHandler.connectionAllocated(conn)

    Got pretty bad race condition when sending async requests.
    Symptoms & Environment: We have target host which sometimes closes connections after
requests processing but no `Connection: close` header was set. Some requests hangs, i.e. `BasicFuture`
methods `.cancelled`, `.completed` or `.failed` methods are never called.
    
    Analysis:
    Let there be two threads:
    - T1 is the async client reactor thread.
    - T2 is the thread that initiates HTTP request.
    
    T1: completes HTTP request and calls `connmgr.releaseConnection`
    T2: initiates HTTP requests asks connmgr for connection and immediately gets connection
that was just released by T1
    T1: receives EOF on released connection and falls into `HttpAsyncRequestExecutor.endOfInput`.
Connection has valid `HTTP_EXCHANGE_STATE`, but `HTTP_HANDLER` is null
    T2: sets `HTTP_HANDLER`, requests output and checks `.isState`. Everything is OK here.
    T1: goes to the end of `.endOfInput` and closes connection. `HTTP_HANDLER` is not null
anymore, but is won't be notified about connection close, soon connection will be garbage
collected and it will never be released and `HTTP_HANDLER.basicFuture` will never be notified.
    
    Suggested fix (not sure if it is optimal, but it changed behaviour to be more redundant):
    At the start of `.endOfInput` try to notify `HTTP_HANDLER`. If there is no one, set variable
`handlerNotified` to false.
    After connection close check `handlerNotified` and if it wasn't, then check if somebody
has set `HTTP_HANDLER` externally.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/hirthwork/httpcore 4.4.x

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/httpcore/pull/21.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #21
    
----
commit aa680cafe67e92dfe7bdb96e03c67bb9f1905b87
Author: Dmitry Potapov <dpotapov@yandex-team.ru>
Date:   2016-02-05T18:44:15Z

    HttpAsyncRequestExecutor.getState(conn) and HttpAsyncRequestExecutor.getHandler(conn)
made static

commit 363df092bfbabe8b32e2223615af0671f9846bcb
Author: Dmitry Potapov <dpotapov@yandex-team.ru>
Date:   2016-02-05T18:47:28Z

    Double check for HttpAsyncClientExchangeHandler presence in HttpAsyncRequestExecutor.endOfInput

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Mime
View raw message