Re: [PATCH 3/3] nptl: Fix race between pthread_kill and thread exit (bug 12889)

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 19/08/2021 13:37, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> On 17/08/2021 10:51, Florian Weimer via Libc-alpha wrote:
>>> Thread exit is delayed until all pending pthread_kill operations
>>> on the thread have completed.  This avoids sending signals to the
>>> wrong thread or a spurious ESRCH error.
>>>
>>> The test sysdeps/pthread/tst-pthread_cancel-select-loop.c is derived
>>> from a downstream test originally written by Marek Polacek.
>>
>> And I really hope we could first sort out the BZ#19951 and move away
>> of using pthread::tid as the synchronization member to check thread
>> termination.  I sent a possible fix [1], but it does not fully handle
>> the pthread::tid access, it would require to a proper lock to 
>> synchronize between thread exit and pthread_kill (as Rich has suggested
>> on BZ#12889).
>>
>> I will need to send an updated version of my pthread fixes patchset,
>> since clone3 changes broke it and INVALID_TD_P needs fixing as well.
>>
>> [1] https://patchwork.sourceware.org/project/glibc/patch/20210610193639.3650754-5-adhemerval.zanella@linaro.org/
> 
> I think my patch is independent of the other fixes.  Its advantage over
> other proposals is that it does not add waiting to pthread_kill, so
> there is no signal-safety issue.

I am not sure, the way I think we should fix to also fix BZ#19951
would to add a lock to access 'tid' and uses not only on pthread_kill,
but also on pthread_getcpuclockid, pthread_getschedparam, 
pthread_setschedparam, and pthread_setschedprio.  The lock also
simplifies the code, there is no need to special handling of futex
internal state. I also think pthread_kill would also be simplified.

The lock is just to simplify the code, if we really need to add
some scalability we might use a more clever code as you are proposing.

> 
> (But I probably should test if cancellation from a signal handler that
> is invoked synchronously via pthread_kill still works.)
> 
> Thanks,
> Florian
>