[
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?
Oleg
> 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 methods.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
