[PATCH] Bug 3420: Request body consumption races and !theConsumer

From: Alex Rousskov <rousskov_at_measurement-factory.com>
Date: Thu, 17 Nov 2011 15:51:31 -0700

Hello,

    Closed bug 3190, open bug 3420, open bug 3417, and several
unreported problems deal with stuck requests, early responses, aborted
requests/responses and combination of those. The attached patch attempts
to solve many of these mushrooming problems, but it is a significant
change that may introduce other bugs. Please review.

The patch is against v3.1 because most sufferers are running that
version. I will port changes to trunk if approved. Technical details are
copied below from the patch preamble.

Before these changes, the client side used a single "closing" state to
handle two different error conditions:

  1. We stopped receiving request body because of some error.
  2. We stopped sending response because of some error.

When a "directional" error occurred, we try to keep the transaction
going in the other direction (e.g., to give ICAP the entire request or
to give HTTP client the entire response). However, because there was
just one "closing" state, the code failed to correctly detect or process
many corner cases, resulting in stuck transactions and !theConsumer
assertions/exceptions due to races between enableAutoConsumption() and
expectNoConsumption() calls.

This patch replaces the "closing" state with two direction-specific "we
stopped sending/receiving" flags.

Now, when the response sending code is done, it now checks whether the
receiving code stopped and closes the connection as needed. This is done
both when we encounter a sending error
(ClientSocketContext::initiateClose) and when we successfully sent the
entire response to the client (ClientSocketContext::keepaliveNextRequest).

Similarly, when the request body reading code is done, it now checks
whether the receiving code stopped and closes the connection as needed.
This is done both when we encounter a receiving error
(ConnStateData::noteBodyConsumerAborted) and when we successfully
receive the entire request body from the client
(ClientSocketContext::writeComplete).

TODO: This patch focuses on various error cases. We might still have
problems when there is an early HTTP response and no errors of any kind.
I marked the corresponding old code with an XXX.

Thank you,

Alex.

Received on Thu Nov 17 2011 - 22:51:56 MST

This archive was generated by hypermail 2.2.0 : Sat Nov 26 2011 - 12:00:07 MST