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

On 18/11/2011 11:51 a.m., Alex Rousskov wrote:
> 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.

Nearly been 10 days. Any progress on usage testing feedback?

Amos
Received on Sat Nov 26 2011 - 13:58:54 MST