Re: [Tarantool-patches] [PATCH 1/1] fiber_cond: remove rlist_shift usages

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On 02.03.2021 10:04, Serge Petrenko wrote:
> 
> 
> 02.03.2021 10:37, Cyrill Gorcunov пишет:
>> On Mon, Mar 01, 2021 at 11:23:13PM +0100, Vladislav Shpilevoy wrote:
>>> @@ -94,7 +94,7 @@ fiber_cond_broadcast(struct fiber_cond *e)
>>>   {
>>>       while (! rlist_empty(&e->waiters)) {
>>>           struct fiber *f;
>>> -        f = rlist_shift_entry(&e->waiters, struct fiber, state);
>>> +        f = rlist_first_entry(&e->waiters, struct fiber, state);
>>>           fiber_wakeup(f);
>>>       }
>>>   }
>> The fiber_wakeup ignores
>>
>>     if (f->flags & (FIBER_IS_READY | FIBER_IS_DEAD))
>>         return;
>>
>> can't we hit the situation where fiber_cond_broadcast called with
>> dead fiber so that it won't be deleted from the list with new code?
> 
> Good point, I never thought of this.
> 
> Once a fiber dies, it's removed from any list that could wake it up:
> 
>         /* reset pending wakeups */
>         rlist_del(&fiber->state);
> 
> And FIBER_IS_READY is set only from fiber_wakeup() to guard from
> concurrent wake-ups. So the patch must be fine indeed.

Moreover, a fiber can't be dead and be in fiber_cond. Fiber dies only
after it returns from its main function. Until then it can be at most
CANCELLED, which does not affect the yields. You still can sleep,
wait, yield, be woken up, and so on.