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

On 11/26/2011 06:58 AM, Amos Jeffries wrote:
> 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?

I have not heard anything, unfortunately.

Alex.
Received on Tue Nov 29 2011 - 17:23:48 MST