On 10/09/2016 7:26 a.m., Alex Rousskov wrote:
> On 09/09/2016 11:21 AM, Christos Tsantilas wrote:
>> On 09/09/2016 07:00 PM, Alex Rousskov wrote:
>>> On 09/09/2016 07:34 AM, Christos Tsantilas wrote:
>>>> On 09/09/2016 02:21 PM, Amos Jeffries wrote:
>>>>> Also the IndependentRunner::registerRunner() method is not used
>>>>> anywhere. Was it supposed to be called by the child classes ?
>>>>> (IdleConnList and ConnStateData)
>
>>>> Well, the registerRunner() method should be used instead of
>>>> RegisterRunner for ConnStateData and IDleConnList. But this change
>>>> somewhere lost while I was playing with the patches.
>
>>> [rant] Which means our testing was inadequate again. Accidents happen,
>>> but such a major bug should not have went unnoticed if proper testing
>>> was in place.[/rant]
>
>> But it is not exactly bug. The registereRunner() method just calls
>> "RegisterRunner(this)", it does not do anything else.
>> Just we did not replace RegisterRunner(this); with the
>> IndependedRunner::registerRunner call.
>
> Ah, this is minor, and no functionality testing could expose this. I
> incorrectly thought that we are not registering independent runners at all.
>
Well it is rare-ish, but I think it can happen with ConnStateData if a
connection is accepted after shutdown starts during the grace period
witing for client connections to finish up.
> BTW, you can probably move the sketched RegisterRunnerUnlessShutdown()
> function into the IndependedRunner::registerRunner() itself and also add
> a Must(not an independent runner) check to the sketched
> RegisterOrGetRidOfRunner().
What I was thinking was to have this:
IndependedRunner::registerRunner() {
Must(RegisterRunner(this));
}
constructors are allowed to throw, so if it is not caught by the
Call/callback handler from accept() thats a separate issue to fix.
Amos
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
