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] [Resolved] (HTTPASYNC-56) Deadlock in DefaultClientExchangeHandlerImpl.start()
Date Sun, 13 Oct 2013 12:25:43 GMT

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

Oleg Kalnichevski resolved HTTPASYNC-56.
----------------------------------------

    Resolution: Fixed

I moved code requiring synchronization out of #connectionAllocated to other methods executed
by I/O dispatch threads and now I think the problem has been properly resolved. Thread safety
in DefaultClientExchangeHandlerImpl is now achieved by the use of atomic references and booleans
in critical sections rather than through synchonisation on method level.

Please re-test / review.

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


Mime
View raw message