[squid-dev] New Defects reported by Coverity Scan for Squid after IndependentRunner

[Alex Rousskov]

On 09/10/2016 06:54 AM, Amos Jeffries wrote:
> 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.

>>>>> Well, the registerRunner() method should be used instead of
>>>>> RegisterRunner for ConnStateData and IDleConnList.

>>>> such a major bug should not have went unnoticed

>>> But it is not exactly bug.
>>> 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.

Yes, the registration may be "late", but the committed code supports
late registrations well AFAIK -- there is no functionality bug. There is
an unused method. I trust Christos will fix that (and improve
surrounding code while he is at it).

I misinterpreted Christos earlier response and thought that there is a
major bug that we missed. That major bug does not exist. The code works
OK but needs some polishing.


> What I was thinking was to have this:
> 
>  IndependedRunner::registerRunner() {
>     Must(RegisterRunner(this));
>  }

It is wrong to throw when things are working OK. A failed RR
registration is not a bug; it is only a late registration that can be
ignored.

Alex.