Re: [PATCH 19/19] Linux: Use __pthread_attr_setsigmask_internal for timer helper thread

[Carlos O'Donell <carlos@redhat.com>]

On 5/19/20 6:45 AM, Florian Weimer via Libc-alpha wrote:
> timer_create needs to create threads with all signals blocked,
> including SIGTIMER (which happens to equal SIGCANCEL).  Add a new
> internal interface which provides an explicit way to achieve that.

OK with commit message fixed (last sentence removed).

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> 
> Fixes commit b3cae39dcbfa2432b3f3aa28854d8ac57f0de1b8 ("nptl: Start
> new threads with all signals blocked [BZ #25098]").
> ---
>  sysdeps/unix/sysv/linux/timer_routines.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/timer_routines.c b/sysdeps/unix/sysv/linux/timer_routines.c
> index 63083f6f91..86fad2a2c0 100644
> --- a/sysdeps/unix/sysv/linux/timer_routines.c
> +++ b/sysdeps/unix/sysv/linux/timer_routines.c
> @@ -136,23 +136,24 @@ __start_helper_thread (void)
>    (void) pthread_attr_init (&attr);
>    (void) pthread_attr_setstacksize (&attr, __pthread_get_minstack (&attr));
>  
> -  /* Block all signals in the helper thread but SIGSETXID.  To do this
> -     thoroughly we temporarily have to block all signals here.  The
> -     helper can lose wakeups if SIGTIMER is not blocked throughout.  */
> +  /* Block all signals in the helper thread but SIGSETXID.  */
>    sigset_t ss;
> -  __libc_signal_block_app (&ss);
> -  __libc_signal_block_sigtimer (NULL);
> +  __sigfillset (&ss);
> +  __sigdelset (&ss, SIGSETXID);
> +  int res = __pthread_attr_setsigmask_internal (&attr, &ss);

OK. Yay, use the interface :-)

> +  if (res != 0)
> +    {
> +      pthread_attr_destroy (&attr);
> +      return;
> +    }
>  
>    /* Create the helper thread for this timer.  */
>    pthread_t th;
> -  int res = pthread_create (&th, &attr, timer_helper_thread, NULL);
> +  res = pthread_create (&th, &attr, timer_helper_thread, NULL);

OK.

>    if (res == 0)
>      /* We managed to start the helper thread.  */
>      __helper_tid = ((struct pthread *) th)->tid;
>  
> -  /* Restore the signal mask.  */
> -  __libc_signal_restore_set (&ss);

OK.

> -
>    /* No need for the attribute anymore.  */
>    (void) pthread_attr_destroy (&attr);
>  
> 


-- 
Cheers,
Carlos.