[PATCH v2 05/19] nptl: Move setxid flag out of cancelhandling

[Florian Weimer]

* Adhemerval Zanella:

> On 26/08/2021 12:21, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 26/08/2021 08:34, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> Now that the thread state is tracked by 'joinstate' field, there is no
>>>>> need to keep the setxid flag within the 'cancelhandling'.  It simplifies
>>>>> the atomic code to set and reset it (since there is no need to handle
>>>>> the thread state concurrent update).
>>>>
>>>> Some of this functionality reimplements the tidlock.
>>>>
>>>> There is some functionality around thread creation, but I do not know if
>>>> the synchronization is entirely correct.  This makes it rather difficult
>>>> to review changes.
>>>>
>>>> It's also not clear to me if we need to do anything to prevent early
>>>> setxid calls:
>>>>
>>>>>  	  /* Don't allow setxid until cloned.  */
>>>>
>>>> I suspect this should just look at the TID instead.  We create the
>>>> thread with all signals blocked, so once there is a TID (and the thread
>>>> has not exited), it is safe to send the signal.
>>>
>>> I agree that we might improve the setxid code, however what I intended
>>> with this changes is to keep its semantic as-is and just decouple it
>>> from cancelhandling.  I might revise it on a later patch to follow your
>>> suggestions.
>> 
>> I wonder if there is a new race here:
>> 
>> @@ -109,41 +100,23 @@ setxid_mark_thread (struct xid_command *cmdp, struct pthread *t)
>>    /* Don't let the thread exit before the setxid handler runs.  */
>>    t->setxid_futex = 0;
>>  
>> +  /* If thread is exiting right now, ignore it.  */
>> +  if (atomic_load_acquire (&t->joinstate) == THREAD_STATE_EXITING)
>> +    return;
>>  
>> +  /* Release the futex if there is no other setxid in progress.  */
>> +  if (atomic_exchange_acquire (&t->setxid_flag, 1) == 0)
>> +    {
>> +      t->setxid_futex = 1;
>> +      futex_wake (&t->setxid_futex, 1, FUTEX_PRIVATE);
>>      }
>> 
>> That is, if the setxid-sending thread can now wait for a futex wake that
>> never comes.
>
> Wouldn't be handled by setxid_unmark_thread, which wakes the futex
> unconditionally?

But that code only runs after the loop around signalled has concluded.
That's why I'm worried.

Thanks,
Florian