puppetlabs / clj-http-client

HTTP client library wrapping Apache HttpAsyncClient
Apache License 2.0
15 stars 30 forks source link

(TK-101) Move closing of http clients to a separate thread #21

Closed camlow325 closed 10 years ago

camlow325 commented 10 years ago

This commit moves each close call made on a CloseableHttpAsyncClient from the thread on which the request callback is made over to a different thread in order to avoid a deadlock.

Previously, the client close call was made in the context of thread from which the callback was invoked. The Apache HTTP async client library makes these calls from one of its i/o reactor threads. In the case of a "failed" request, e.g., failure to make the client connection, this call occurs in the context of the primary i/o reactor thread. The client close call does an indefinite join on the primary i/o reactor thread and, therefore, this call causes a deadlock. The client never closes and all resources associated with it - threads, file descriptors, etc. - remain open until the process is shutdown.

This commit introduces a new class, AsyncClose, which manages a small, fixed size thread pool from which client close calls can be executed. The async Clojure and JavaClient APIs were updated to use AsyncClose whenever a client needs to be closed from a callback.

cprice404 commented 10 years ago

+1 :disappointed_relieved: I hate this but I think this is about as clean as it could be implemented, and after spending almost an hour talking through short-term and long-term solutions for this, I don't think there is a better short-term fix.

KevinCorcoran commented 10 years ago

gross but necessary, i guess. :busts_in_silhouette: