[v2] linux: Do not spawn a new thread for SIGEV_THREAD (BZ 30558, 27895, 29705, 32833)

The current timer_create SIGEV_THREAD implementation has some
downsides:

  1. There is no way to report failure at thread creation when a
     timer triggers.  It means that it might occur unreported and with
     missed events depending of the system load.

  2. The backgroup thread also kept in backgroun even when there is no
     more timers, consuming resources and also misleading memory
     profile tools (BZ 29705).

  3. There is a lot of metadata that required to be kept: a control
     variable for helper thread creation, a list of active SIGEV_THREAD
     timers, atfork handlers to cleanup the list.

  4. timer_create does not propagate all thread attributes to the new
     thread (BZ 27895).

  5. Kernel might deliver in-flight events for a timer after it was
     destroyed by timer_delete.  The timer_helper_thread mechanism to
     handle it does not cover all possible issue, which leads to
     callbacks being wrong triggered (BZ 32833).

This new implementation moves the thread creation to timer_create, so
any failure is reported to the caller.  Also, the same thread will
issues the multiple timers, thus there is no unreported missed events.
Also, avoiding parallel timer activation also avoid possible parallel
timer invocation to see the same overrun value.

To implement using SIGTIMER internally as SIGCANCEL, it requires to
mask out SIGCANCEL on thread creation.  It essentially disable async
thread cancellation, but POSIX requires that SIGEV_THREAD is always
created in detached mode and cancelling detached thread s UB (glibc
check the internal tid, but the memory referenced by pthread_t might
not always be valid as the momento of pthread_cancel call).

And to avoid the need to recreate the thread for pthread_exit call
(and having possible unreported missed due failed thread creation),
the SIGEV_THREAD install a cleanup handler that reset all internal
thread state.

It also prevents the re-use issue when a newly-allocated timer has
in-flight event being delivered by the kernel (BZ 32833).

Performance-wise it see it uses less CPU timer for multiple thread
activation, although each thread now requires a sigwaitinfo which
generate more context-switches/page-faults (check comment 7 from
BZ 30558).  I would expect that latency should improve, since it
avoid a thread creation for each timer expiration.

Checked on aarch64-linux-gnu, x86_64-linux-gnu and i686-linux-gnu.

--
Changes from v2:
- Fixed some issues with timer_delete due using timeid to signal the
  thread.
- Added BZ#32833 as fixed bug.
- Rebased against master.

---
 nptl/descr.h                                  |   3 +
 rt/Makefile                                   |   4 +-
 rt/tst-timer-sigmask.c                        |   7 +-
 rt/tst-timer6.c                               |  79 +++++
 sysdeps/nptl/Makefile                         |   2 -
 sysdeps/nptl/fork.h                           |   2 -
 sysdeps/unix/sysv/linux/internal-signals.h    |   8 -
 .../unix/sysv/linux/kernel-posix-cpu-timers.h |   2 +
 sysdeps/unix/sysv/linux/kernel-posix-timers.h |  75 ++---
 sysdeps/unix/sysv/linux/timer_create.c        | 289 ++++++++++++------
 sysdeps/unix/sysv/linux/timer_delete.c        |  46 +--
 sysdeps/unix/sysv/linux/timer_routines.c      | 154 ----------
 12 files changed, 323 insertions(+), 348 deletions(-)
 create mode 100644 rt/tst-timer6.c
 delete mode 100644 sysdeps/unix/sysv/linux/timer_routines.c

fwiw, this is how bionic always worked, and i've never seen any
reports of compatibility issues with code expecting the old glibc
semantics. (to the extent that i was not aware glibc didn't already
work like this!)

On Fri, Apr 4, 2025 at 2:08 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> The current timer_create SIGEV_THREAD implementation has some
> downsides:
>
>   1. There is no way to report failure at thread creation when a
>      timer triggers.  It means that it might occur unreported and with
>      missed events depending of the system load.
>
>   2. The backgroup thread also kept in backgroun even when there is no
>      more timers, consuming resources and also misleading memory
>      profile tools (BZ 29705).
>
>   3. There is a lot of metadata that required to be kept: a control
>      variable for helper thread creation, a list of active SIGEV_THREAD
>      timers, atfork handlers to cleanup the list.
>
>   4. timer_create does not propagate all thread attributes to the new
>      thread (BZ 27895).
>
>   5. Kernel might deliver in-flight events for a timer after it was
>      destroyed by timer_delete.  The timer_helper_thread mechanism to
>      handle it does not cover all possible issue, which leads to
>      callbacks being wrong triggered (BZ 32833).
>
> This new implementation moves the thread creation to timer_create, so
> any failure is reported to the caller.  Also, the same thread will
> issues the multiple timers, thus there is no unreported missed events.
> Also, avoiding parallel timer activation also avoid possible parallel
> timer invocation to see the same overrun value.
>
> To implement using SIGTIMER internally as SIGCANCEL, it requires to
> mask out SIGCANCEL on thread creation.  It essentially disable async
> thread cancellation, but POSIX requires that SIGEV_THREAD is always
> created in detached mode and cancelling detached thread s UB (glibc
> check the internal tid, but the memory referenced by pthread_t might
> not always be valid as the momento of pthread_cancel call).
>
> And to avoid the need to recreate the thread for pthread_exit call
> (and having possible unreported missed due failed thread creation),
> the SIGEV_THREAD install a cleanup handler that reset all internal
> thread state.
>
> It also prevents the re-use issue when a newly-allocated timer has
> in-flight event being delivered by the kernel (BZ 32833).
>
> Performance-wise it see it uses less CPU timer for multiple thread
> activation, although each thread now requires a sigwaitinfo which
> generate more context-switches/page-faults (check comment 7 from
> BZ 30558).  I would expect that latency should improve, since it
> avoid a thread creation for each timer expiration.
>
> Checked on aarch64-linux-gnu, x86_64-linux-gnu and i686-linux-gnu.
>
> --
> Changes from v2:
> - Fixed some issues with timer_delete due using timeid to signal the
>   thread.
> - Added BZ#32833 as fixed bug.
> - Rebased against master.
>
> ---
>  nptl/descr.h                                  |   3 +
>  rt/Makefile                                   |   4 +-
>  rt/tst-timer-sigmask.c                        |   7 +-
>  rt/tst-timer6.c                               |  79 +++++
>  sysdeps/nptl/Makefile                         |   2 -
>  sysdeps/nptl/fork.h                           |   2 -
>  sysdeps/unix/sysv/linux/internal-signals.h    |   8 -
>  .../unix/sysv/linux/kernel-posix-cpu-timers.h |   2 +
>  sysdeps/unix/sysv/linux/kernel-posix-timers.h |  75 ++---
>  sysdeps/unix/sysv/linux/timer_create.c        | 289 ++++++++++++------
>  sysdeps/unix/sysv/linux/timer_delete.c        |  46 +--
>  sysdeps/unix/sysv/linux/timer_routines.c      | 154 ----------
>  12 files changed, 323 insertions(+), 348 deletions(-)
>  create mode 100644 rt/tst-timer6.c
>  delete mode 100644 sysdeps/unix/sysv/linux/timer_routines.c
>
> diff --git a/nptl/descr.h b/nptl/descr.h
> index ada6867a19..07eedda067 100644
> --- a/nptl/descr.h
> +++ b/nptl/descr.h
> @@ -413,6 +413,9 @@ struct pthread
>    /* getrandom vDSO per-thread opaque state.  */
>    void *getrandom_buf;
>
> +  /* POSIX per-process timer.  */
> +  int timerid;
> +
>    /* Amount of end padding, if any, in this structure.
>       This definition relies on getrandom_buf being last.  */
>  #define PTHREAD_STRUCT_END_PADDING \
> diff --git a/rt/Makefile b/rt/Makefile
> index 8880e25b64..bdda9dd660 100644
> --- a/rt/Makefile
> +++ b/rt/Makefile
> @@ -79,7 +79,8 @@ tests := tst-shm tst-timer tst-timer2 \
>          tst-cpuclock2 tst-cputimer1 tst-cputimer2 tst-cputimer3 \
>          tst-clock_nanosleep2 \
>          tst-shm-cancel \
> -        tst-mqueue10
> +        tst-mqueue10 \
> +        tst-timer6
>  tests-internal := tst-timer-sigmask
>
>  tests-time64 := \
> @@ -101,6 +102,7 @@ include ../Rules
>  CFLAGS-aio_suspend.c += -fexceptions
>  CFLAGS-mq_timedreceive.c += -fexceptions -fasynchronous-unwind-tables
>  CFLAGS-mq_timedsend.c += -fexceptions -fasynchronous-unwind-tables
> +CFLAGS-timer_create.c += -fexceptions -fasynchronous-unwind-tables
>
>  # Exclude fortified routines from being built with _FORTIFY_SOURCE
>  routines_no_fortify += \
> diff --git a/rt/tst-timer-sigmask.c b/rt/tst-timer-sigmask.c
> index d8a576bba7..61b7927863 100644
> --- a/rt/tst-timer-sigmask.c
> +++ b/rt/tst-timer-sigmask.c
> @@ -39,12 +39,9 @@ thread_handler (union sigval sv)
>    for (int sig = 1; sig < NSIG; sig++)
>      {
>        /* POSIX timers threads created to handle SIGEV_THREAD block all
> -        signals except SIGKILL, SIGSTOP and glibc internals ones.  */
> +        signals except SIGKILL, SIGSTOP, and SIGSETXID.  */
>        if (sigismember (&ss, sig))
> -       {
> -         TEST_VERIFY (sig != SIGKILL && sig != SIGSTOP);
> -         TEST_VERIFY (!is_internal_signal (sig));
> -       }
> +       TEST_VERIFY (sig != SIGKILL && sig != SIGSTOP && sig != SIGSETXID);
>        if (test_verbose && sigismember (&ss, sig))
>         printf ("%d, ", sig);
>      }
> diff --git a/rt/tst-timer6.c b/rt/tst-timer6.c
> new file mode 100644
> index 0000000000..d0f3b030b6
> --- /dev/null
> +++ b/rt/tst-timer6.c
> @@ -0,0 +1,79 @@
> +/* Check re-use timer id for SIGEV_THREAD (BZ 32833)
> +   Copyright (C) 2025 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public License as
> +   published by the Free Software Foundation; either version 2.1 of the
> +   License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; see the file COPYING.LIB.  If
> +   not, see <https://www.gnu.org/licenses/>.  */
> +
> +#include <signal.h>
> +#include <time.h>
> +#include <support/check.h>
> +
> +/* The test depends of the system load and scheduler pressure, so the
> +   number of iteration is arbitrary to not take too much time.  */
> +enum { niters = 1<<13 };
> +
> +static void
> +on_good_timer (union sigval sv)
> +{
> +}
> +
> +static void
> +on_bad_timer (union sigval sv)
> +{
> +  FAIL_EXIT1 ("triggered bad timer");
> +}
> +
> +static int
> +do_test (void)
> +{
> +  struct itimerspec its_long =  {. it_value = { .tv_sec = 180 } };
> +  struct itimerspec its_short = { .it_value = { .tv_nsec = 1000 } };
> +  struct itimerspec its_zero =  { .it_interval = { .tv_sec = 0} };
> +
> +  struct sigevent ev_short =
> +    {
> +      .sigev_notify = SIGEV_THREAD,
> +      .sigev_notify_function = on_good_timer,
> +    };
> +
> +  struct sigevent ev_long =
> +    {
> +      .sigev_notify = SIGEV_THREAD,
> +      .sigev_notify_function = on_bad_timer,
> +    };
> +
> +  for (int which = 0; which < niters; which++)
> +    {
> +      struct sigevent * ev = which & 0x1 ? &ev_short : &ev_long;
> +      struct itimerspec * its = which & 0x1? &its_short : &its_long;
> +
> +      timer_t timerid;
> +      if (timer_create (CLOCK_REALTIME, ev, &timerid) == -1)
> +       FAIL_EXIT1 ("timer_create: %m");
> +
> +      if (timer_settime (timerid, 0, its, NULL) == -1)
> +       FAIL_EXIT1 ("timer_settime: %m");
> +
> +      if (timer_settime (timerid, 0, &its_zero, NULL) == -1)
> +       FAIL_EXIT1 ("timer_settime: %m");
> +
> +      if (timer_delete (timerid) == -1)
> +       FAIL_EXIT1 ("time_delete: %m");
> +    }
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/nptl/Makefile b/sysdeps/nptl/Makefile
> index c6e15d2351..12b7cb5bc2 100644
> --- a/sysdeps/nptl/Makefile
> +++ b/sysdeps/nptl/Makefile
> @@ -16,8 +16,6 @@
>  # <https://www.gnu.org/licenses/>.
>
>  ifeq ($(subdir),rt)
> -sysdep_routines += timer_routines
> -
>  tests += tst-mqueue8x
>  CFLAGS-tst-mqueue8x.c += -fexceptions
>  endif
> diff --git a/sysdeps/nptl/fork.h b/sysdeps/nptl/fork.h
> index c7b4a195c1..b241ffaffa 100644
> --- a/sysdeps/nptl/fork.h
> +++ b/sysdeps/nptl/fork.h
> @@ -20,7 +20,6 @@
>  #define _FORK_H
>
>  #include <assert.h>
> -#include <kernel-posix-timers.h>
>  #include <ldsodefs.h>
>  #include <list.h>
>  #include <mqueue.h>
> @@ -46,7 +45,6 @@ fork_system_setup_after_fork (void)
>    __default_pthread_attr_lock = LLL_LOCK_INITIALIZER;
>
>    call_function_static_weak (__mq_notify_fork_subprocess);
> -  call_function_static_weak (__timer_fork_subprocess);
>    call_function_static_weak (__getrandom_fork_subprocess);
>  }
>
> diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
> index ecb00f5f3c..13b840ca08 100644
> --- a/sysdeps/unix/sysv/linux/internal-signals.h
> +++ b/sysdeps/unix/sysv/linux/internal-signals.h
> @@ -108,12 +108,4 @@ static const sigset_t sigtimer_set = {
>    }
>  };
>
> -/* Unblock only SIGTIMER.  */
> -static inline void
> -signal_unblock_sigtimer (void)
> -{
> -  INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_UNBLOCK, &sigtimer_set, NULL,
> -                        __NSIG_BYTES);
> -}
> -
>  #endif
> diff --git a/sysdeps/unix/sysv/linux/kernel-posix-cpu-timers.h b/sysdeps/unix/sysv/linux/kernel-posix-cpu-timers.h
> index bea1e0e62d..eda53be167 100644
> --- a/sysdeps/unix/sysv/linux/kernel-posix-cpu-timers.h
> +++ b/sysdeps/unix/sysv/linux/kernel-posix-cpu-timers.h
> @@ -8,6 +8,8 @@
>    - A clockid is invalid if bits 2, 1, and 0 are all set.
>   */
>
> +#include <time.h>
> +
>  #define CPUCLOCK_PID(clock)            ((pid_t) ~((clock) >> 3))
>  #define CPUCLOCK_PERTHREAD(clock) \
>         (((clock) & (clockid_t) CPUCLOCK_PERTHREAD_MASK) != 0)
> diff --git a/sysdeps/unix/sysv/linux/kernel-posix-timers.h b/sysdeps/unix/sysv/linux/kernel-posix-timers.h
> index 3000953754..046cd3c993 100644
> --- a/sysdeps/unix/sysv/linux/kernel-posix-timers.h
> +++ b/sysdeps/unix/sysv/linux/kernel-posix-timers.h
> @@ -19,29 +19,7 @@
>  #include <setjmp.h>
>  #include <signal.h>
>  #include <sys/types.h>
> -
> -
> -/* Nonzero if the system calls are not available.  */
> -extern int __no_posix_timers attribute_hidden;
> -
> -/* Callback to start helper thread.  */
> -extern void __timer_start_helper_thread (void) attribute_hidden;
> -
> -/* Control variable for helper thread creation.  */
> -extern pthread_once_t __timer_helper_once attribute_hidden;
> -
> -/* Called from fork so that the new subprocess re-creates the
> -   notification thread if necessary.  */
> -void __timer_fork_subprocess (void) attribute_hidden;
> -
> -/* TID of the helper thread.  */
> -extern pid_t __timer_helper_tid attribute_hidden;
> -
> -/* List of active SIGEV_THREAD timers.  */
> -extern struct timer *__timer_active_sigev_thread attribute_hidden;
> -
> -/* Lock for __timer_active_sigev_thread.  */
> -extern pthread_mutex_t __timer_active_sigev_thread_lock attribute_hidden;
> +#include <nptl/descr.h>
>
>  extern __typeof (timer_create) __timer_create;
>  libc_hidden_proto (__timer_create)
> @@ -53,25 +31,12 @@ libc_hidden_proto (__timer_getoverrun)
>  /* Type of timers in the kernel.  */
>  typedef int kernel_timer_t;
>
> -/* Internal representation of SIGEV_THREAD timer.  */
> -struct timer
> -{
> -  kernel_timer_t ktimerid;
> -
> -  void (*thrfunc) (sigval_t);
> -  sigval_t sival;
> -  pthread_attr_t attr;
> -
> -  /* Next element in list of active SIGEV_THREAD timers.  */
> -  struct timer *next;
> -};
> -
> -
>  /* For !SIGEV_THREAD, the resulting 'timer_t' is the returned kernel timer
> -   identifier (kernel_timer_t), while for SIGEV_THREAD it uses the fact malloc
> -   returns at least _Alignof (max_align_t) pointers plus that valid
> -   kernel_timer_t are always positive to set the MSB bit of the returned
> -   'timer_t' to indicate the timer handles a SIGEV_THREAD.  */
> +   identifier (kernel_timer_t), while for SIGEV_THREAD it assumes the
> +   pthread_t at least 8-bytes aligned.
> +
> +   For SIGEV_THREAD, the MSB bit (INT_MAX) is used on timer_delete to
> +   signal the helper thread to stop and issue the timer_delete syscall.  */
>
>  static inline timer_t
>  kernel_timer_to_timerid (kernel_timer_t ktimerid)
> @@ -80,7 +45,7 @@ kernel_timer_to_timerid (kernel_timer_t ktimerid)
>  }
>
>  static inline timer_t
> -timer_to_timerid (struct timer *ptr)
> +pthread_to_timerid (pthread_t ptr)
>  {
>    return (timer_t) (INTPTR_MIN | (uintptr_t) ptr >> 1);
>  }
> @@ -91,19 +56,33 @@ timer_is_sigev_thread (timer_t timerid)
>    return (intptr_t) timerid < 0;
>  }
>
> -static inline struct timer *
> -timerid_to_timer (timer_t timerid)
> +static inline struct pthread *
> +timerid_to_pthread (timer_t timerid)
>  {
> -  return (struct timer *)((uintptr_t) timerid << 1);
> +  return (struct pthread *)((uintptr_t) timerid << 1);
>  }
>
>  static inline kernel_timer_t
>  timerid_to_kernel_timer (timer_t timerid)
>  {
>    if (timer_is_sigev_thread (timerid))
> -    return timerid_to_timer (timerid)->ktimerid;
> -  else
> -    return (kernel_timer_t) ((uintptr_t) timerid);
> +    {
> +      struct pthread *pthr = timerid_to_pthread (timerid);
> +      return pthr->timerid & INT_MAX;
> +    }
> +  return (uintptr_t) timerid;
> +}
> +
> +static inline void
> +timerid_signal_delete (kernel_timer_t *timerid)
> +{
> +  atomic_fetch_or_relaxed (timerid, INT_MIN);
> +}
> +
> +static inline kernel_timer_t
> +timerid_clear (kernel_timer_t timerid)
> +{
> +  return timerid & INT_MAX;
>  }
>
>  /* New targets use int instead of timer_t.  The difference only
> diff --git a/sysdeps/unix/sysv/linux/timer_create.c b/sysdeps/unix/sysv/linux/timer_create.c
> index ca377a69f4..8ec71254a7 100644
> --- a/sysdeps/unix/sysv/linux/timer_create.c
> +++ b/sysdeps/unix/sysv/linux/timer_create.c
> @@ -15,46 +15,196 @@
>     License along with the GNU C Library; see the file COPYING.LIB.  If
>     not, see <https://www.gnu.org/licenses/>.  */
>
> -#include <errno.h>
> -#include <pthread.h>
> -#include <signal.h>
> -#include <stdlib.h>
> -#include <string.h>
> -#include <time.h>
> -#include <sysdep.h>
> -#include <internaltypes.h>
> +#include <jmpbuf-unwind.h>
> +#include <kernel-posix-cpu-timers.h>
> +#include <kernel-posix-timers.h>
> +#include <ldsodefs.h>
> +#include <libc-internal.h>
> +#include <libc-lock.h>
>  #include <pthreadP.h>
> -#include "kernel-posix-timers.h"
> -#include "kernel-posix-cpu-timers.h"
>  #include <shlib-compat.h>
>
> +struct timer_helper_thread_args_t
> +{
> +  /* The barrier is used to synchronize the arguments copy from timer_create
> +     and the SIGEV_THREAD thread and to instruct the thread to exit if the
> +     timer_create syscall fails.  */
> +  pthread_barrier_t b;
> +  struct sigevent *evp;
> +};
> +
> +struct cleanup_args_t
> +{
> +  struct pthread_unwind_buf *cleanup_jmp_buf;
> +  jmp_buf jb;
> +};
> +
> +/* Reset internal thread state if the callback issues pthread_exit.  It avoids
> +   recreating the thread and having possible unreported missed events due
> +   thread creation failure.  */
> +static void
> +timer_helper_thread_cleanup (void *arg)
> +{
> +  struct pthread *self = THREAD_SELF;
> +
> +  /* Call destructors for the thread_local TLS variables.  */
> +  call_function_static_weak (__call_tls_dtors);
> +
> +  /* Run the destructor for the thread-local data.  */
> +  __nptl_deallocate_tsd ();
> +
> +  /* Clean up any state libc stored in thread-local variables.  */
> +  __libc_thread_freeres ();
> +
> +  /* Reset internal TCB state.  */
> +  struct cleanup_args_t *args = arg;
> +  self->cleanup_jmp_buf = args->cleanup_jmp_buf;
> +  self->cleanup_jmp_buf->priv.data.prev = NULL;
> +  self->cleanup_jmp_buf->priv.data.cleanup = NULL;
> +  self->cleanup_jmp_buf->priv.data.canceltype = 0;
> +  self->cleanup = NULL;
> +  self->exc = (struct _Unwind_Exception) { 0 };
> +  self->cancelhandling = 0;
> +  self->nextevent = NULL;
> +
> +  /* Re-initialize the TLS.  */
> +  _dl_allocate_tls_init (TLS_TPADJ (self), true);
> +
> +  /* Reset to the expected initial signal mask.  */
> +  internal_sigset_t ss;
> +  internal_sigfillset (&ss);
> +  internal_sigdelset (&ss, SIGSETXID);
> +  internal_sigprocmask (SIG_SETMASK, &ss, NULL);
> +
> +  /* There is no need to perform any additional cleanup by the frames.  */
> +  struct __jmp_buf_tag *env = args->jb;
> +  __longjmp (env[0].__jmpbuf, 1);
> +}
> +
> +static void *
> +timer_helper_thread (void *arg)
> +{
> +  struct pthread *self = THREAD_SELF;
> +  struct timer_helper_thread_args_t *args = arg;
> +  struct cleanup_args_t clargs = {
> +    .cleanup_jmp_buf = self->cleanup_jmp_buf
> +  };
> +
> +  void (*thrfunc) (sigval_t) = args->evp->sigev_notify_function;
> +  sigval_t sival = args->evp->sigev_value;
> +
> +  __pthread_barrier_wait (&args->b);
> +  /* timer_create syscall failed.  */
> +  if (self->exiting)
> +    return 0;
> +
> +  while (1)
> +    {
> +      siginfo_t si;
> +      while (__sigwaitinfo (&sigtimer_set, &si) < 0);
> +
> +      if (si.si_code == SI_TIMER && !setjmp (clargs.jb))
> +       {
> +         pthread_cleanup_push (timer_helper_thread_cleanup, &clargs);
> +         thrfunc (sival);
> +         pthread_cleanup_pop (0);
> +       }
> +
> +      /* timer_delete will set the MSB and signal the thread.  */
> +      if (self->timerid < 0)
> +       break;
> +    }
> +
> +  /* Clear the MSB bit set by timer_delete.  */
> +  INTERNAL_SYSCALL_CALL (timer_delete, timerid_clear (self->timerid));
> +
> +  return NULL;
> +}
> +
> +static int
> +timer_create_sigev_thread (clockid_t clockid, struct sigevent *evp,
> +                          timer_t *timerid, pthread_attr_t *attr)
> +{
> +  /* Block all signals in the helper thread but SIGSETXID.  */
> +  sigset_t ss;
> +  __sigfillset (&ss);
> +  __sigdelset (&ss, SIGSETXID);
> +  if (__pthread_attr_setsigmask_internal (attr, &ss) < 0)
> +    return -1;
> +
> +  struct timer_helper_thread_args_t args;
> +  __pthread_barrier_init (&args.b, NULL, 2);
> +  args.evp = evp;
> +
> +  pthread_t th;
> +  int r = __pthread_create (&th, attr, timer_helper_thread, &args);
> +  if (r != 0)
> +    {
> +      __set_errno (r);
> +      return -1;
> +    }
> +
> +  struct pthread *pthr = (struct pthread *)th;
> +  struct sigevent kevp =
> +    {
> +      .sigev_value.sival_ptr = NULL,
> +      .sigev_signo = SIGTIMER,
> +      .sigev_notify = SIGEV_THREAD_ID,
> +      ._sigev_un = { ._tid = pthr->tid },
> +    };
> +
> +  kernel_timer_t ktimerid;
> +  if (INLINE_SYSCALL_CALL (timer_create, clockid, &kevp, &ktimerid) < 0)
> +    {
> +      ktimerid = -1;
> +      /* On timer creation failure we need to signal the helper thread to
> +        exit and we can not use the an negative timerid value after the
> +        ptherad_barrier_wait because we can not distinguish between
> +        a timer creation failure and request to delete a timer if it happens
> +        to arrive quickly (for where two timers are create in sequence,
> +        where first succeeds).
> +
> +        We re-use the 'exiting' member to signal the failure, it is set only
> +        at pthread_create to avoid pthread_kill to send further signals.
> +        Since the thread should not be user-visible, signal are only sent
> +        during timer_delete.  */
> +      pthr->exiting = true;
> +    }
> +  pthr->timerid = ktimerid;
> +  /* Signal the thread to continue execution after it copies the arguments
> +     or exit if the timer can not be created.  */
> +  __pthread_barrier_wait (&args.b);
> +
> +  if (ktimerid < 0)
> +    return -1;
> +
> +  *timerid = pthread_to_timerid (th);
> +
> +  return 0;
> +}
> +
>  int
>  ___timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid)
>  {
> -  {
> -    clockid_t syscall_clockid = (clock_id == CLOCK_PROCESS_CPUTIME_ID
> -                                ? PROCESS_CLOCK
> -                                : clock_id == CLOCK_THREAD_CPUTIME_ID
> -                                ? THREAD_CLOCK
> -                                : clock_id);
> +  clockid_t syscall_clockid = (clock_id == CLOCK_PROCESS_CPUTIME_ID
> +                              ? PROCESS_CLOCK
> +                              : clock_id == CLOCK_THREAD_CPUTIME_ID
> +                              ? THREAD_CLOCK
> +                              : clock_id);
>
> -    /* If the user wants notification via a thread we need to handle
> -       this special.  */
> -    if (evp == NULL
> -       || __builtin_expect (evp->sigev_notify != SIGEV_THREAD, 1))
> +  switch (evp != NULL ? evp->sigev_notify : SIGEV_SIGNAL)
> +    {
> +    case SIGEV_NONE:
> +    case SIGEV_SIGNAL:
> +    case SIGEV_THREAD_ID:
>        {
> -       struct sigevent local_evp;
> -
> +       struct sigevent kevp;
>         if (evp == NULL)
>           {
> -           /* The kernel has to pass up the timer ID which is a
> -              userlevel object.  Therefore we cannot leave it up to
> -              the kernel to determine it.  */
> -           local_evp.sigev_notify = SIGEV_SIGNAL;
> -           local_evp.sigev_signo = SIGALRM;
> -           local_evp.sigev_value.sival_ptr = NULL;
> -
> -           evp = &local_evp;
> +           kevp.sigev_notify = SIGEV_SIGNAL;
> +           kevp.sigev_signo = SIGALRM;
> +           kevp.sigev_value.sival_ptr = NULL;
> +           evp = &kevp;
>           }
>
>         kernel_timer_t ktimerid;
> @@ -64,75 +214,28 @@ ___timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid)
>
>         *timerid = kernel_timer_to_timerid (ktimerid);
>        }
> -    else
> +      break;
> +    case SIGEV_THREAD:
>        {
> -       /* Create the helper thread.  */
> -       __pthread_once (&__timer_helper_once, __timer_start_helper_thread);
> -       if (__timer_helper_tid == 0)
> -         {
> -           /* No resources to start the helper thread.  */
> -           __set_errno (EAGAIN);
> -           return -1;
> -         }
> -
> -       struct timer *newp = malloc (sizeof (struct timer));
> -       if (newp == NULL)
> -         return -1;
> -
> -       /* Copy the thread parameters the user provided.  */
> -       newp->sival = evp->sigev_value;
> -       newp->thrfunc = evp->sigev_notify_function;
> -
> -       /* We cannot simply copy the thread attributes since the
> -          implementation might keep internal information for
> -          each instance.  */
> -       __pthread_attr_init (&newp->attr);
> +       pthread_attr_t attr;
>         if (evp->sigev_notify_attributes != NULL)
> -         {
> -           struct pthread_attr *nattr;
> -           struct pthread_attr *oattr;
> +         __pthread_attr_copy (&attr, evp->sigev_notify_attributes);
> +       else
> +         __pthread_attr_init (&attr);
> +       __pthread_attr_setdetachstate (&attr, PTHREAD_CREATE_DETACHED);
>
> -           nattr = (struct pthread_attr *) &newp->attr;
> -           oattr = (struct pthread_attr *) evp->sigev_notify_attributes;
> +        int r = timer_create_sigev_thread (syscall_clockid, evp, timerid,
> +                                          &attr);
>
> -           nattr->schedparam = oattr->schedparam;
> -           nattr->schedpolicy = oattr->schedpolicy;
> -           nattr->flags = oattr->flags;
> -           nattr->guardsize = oattr->guardsize;
> -           nattr->stackaddr = oattr->stackaddr;
> -           nattr->stacksize = oattr->stacksize;
> -         }
> +       if (&attr != evp->sigev_notify_attributes)
> +         __pthread_attr_destroy (&attr);
>
> -       /* In any case set the detach flag.  */
> -       __pthread_attr_setdetachstate (&newp->attr, PTHREAD_CREATE_DETACHED);
> -
> -       /* Create the event structure for the kernel timer.  */
> -       struct sigevent sev =
> -         { .sigev_value.sival_ptr = newp,
> -           .sigev_signo = SIGTIMER,
> -           .sigev_notify = SIGEV_SIGNAL | SIGEV_THREAD_ID,
> -           ._sigev_un = { ._pad = { [0] = __timer_helper_tid } } };
> -
> -       /* Create the timer.  */
> -       int res;
> -       res = INTERNAL_SYSCALL_CALL (timer_create, syscall_clockid, &sev,
> -                                    &newp->ktimerid);
> -       if (INTERNAL_SYSCALL_ERROR_P (res))
> -         {
> -           free (newp);
> -           __set_errno (INTERNAL_SYSCALL_ERRNO (res));
> -           return -1;
> -         }
> -
> -       /* Add to the queue of active timers with thread delivery.  */
> -       __pthread_mutex_lock (&__timer_active_sigev_thread_lock);
> -       newp->next = __timer_active_sigev_thread;
> -       __timer_active_sigev_thread = newp;
> -       __pthread_mutex_unlock (&__timer_active_sigev_thread_lock);
> -
> -       *timerid = timer_to_timerid (newp);
> +       return r;
>        }
> -  }
> +    default:
> +      __set_errno (EINVAL);
> +      return -1;
> +    }
>
>    return 0;
>  }
> diff --git a/sysdeps/unix/sysv/linux/timer_delete.c b/sysdeps/unix/sysv/linux/timer_delete.c
> index 69f26b266b..0fd3cb15f1 100644
> --- a/sysdeps/unix/sysv/linux/timer_delete.c
> +++ b/sysdeps/unix/sysv/linux/timer_delete.c
> @@ -15,10 +15,8 @@
>     License along with the GNU C Library; see the file COPYING.LIB.  If
>     not, see <https://www.gnu.org/licenses/>.  */
>
> -#include <errno.h>
> -#include <stdlib.h>
> +#include <unistd.h>
>  #include <time.h>
> -#include <sysdep.h>
>  #include "kernel-posix-timers.h"
>  #include <pthreadP.h>
>  #include <shlib-compat.h>
> @@ -26,42 +24,20 @@
>  int
>  ___timer_delete (timer_t timerid)
>  {
> -  kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid);
> -  int res = INLINE_SYSCALL_CALL (timer_delete, ktimerid);
> -
> -  if (res == 0)
> +  if (timer_is_sigev_thread (timerid))
>      {
> -      if (timer_is_sigev_thread (timerid))
> -       {
> -         struct timer *kt = timerid_to_timer (timerid);
> -
> -         /* Remove the timer from the list.  */
> -         __pthread_mutex_lock (&__timer_active_sigev_thread_lock);
> -         if (__timer_active_sigev_thread == kt)
> -           __timer_active_sigev_thread = kt->next;
> -         else
> -           {
> -             struct timer *prevp = __timer_active_sigev_thread;
> -             while (prevp->next != NULL)
> -               if (prevp->next == kt)
> -                 {
> -                   prevp->next = kt->next;
> -                   break;
> -                 }
> -               else
> -                 prevp = prevp->next;
> -           }
> -         __pthread_mutex_unlock (&__timer_active_sigev_thread_lock);
> -
> -         free (kt);
> -       }
> +      struct pthread *th = timerid_to_pthread (timerid);
>
> +      /* The helper thread itself will be responsible to call the
> +        timer_delete syscall.  */
> +      timerid_signal_delete (&th->timerid);
> +      /* We can send the signal directly instead of through
> +        __pthread_kill_internal because the thread is not user-visible
> +        and it blocks SIGTIMER.  */
> +      INTERNAL_SYSCALL_CALL (tgkill, __getpid (), th->tid, SIGTIMER);
>        return 0;
>      }
> -
> -  /* The kernel timer is not known or something else bad happened.
> -     Return the error.  */
> -  return -1;
> +  return INLINE_SYSCALL_CALL (timer_delete, timerid);
>  }
>  versioned_symbol (libc, ___timer_delete, timer_delete, GLIBC_2_34);
>  libc_hidden_ver (___timer_delete, __timer_delete)
> diff --git a/sysdeps/unix/sysv/linux/timer_routines.c b/sysdeps/unix/sysv/linux/timer_routines.c
> deleted file mode 100644
> index b2affaac91..0000000000
> --- a/sysdeps/unix/sysv/linux/timer_routines.c
> +++ /dev/null
> @@ -1,154 +0,0 @@
> -/* Copyright (C) 2003-2025 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public License as
> -   published by the Free Software Foundation; either version 2.1 of the
> -   License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; see the file COPYING.LIB.  If
> -   not, see <https://www.gnu.org/licenses/>.  */
> -
> -#include <errno.h>
> -#include <setjmp.h>
> -#include <signal.h>
> -#include <stdbool.h>
> -#include <sysdep-cancel.h>
> -#include <pthreadP.h>
> -#include "kernel-posix-timers.h"
> -
> -
> -/* List of active SIGEV_THREAD timers.  */
> -struct timer *__timer_active_sigev_thread;
> -
> -/* Lock for _timer_active_sigev_thread.  */
> -pthread_mutex_t __timer_active_sigev_thread_lock = PTHREAD_MUTEX_INITIALIZER;
> -
> -struct thread_start_data
> -{
> -  void (*thrfunc) (sigval_t);
> -  sigval_t sival;
> -};
> -
> -
> -/* Helper thread to call the user-provided function.  */
> -static void *
> -timer_sigev_thread (void *arg)
> -{
> -  signal_unblock_sigtimer ();
> -
> -  struct thread_start_data *td = (struct thread_start_data *) arg;
> -  void (*thrfunc) (sigval_t) = td->thrfunc;
> -  sigval_t sival = td->sival;
> -
> -  /* The TD object was allocated in timer_helper_thread.  */
> -  free (td);
> -
> -  /* Call the user-provided function.  */
> -  thrfunc (sival);
> -
> -  return NULL;
> -}
> -
> -
> -/* Helper function to support starting threads for SIGEV_THREAD.  */
> -static _Noreturn void *
> -timer_helper_thread (void *arg)
> -{
> -  /* Endless loop of waiting for signals.  The loop is only ended when
> -     the thread is canceled.  */
> -  while (1)
> -    {
> -      siginfo_t si;
> -
> -      while (__sigwaitinfo (&sigtimer_set, &si) < 0);
> -      if (si.si_code == SI_TIMER)
> -       {
> -         struct timer *tk = (struct timer *) si.si_ptr;
> -
> -         /* Check the timer is still used and will not go away
> -            while we are reading the values here.  */
> -         __pthread_mutex_lock (&__timer_active_sigev_thread_lock);
> -
> -         struct timer *runp = __timer_active_sigev_thread;
> -         while (runp != NULL)
> -           if (runp == tk)
> -             break;
> -         else
> -           runp = runp->next;
> -
> -         if (runp != NULL)
> -           {
> -             struct thread_start_data *td = malloc (sizeof (*td));
> -
> -             /* There is not much we can do if the allocation fails.  */
> -             if (td != NULL)
> -               {
> -                 /* This is the signal we are waiting for.  */
> -                 td->thrfunc = tk->thrfunc;
> -                 td->sival = tk->sival;
> -
> -                 pthread_t th;
> -                 __pthread_create (&th, &tk->attr, timer_sigev_thread, td);
> -               }
> -           }
> -
> -         __pthread_mutex_unlock (&__timer_active_sigev_thread_lock);
> -       }
> -    }
> -}
> -
> -
> -/* Control variable for helper thread creation.  */
> -pthread_once_t __timer_helper_once = PTHREAD_ONCE_INIT;
> -
> -
> -/* TID of the helper thread.  */
> -pid_t __timer_helper_tid;
> -
> -
> -/* Reset variables so that after a fork a new helper thread gets started.  */
> -void
> -__timer_fork_subprocess (void)
> -{
> -  __timer_helper_once = PTHREAD_ONCE_INIT;
> -  __timer_helper_tid = 0;
> -}
> -
> -
> -void
> -__timer_start_helper_thread (void)
> -{
> -  /* The helper thread needs only very little resources
> -     and should go away automatically when canceled.  */
> -  pthread_attr_t attr;
> -  __pthread_attr_init (&attr);
> -  __pthread_attr_setstacksize (&attr, __pthread_get_minstack (&attr));
> -
> -  /* Block all signals in the helper thread but SIGSETXID.  */
> -  sigset_t ss;
> -  __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;
> -  res = __pthread_create (&th, &attr, timer_helper_thread, NULL);
> -  if (res == 0)
> -    /* We managed to start the helper thread.  */
> -    __timer_helper_tid = ((struct pthread *) th)->tid;
> -
> -  /* No need for the attribute anymore.  */
> -  __pthread_attr_destroy (&attr);
> -}
> --
> 2.43.0
>

This is great to see, I'd love to see it get merged.

If I may respectfully ask two architectural questions:

Why is the timer thread detached rather than joinable? timer_delete() 
depends on the validity of the pthread pointer for memory safety. A 
joinable thread would ensure that the pointer remains valid until 
timer_delete() is called. Perhaps timer_delete() could actually join 
the thread, which would guarantee for the caller that no events will 
be delivered after timer_delete() returns, a desirable simplification 
for applications which may want to free memory associated with the 
timer's sival_ptr.

Secondly, why don't you call the timer_create syscall before creating 
the thread? Then you could pass the timer ID in the args instead of 
adding a member to the pthread structure. The reuse of 
"pthr->exiting", documented as being internal to 
__pthread_kill_internal, would not be necessary.

A couple of inline comments are below.

> @@ -413,6 +413,9 @@ struct pthread
>     /* getrandom vDSO per-thread opaque state.  */
>     void *getrandom_buf;
>   
> +  /* POSIX per-process timer.  */
> +  int timerid;
> +
>     /* Amount of end padding, if any, in this structure.
>        This definition relies on getrandom_buf being last.  */

Per the comment, timerid should be above getrandom_buf.

> +      timer_t timerid;
> +      if (timer_create (CLOCK_REALTIME, ev, &timerid) == -1)
> +	FAIL_EXIT1 ("timer_create: %m");
> +
> +      if (timer_settime (timerid, 0, its, NULL) == -1)
> +	FAIL_EXIT1 ("timer_settime: %m");

Sleeping here for 1us, i.e. its_short.it_value, would probably help to 
reduce the number of iterations required for reproducibility. The key 
thing is that there needs to be an event in flight when timer_delete() 
is called, so it makes sense to wait for the event to be generated.

> +      /* On timer creation failure we need to signal the helper thread to
> +	 exit and we can not use the an negative timerid value after the
> +	 ptherad_barrier_wait because we can not distinguish between

typo pthread_barrier_wait

* Adhemerval Zanella:

> +/* Reset internal thread state if the callback issues pthread_exit.  It avoids
> +   recreating the thread and having possible unreported missed events due
> +   thread creation failure.  */
> +static void
> +timer_helper_thread_cleanup (void *arg)
> +{
> +  struct pthread *self = THREAD_SELF;
> +
> +  /* Call destructors for the thread_local TLS variables.  */
> +  call_function_static_weak (__call_tls_dtors);
> +
> +  /* Run the destructor for the thread-local data.  */
> +  __nptl_deallocate_tsd ();
> +
> +  /* Clean up any state libc stored in thread-local variables.  */
> +  __libc_thread_freeres ();
> +
> +  /* Reset internal TCB state.  */
> +  struct cleanup_args_t *args = arg;
> +  self->cleanup_jmp_buf = args->cleanup_jmp_buf;
> +  self->cleanup_jmp_buf->priv.data.prev = NULL;
> +  self->cleanup_jmp_buf->priv.data.cleanup = NULL;
> +  self->cleanup_jmp_buf->priv.data.canceltype = 0;
> +  self->cleanup = NULL;
> +  self->exc = (struct _Unwind_Exception) { 0 };
> +  self->cancelhandling = 0;
> +  self->nextevent = NULL;
> +
> +  /* Re-initialize the TLS.  */
> +  _dl_allocate_tls_init (TLS_TPADJ (self), true);
> +
> +  /* Reset to the expected initial signal mask.  */
> +  internal_sigset_t ss;
> +  internal_sigfillset (&ss);
> +  internal_sigdelset (&ss, SIGSETXID);
> +  internal_sigprocmask (SIG_SETMASK, &ss, NULL);
> +
> +  /* There is no need to perform any additional cleanup by the frames.  */
> +  struct __jmp_buf_tag *env = args->jb;
> +  __longjmp (env[0].__jmpbuf, 1);
> +}
> +
> +static void *
> +timer_helper_thread (void *arg)
> +{
> +  struct pthread *self = THREAD_SELF;
> +  struct timer_helper_thread_args_t *args = arg;
> +  struct cleanup_args_t clargs = {
> +    .cleanup_jmp_buf = self->cleanup_jmp_buf
> +  };

It might be moore maintainable to integrate this special handling into
start_thread in nptl/pthread_create.c.  Otherwise, this looks like lots
of layering violations to me (access to cancellation internals from the
outside of the cancellation subsystem).

Thanks,
Florian

On 07/04/25 02:29, Tim Starling wrote:
> This is great to see, I'd love to see it get merged.
> 
> If I may respectfully ask two architectural questions:
> 
> Why is the timer thread detached rather than joinable? timer_delete() depends on the validity of the pthread pointer for memory safety. A joinable thread would ensure that the pointer remains valid until timer_delete() is called. Perhaps timer_delete() could actually join the thread, which would guarantee for the caller that no events will be delivered after timer_delete() returns, a desirable simplification for applications which may want to free memory associated with the timer's sival_ptr.

Because this is specified by POSIX in function description [1] and Realtime 
Signal Generation and Delivery [2]:

  If a timer is created which has evp->sigev_sigev_notify set to SIGEV_THREAD 
  and the attribute pointed to by evp->sigev_notify_attributes has a thread 
  stack address specified by a call to pthread_attr_setstack(), the memory 
  dedicated as a thread stack cannot be recovered. The reason for this is that 
  the threads created in response to a timer expiration are created detached, 
  or in an unspecified way if the thread attribute's detachstate is 
  PTHREAD_CREATE_JOINABLE. In neither case is it valid to call pthread_join(), 
  which makes it impossible to determine the lifetime of the created thread 
  which thus means the stack memory cannot be reused.

The thread management requires each timer activation to reset its internal
state, because POSIX also defines it:

  The function shall be executed in a newly created thread as if it were the 
  start_routine for a call to pthread_create() with the thread attributes 
  specified by sigev_notify_attributes. If sigev_notify_attributes is NULL, 
  the behavior shall be as if the thread were created with the detachstate 
  attribute set to PTHREAD_CREATE_DETACHED. Supplying an attributes structure 
  with a detachstate attribute of PTHREAD_CREATE_JOINABLE results in undefined 
  behavior. It is implementation-defined whether the signal mask of this thread 
  has all signals except SIGKILL and SIGSTOP blocked, or is the same as the mask 
  that was in effect for the thread which installed the sigevent notification 
  handler at the time of the call that installed the handler.

That's why we have the loop on timer_helper_thread with the cleanup function
timer_helper_thread_cleanup.  We need to avoid re-creating the thread because
during time reactivation there is no way to advertise possible thread creation
failure (and it does help on latency as well).

And I think making the background thread responsible to call timer_delete here
is the correct approach, otherwise we will need to add another synchronization
way to stop the timer thread, delete the signal, and then signal again so it
can exit.

The SIGEV_THREAD has some special semantics, it is internally created thread
and libc knows that at the moment of timer_delete the thread has its lifetime
defines (it is still undefined behavior or calling thread_delete on a already
delete timer). We support pthread_cancel, but I am not sure if this is really
allowed, and implementation detail, or just undefined behavior by POSIX.

[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/timer_create.html
[2] https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html

> 
> Secondly, why don't you call the timer_create syscall before creating the thread? Then you could pass the timer ID in the args instead of adding a member to the pthread structure. The reuse of "pthr->exiting", documented as being internal to __pthread_kill_internal, would not be necessary.
> 
> A couple of inline comments are below.
> 
>> @@ -413,6 +413,9 @@ struct pthread
>>    /* getrandom vDSO per-thread opaque state.  */
>>    void *getrandom_buf;
>>  
>> +  /* POSIX per-process timer.  */
>> +  int timerid;
>> +
>>    /* Amount of end padding, if any, in this structure.
>>       This definition relies on getrandom_buf being last.  */
> 
> Per the comment, timerid should be above getrandom_buf.

Indeed, I will fix it.

> 
>> +      timer_t timerid;
>> +      if (timer_create (CLOCK_REALTIME, ev, &timerid) == -1)
>> +	FAIL_EXIT1 ("timer_create: %m");
>> +
>> +      if (timer_settime (timerid, 0, its, NULL) == -1)
>> +	FAIL_EXIT1 ("timer_settime: %m");
> 
> Sleeping here for 1us, i.e. its_short.it_value, would probably help to reduce the number of iterations required for reproducibility. The key thing is that there needs to be an event in flight when timer_delete() is called, so it makes sense to wait for the event to be generated.

I think it is doing for its_short, doesn't it? The main problem is
the number of iteration required to trigger this is issue depends of
lot of factor (scheduler pressure, system load, etc.).  I do see 
it reliable triggering with an infinite loop, but I could not get
a iteration number to make it so.

> 
>> +      /* On timer creation failure we need to signal the helper thread to
>> +	 exit and we can not use the an negative timerid value after the
>> +	 ptherad_barrier_wait because we can not distinguish between
> 
> typo pthread_barrier_wait
> 

Ack.

On 07/04/25 04:47, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> +/* Reset internal thread state if the callback issues pthread_exit.  It avoids
>> +   recreating the thread and having possible unreported missed events due
>> +   thread creation failure.  */
>> +static void
>> +timer_helper_thread_cleanup (void *arg)
>> +{
>> +  struct pthread *self = THREAD_SELF;
>> +
>> +  /* Call destructors for the thread_local TLS variables.  */
>> +  call_function_static_weak (__call_tls_dtors);
>> +
>> +  /* Run the destructor for the thread-local data.  */
>> +  __nptl_deallocate_tsd ();
>> +
>> +  /* Clean up any state libc stored in thread-local variables.  */
>> +  __libc_thread_freeres ();
>> +
>> +  /* Reset internal TCB state.  */
>> +  struct cleanup_args_t *args = arg;
>> +  self->cleanup_jmp_buf = args->cleanup_jmp_buf;
>> +  self->cleanup_jmp_buf->priv.data.prev = NULL;
>> +  self->cleanup_jmp_buf->priv.data.cleanup = NULL;
>> +  self->cleanup_jmp_buf->priv.data.canceltype = 0;
>> +  self->cleanup = NULL;
>> +  self->exc = (struct _Unwind_Exception) { 0 };
>> +  self->cancelhandling = 0;
>> +  self->nextevent = NULL;
>> +
>> +  /* Re-initialize the TLS.  */
>> +  _dl_allocate_tls_init (TLS_TPADJ (self), true);
>> +
>> +  /* Reset to the expected initial signal mask.  */
>> +  internal_sigset_t ss;
>> +  internal_sigfillset (&ss);
>> +  internal_sigdelset (&ss, SIGSETXID);
>> +  internal_sigprocmask (SIG_SETMASK, &ss, NULL);
>> +
>> +  /* There is no need to perform any additional cleanup by the frames.  */
>> +  struct __jmp_buf_tag *env = args->jb;
>> +  __longjmp (env[0].__jmpbuf, 1);
>> +}
>> +
>> +static void *
>> +timer_helper_thread (void *arg)
>> +{
>> +  struct pthread *self = THREAD_SELF;
>> +  struct timer_helper_thread_args_t *args = arg;
>> +  struct cleanup_args_t clargs = {
>> +    .cleanup_jmp_buf = self->cleanup_jmp_buf
>> +  };
> 
> It might be moore maintainable to integrate this special handling into
> start_thread in nptl/pthread_create.c.  Otherwise, this looks like lots
> of layering violations to me (access to cancellation internals from the
> outside of the cancellation subsystem).

I will check how hard it would be to move these on pthread_create.c.  I think
it should be feasible, my main concern is also tying the pthread_create with
POSIX timer (although I think the way to proper fix it we will need some
intersection).

On 07/04/25 13:42, Adhemerval Zanella Netto wrote:
> 
> 
> On 07/04/25 04:47, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> +/* Reset internal thread state if the callback issues pthread_exit.  It avoids
>>> +   recreating the thread and having possible unreported missed events due
>>> +   thread creation failure.  */
>>> +static void
>>> +timer_helper_thread_cleanup (void *arg)
>>> +{
>>> +  struct pthread *self = THREAD_SELF;
>>> +
>>> +  /* Call destructors for the thread_local TLS variables.  */
>>> +  call_function_static_weak (__call_tls_dtors);
>>> +
>>> +  /* Run the destructor for the thread-local data.  */
>>> +  __nptl_deallocate_tsd ();
>>> +
>>> +  /* Clean up any state libc stored in thread-local variables.  */
>>> +  __libc_thread_freeres ();
>>> +
>>> +  /* Reset internal TCB state.  */
>>> +  struct cleanup_args_t *args = arg;
>>> +  self->cleanup_jmp_buf = args->cleanup_jmp_buf;
>>> +  self->cleanup_jmp_buf->priv.data.prev = NULL;
>>> +  self->cleanup_jmp_buf->priv.data.cleanup = NULL;
>>> +  self->cleanup_jmp_buf->priv.data.canceltype = 0;
>>> +  self->cleanup = NULL;
>>> +  self->exc = (struct _Unwind_Exception) { 0 };
>>> +  self->cancelhandling = 0;
>>> +  self->nextevent = NULL;
>>> +
>>> +  /* Re-initialize the TLS.  */
>>> +  _dl_allocate_tls_init (TLS_TPADJ (self), true);
>>> +
>>> +  /* Reset to the expected initial signal mask.  */
>>> +  internal_sigset_t ss;
>>> +  internal_sigfillset (&ss);
>>> +  internal_sigdelset (&ss, SIGSETXID);
>>> +  internal_sigprocmask (SIG_SETMASK, &ss, NULL);
>>> +
>>> +  /* There is no need to perform any additional cleanup by the frames.  */
>>> +  struct __jmp_buf_tag *env = args->jb;
>>> +  __longjmp (env[0].__jmpbuf, 1);
>>> +}
>>> +
>>> +static void *
>>> +timer_helper_thread (void *arg)
>>> +{
>>> +  struct pthread *self = THREAD_SELF;
>>> +  struct timer_helper_thread_args_t *args = arg;
>>> +  struct cleanup_args_t clargs = {
>>> +    .cleanup_jmp_buf = self->cleanup_jmp_buf
>>> +  };
>>
>> It might be moore maintainable to integrate this special handling into
>> start_thread in nptl/pthread_create.c.  Otherwise, this looks like lots
>> of layering violations to me (access to cancellation internals from the
>> outside of the cancellation subsystem).
> 
> I will check how hard it would be to move these on pthread_create.c.  I think
> it should be feasible, my main concern is also tying the pthread_create with
> POSIX timer (although I think the way to proper fix it we will need some
> intersection).

So I am not sure this really help here, it would require to add the SIGEV_THREAD
on pthread_create including the sigwaitinfo, the state synchronization (including
the pthread barrier) and the timeid handling.  And I don't think this is the best
place to add this logic.

I think it would be better to move the timer_helper_thread_cleanup to 
pthread_create.c with a different name (__pthread_reset_state or something
like that).

* Adhemerval Zanella Netto:

> So I am not sure this really help here, it would require to add the
> SIGEV_THREAD on pthread_create including the sigwaitinfo, the state
> synchronization (including the pthread barrier) and the timeid
> handling.  And I don't think this is the best place to add this logic.
>
> I think it would be better to move the timer_helper_thread_cleanup to 
> pthread_create.c with a different name (__pthread_reset_state or something
> like that).

What about just adding the loop on the pthread_create side?

Thanks,
Florian

On 07/04/25 15:30, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>> So I am not sure this really help here, it would require to add the
>> SIGEV_THREAD on pthread_create including the sigwaitinfo, the state
>> synchronization (including the pthread barrier) and the timeid
>> handling.  And I don't think this is the best place to add this logic.
>>
>> I think it would be better to move the timer_helper_thread_cleanup to 
>> pthread_create.c with a different name (__pthread_reset_state or something
>> like that).
> 
> What about just adding the loop on the pthread_create side?

The loops only make sense for SIGEV_THREAD, where it calls sigwaitinfo
and calls the provided callback.  Moving to pthread_create will also 
require to add another hook to call INTERNAL_SYSCALL_CALL (timer_delete, ...)
on thread exit (and also expose how POSIX timers mask out timerid). 

I really don't think this simplifies the code here.

* Adhemerval Zanella Netto:

> On 07/04/25 15:30, Florian Weimer wrote:
>> * Adhemerval Zanella Netto:
>> 
>>> So I am not sure this really help here, it would require to add the
>>> SIGEV_THREAD on pthread_create including the sigwaitinfo, the state
>>> synchronization (including the pthread barrier) and the timeid
>>> handling.  And I don't think this is the best place to add this logic.
>>>
>>> I think it would be better to move the timer_helper_thread_cleanup to 
>>> pthread_create.c with a different name (__pthread_reset_state or something
>>> like that).
>> 
>> What about just adding the loop on the pthread_create side?
>
> The loops only make sense for SIGEV_THREAD, where it calls sigwaitinfo
> and calls the provided callback.  Moving to pthread_create will also 
> require to add another hook to call INTERNAL_SYSCALL_CALL (timer_delete, ...)
> on thread exit (and also expose how POSIX timers mask out timerid). 
>
> I really don't think this simplifies the code here.

Okay, please post a version with __pthread_reset_state then.

Thanks,
Florian