hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Oleg Kalnichevski (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HTTPASYNC-56) Deadlock in DefaultClientExchangeHandlerImpl.start()
Date Sat, 12 Oct 2013 15:34:43 GMT

    [ https://issues.apache.org/jira/browse/HTTPASYNC-56?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13793373#comment-13793373

Oleg Kalnichevski commented on HTTPASYNC-56:

Actually method level synchronization in DefaultClientExchangeHandlerImpl is not warranted.
Instances of this class are expected to be accessed by one thread at a time. Only #cancel
and #close methods can be used safely by multiple threads concurrently. In all other cases
the I/O reactor ensures only one I/O dispatch thread ever interacts with the handler at a
time. There is only one fringe case I can think of when the same handler might end up being
accessed concurrently my two threads: when the opposite end sends an out of sequence response
(without a prior request) while the same connection is being leased from the pool. I need
to think whether or not the overhead of synchronization is actually justifies a more graceful
handling of such an extreme case.

Could you please re-test your application against the latest snapshot?


> Deadlock in DefaultClientExchangeHandlerImpl.start()
> ----------------------------------------------------
>                 Key: HTTPASYNC-56
>                 URL: https://issues.apache.org/jira/browse/HTTPASYNC-56
>             Project: HttpComponents HttpAsyncClient
>          Issue Type: Bug
>    Affects Versions: 4.0-beta4
>            Reporter: Dmitry Potapov
>             Fix For: 4.0 Final
> How it looks like:
> 1. Theads A and B calls CloseableHttpAsyncClient.execute() of the same CloseableHttpAsyncClient
(problem may reproduce on two separate clients sharing single connection pool)
> 2. Each CloseableHttpAsyncClient.execute in turn calls InternalHttpAsyncClient.execute()
which creates DefaultClientExchangeHandlerImpl instance and calls DefaultClientExchangeHandlerImpl.start(),
which is synchronized
> 3. At this point, we have two DefaultClientExchangeHandlerImpl with locked monitors,
let these instances have names AH and BH.
> 4. DefaultClientExchangeHandlerImpl.start() calls requestConnection(), which in turn
calls PoolingNHttpClientConnectionManager.requestConnection()
> 5. At thread A: AbstractNIOConnPool.lease() adds completed request to the completedRequests
queue (line 271). This request callback has reference to the AH
> 6. At thread B: AbstractNIOConnPool.lease() adds completed request to the completedRequests
queue. This request callback has reference to BH
> 7. At thread B: AbstractNIOConnPool.fireCallbacks() is called. It polls AH from completedRequests
and calls AH callback, which tries to enter AH monitor and locks, because this monitor is
already locked.
> 8. At thread A: AbstractNIOConnPool.fireCallbacks() is called. It polls BH from completedRequests
(AH was polled at step 7) and calls BH callback, which tries to enter BH monitor and locks,
because this monitor is already locked.
> At this point we have threads A and B deadlocked.
> I have obvious solution for this particular dead-lock: make DefaultClientExchangeHandlerImpl.start()
not synchronized, because this object created only at single point, and .start() is called
immediately after construction.
> I'm not sure that there is no problems in other scenarious where .fireCallbacks() involved,
because DefaultClientExchangeHandlerImpl.requestConnection() may be called from other synchronized

This message was sent by Atlassian JIRA

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

View raw message