httpclient icon indicating copy to clipboard operation
httpclient copied to clipboard

Asynchronous Connection prone to deadlock for large response bodies

Open teleological opened this issue 13 years ago • 2 comments

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.

teleological avatar Oct 29 '11 22:10 teleological

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...

nahi avatar Apr 19 '12 07:04 nahi

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?

teleological avatar Apr 19 '12 13:04 teleological