Re: [PATCH] Bug 3281

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.
>>
>> ...
>>
>>> It adds checks for the timeout handler, as discussed in bug 3281,
>>> preventing the related type of race from the timeout handler.
>>
>>> + // our connection timeout handler is scheduled to run already. unsafe
>>> + if (fd_table[conn->fd].timeoutHandler == NULL)
>>> + return false;
>>
>> I am not strongly against this, but it would be more elegant and may be
>> more efficient to let the about-to-timeout connections to be reused
>> instead. This can be safely done, for example, by canceling the pending
>> timeout notification callback in the connection giving code.
>
> Cancelling calls here would mean re-writing IdleConnList to store those
> calls to be cancelled. As it stands right now once scheduled there is no
> code outside the AsyncEngine with a pointer to them.
> This patch would still be needed after all that change but would cancel
> and return true instead of just returning false.
>
> 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.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.15
   Beta testers wanted for 3.2.0.10