Re: [PATCH] Bug 3281

On 09/02/2011 02:12 AM, Amos Jeffries wrote:
> On 02/09/11 18:29, Amos Jeffries wrote:
>> On 02/09/11 04:55, Alex Rousskov wrote:
>>> On 09/01/2011 05:47 AM, Amos Jeffries wrote:
>>>> This patch seeks to avoids race conditions between the AsyncCalls for
>>>> the pconn monitoring handlers and lookups pulling pconn up for use.

>> I'll post a new patch shortly. With isAvailable() protecting against
>> unsafe re-use cases the handlers on both read and timeout can now
>> unconditionally cleanup and close the idle conn. Read callback may not
>> even need to bother about the CLOSING flag, but I think the forced
>> fd_table[] close cycle on shutdown (bug 2110) might still cause that
>> case.
>>
>
> ... and here is that.

> + int index = findIndexOf(conn);

Please declare the index variable const if possible.

I would at least add a TODO to the timeout check because we know it
should not, ideally, be there. For example,

    // TODO: cancel timeouts of popped pconns to remove this check

Other than that, I do not have any new comments about the patch. I think
it may go in.

Thank you,

Alex.
P.S. Please try to post patches with much larger context depth. They are
easier to review correctly, IMO.
Received on Fri Sep 02 2011 - 21:34:59 MDT