hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dmitry Potapov (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HTTPASYNC-56) Deadlock in DefaultClientExchangeHandlerImpl.start()
Date Sun, 13 Oct 2013 12:50:42 GMT

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

Dmitry Potapov commented on HTTPASYNC-56:
-----------------------------------------

Oleg,

Thank you for this fix. I'll do the best I can to put latest snapshot under heavy load at
Monday evening.

Concenrning this fix, I have only one notice. Why did you changed DefaultClientExchangeHandlerImpl.completed
from "volatile boolean" to "AtomicBoolean"? The only functions used are get() and set(true),
in such use case atomics has no advantages over volatiles. Please, correct me if I'm wrong.

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


Mime
View raw message