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

Message ID a36f025f85c1d8481308023ad3864ea476c4f75f.1589884403.git.fweimer@redhat.com
State Committed
Headers
Series Signal mask for timer helper thread |

Commit Message

Florian Weimer May 19, 2020, 10:45 a.m. UTC
  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.

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(-)
  

Comments

Carlos O'Donell June 2, 2020, 4:07 a.m. UTC | #1
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);
>  
>
  

Patch

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);
+  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);
   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);
-
   /* No need for the attribute anymore.  */
   (void) pthread_attr_destroy (&attr);