Internal SIGTIMER use (was: Re: [PATCH v3 02/21] nptl: Fix Race conditions in pthread cancellation (BZ#12683))

[Adhemerval Zanella]

On 18/10/2019 09:37, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> -  sa.sa_sigaction = sigcancel_handler;
>> -  sa.sa_flags = SA_SIGINFO;
>> -  (void) __libc_sigaction (SIGCANCEL, &sa, NULL);
>> +  {
>> +    struct sigaction sa;
>> +    sa.sa_sigaction = sigcancel_handler;
>> +    /* The signal handle should be non-interruptible to avoid the risk of
>> +       spurious EINTR caused by SIGCANCEL sent to process or if pthread_cancel
>> +       is called while cancellation is disabled in the target thread.  */
>> +    sa.sa_flags = SA_SIGINFO | SA_RESTART;
>> +    sa.sa_mask = SIGALL_SET;
>> +    __libc_sigaction (SIGCANCEL, &sa, NULL);
>> +  }
> 
> Since SIGCANCEL and SIGTIMER are the same, I wondered whether this
> change impacts timer_create.
> 
> Here is what i found: timer_create arranges for SIGCANCEL/SIGTIMER to be
> sent to the internal helper thread, which does this:
> 
>       /* sigwaitinfo cannot be used here, since it deletes
>          SIGCANCEL == SIGTIMER from the set.  */
> 
>       /* XXX The size argument hopefully will have to be changed to the
>          real size of the user-level sigset_t.  */
>       int result = SYSCALL_CANCEL (rt_sigtimedwait, &ss, &si, NULL, _NSIG / 8);
> 
>       if (result > 0)
>         {
>           if (si.si_code == SI_TIMER)
>             {
>               struct timer *tk = (struct timer *) si.si_ptr;
> …
>           else if (si.si_code == SI_TKILL)
>             /* The thread is canceled.  */
>             pthread_exit (NULL);
>         }
> 
> This suggests to me that the helper thread does NOT depend on EINTR
> being generated for SIGCANCEL/SIGTIMER, and it should be fine to use
> SA_RESTART for that signal as far as timer_create is concerned.
> 
> If you agree, it probably makes sense to add this bit of information to
> the commit message.

Ack, this is what I have added:

--

As a side note regarding SIGCANCEL and SIGTIMER being the the same,
it should not impact timer_create functionality.  It arranges for
SIGCANCEL/SIGTIMER to be sent to the internal helper thread, which
in turn check if the si.si_code is SI_TIMER and call pthread_exit
otherwise (sysdeps/unix/sysv/linux/timer_routines.c:129).

This suggests that the helper thread does NOT depend on EINTR
being generated for SIGCANCEL/SIGTIMER, and it should be fine to use
SA_RESTART for that signal as far as timer_create is concerned.