httpclient
httpclient copied to clipboard
Asynchronous Connection prone to deadlock for large response bodies
The "producer" thread spawned by #do_request_async writes the response body to a pipe. When the pipe becomes full due to being written to without being read, subsequent writes to the pipe cause the producer thread to block.
If a "consumer" thread which has invoked the asynchronous request attempts to join the producer thread (via Connection#join) before the producer thread has finished retrieving the response body and writing it to the pipe, a deadlock will occur if:
- no other threads read from the response body (Body#content), and either
- the producer thread is already blocking on a write to a full pipe, or
- the remaining capacity of the pipe is smaller than the remaining response body to be fetched by the producer thread, causing the producer thread to block on a subsequent write
The following test demonstrates the problem:
def test_get_async_deadlock_on_join
conn = @client.get_async(serverurl + 'largebody')
conn.join # deadlock: mutual blocking
res = conn.pop
assert_equal(1000*1000, res.content.read.length) # never executed
end
A variant of the problem occurs if instead of joining the producer thread, the consumer thread spins until the producer thread reports that it is finished (Connection#finished?):
def test_get_async_spin_never_finished
conn = @client.get_async(serverurl + 'largebody')
Thread.pass until conn.finished? # spins perpetually
res = conn.pop
assert_equal(1000*1000, res.content.read.length) # never executed
end
The first problem can be avoided if you never #join a connection without first testing if it is #finished?, but in that case the join is redundant, and introduces the second problem: How can you determine when the connection is finished?
A blocking read (as in #test_get_async_for_largebody) or a loop that makes non-blocking reads from Body#content without first joining the connection or checking that it is finished seems to work, although there could be an issue if #do_get_stream raises an exception without closing the writable end of the pipe.
Thanks for the explanation. Indeed the code is bad from the beginning, I should have not used pipe. I'm thinking how I can fix this issue...
It seems to me that the pipe is useful in so far as it does not require an unlimited buffer: But maybe it could be replaced by a user-sizable buffer?