[PATCH v5] linux: Do not spawn a new thread for SIGEV_THREAD (BZ 30558, 27895, 29705, 32833)
Carlos O'Donell
carlos@redhat.com
Wed May 13 18:57:57 GMT 2026
On 5/5/26 11:39 AM, Adhemerval Zanella wrote:
> The current timer_create SIGEV_THREAD implementation has some
> downsides:
Looking forward to v6.
Please look at the 15 "Review:" items.
> 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.
OK. This calls for creating the thread up front for timer_create with SIGEV_THREAD.
>
> 2. The background thread is also kept in the background even when there
> are no more timers, consuming resources and also misleading memory
> profile tools (BZ 29705).
OK. This calls for having the thread exit when the timer is deleted via timer_delete.
>
> 3. There is a lot of metadata that needs to be kept: a control
> variable for helper thread creation, a list of active SIGEV_THREAD
> timers, atfork handlers to cleanup the list.
OK. This calls for minimal overhead overall.
>
> 4. timer_create does not propagate all thread attributes to the new
> thread (BZ 27895).
OK. This calls for creating the thread early with timer_create to propagate the
thread attributs to the new thread at that deterministic point in time.
>
> 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 wrongly triggered (BZ 32833).
OK. We need to handle the in-flight delivery gracefully, likely by not
reusing IDs.
>
> This new implementation moves the thread creation to timer_create, so
> any failure is reported to the caller. Also, the same thread will
> serve multiple timers, thus there are no unreported missed events.
> Avoiding parallel timer activation also avoids possible parallel
> timer invocations seeing the same overrun value.
OK.
> To implement using SIGTIMER internally as SIGCANCEL, it requires to
> mask out SIGCANCEL on thread creation. It essentially disables async
> thread cancellation, but POSIX requires that SIGEV_THREAD is always
> created in detached mode and cancelling a detached thread is UB (glibc
> checks the internal tid, but the memory referenced by pthread_t might
> not always be valid at the moment of a pthread_cancel call).
OK.
> And to avoid the need to recreate the thread for pthread_exit call
> (and having possible unreported missed events due to failed thread
> creation), the SIGEV_THREAD installs a cleanup handler that resets all
> internal thread state.
OK.
> It also prevents the re-use issue when a newly-allocated timer has
> in-flight events being delivered by the kernel (BZ 32833).
OK.
> Performance-wise it uses less CPU time for multiple thread activations,
> although each thread now requires a sigwaitinfo which generates more
> context-switches/page-faults (check comment 7 from BZ 30558). I would
> expect that latency should improve, since it avoids a thread creation
> for each timer expiration.
OK.
> Checked on aarch64-linux-gnu, x86_64-linux-gnu and i686-linux-gnu.
> --
> Changes from v5:
> - Call kernel timer_delete synchronously in ___timer_delete. This
> closes the window where the kernel could generate new timer events
> after timer_delete returned to the caller.
OK.
> - Atomic read of self->timerid (timer_create.c:66), to pair correctly
> with the atomic_fetch_or_relaxed store in timerid_signal_delete
Review:
You can't "pair" two relaxed MO reads becasue they do not synchronize-with.
You could pair a Release MO and Acquire MO and document them as such.
Did you mean something else here?
> - __pthread_attr_copy error propagation.
> - Rewrote rt/tst-timer7.c synchronization to avoid possible deadlock
> if scheduling forces the timer to trigger at same time.
> - Extended some comment and fixed some typos.
>
> Changes from v4:
> - Proper setup the thread TLS after each iteration and add a new
> test (tst-timer7.c) to check it.
> - Fix the build against some abi due missing clockid_t definition.
>
> Changes from v3:
> - Move thread reset state to __pthread_reset_state on pthread_create.c.
> - Fixed pthread_attr_t leak after copy.
> - Fixed struct pthread timerid placement.
>
> 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/allocatestack.c | 23 +-
> nptl/descr.h | 3 +
> nptl/pthread_create.c | 61 +++++
> rt/Makefile | 5 +-
> rt/tst-timer-sigmask.c | 7 +-
> rt/tst-timer6.c | 79 ++++++
> rt/tst-timer7.c | 91 +++++++
> sysdeps/nptl/Makefile | 2 -
> sysdeps/nptl/fork.h | 2 -
> sysdeps/nptl/pthreadP.h | 12 +
> 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 | 243 +++++++++++-------
> sysdeps/unix/sysv/linux/timer_delete.c | 50 ++--
> sysdeps/unix/sysv/linux/timer_routines.c | 154 -----------
> 16 files changed, 450 insertions(+), 367 deletions(-)
> create mode 100644 rt/tst-timer6.c
> create mode 100644 rt/tst-timer7.c
> delete mode 100644 sysdeps/unix/sysv/linux/timer_routines.c
>
> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
> index b2ecb001136..f4eb62369b1 100644
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -124,28 +124,7 @@ get_cached_stack (size_t *sizep, void **memp)
> *sizep = result->stackblock_size;
> *memp = result->stackblock;
>
> - /* Cancellation handling is back to the default. */
> - result->cancelhandling = 0;
> - result->cleanup = NULL;
> - result->setup_failed = 0;
> -
> - /* No pending event. */
> - result->nextevent = NULL;
> -
> - result->exiting = false;
> - __libc_lock_init (result->exit_lock);
> - memset (&result->tls_state, 0, sizeof result->tls_state);
> -
> - result->getrandom_buf = NULL;
> -
> - /* Clear the DTV. */
> - dtv_t *dtv = GET_DTV (TLS_TPADJ (result));
> - for (size_t cnt = 0; cnt < dtv[-1].counter; ++cnt)
> - free (dtv[1 + cnt].pointer.to_free);
> - memset (dtv, '\0', (dtv[-1].counter + 1) * sizeof (dtv_t));
> -
> - /* Re-initialize the TLS. */
> - _dl_allocate_tls_init (TLS_TPADJ (result), false);
> + __pthread_init_stack (result);
OK. Refactor for use by the timer helper thread.
>
> return result;
> }
> diff --git a/nptl/descr.h b/nptl/descr.h
> index 627cc3980f0..520307c9738 100644
> --- a/nptl/descr.h
> +++ b/nptl/descr.h
> @@ -414,6 +414,9 @@ struct pthread
> /* Used on strsignal. */
> struct tls_internal_t tls_state;
>
> + /* POSIX per-process timer. */
> + int timerid;
OK. Expands the pthread_t structure to include the per-process timer ID.
> +
> /* getrandom vDSO per-thread opaque state. */
> void *getrandom_buf;
>
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 9a0cefb0f5a..db1815f1511 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -92,6 +92,33 @@ late_init (void)
> NULL, __NSIG_BYTES);
> }
>
> +static void
> +__pthread_init_stack (struct pthread *result)
> +{
> + /* Cancellation handling is back to the default. */
> + result->cancelhandling = 0;
> + result->cleanup = NULL;
> + result->setup_failed = 0;
> +
> + /* No pending event. */
> + result->nextevent = NULL;
> +
> + result->exiting = false;
OK. Resetting.
> + __libc_lock_init (result->exit_lock);
> + memset (&result->tls_state, 0, sizeof result->tls_state);
> +
> + result->getrandom_buf = NULL;
Review:
The current pthread_push/pop always calls the cleanup function to reset.
Therefore on each timer firing we reset the getrandom buffer?
Is this OK from a semantic perspective? Particularly if the thread function
calls getrandom, this is going to always trigger a syscall usage?
Is there a way to avoid this work? Why do we have to reset at each iteration
of the loop? To avoid getrandom leakage across timer firings?
If this is the case we need a comment in the earlier reset loop in timer_helper_thread
that explains this in more detail.
> +
> + /* Clear the DTV. */
> + dtv_t *dtv = GET_DTV (TLS_TPADJ (result));
> + for (size_t cnt = 0; cnt < dtv[-1].counter; ++cnt)
> + free (dtv[1 + cnt].pointer.to_free);
> + memset (dtv, '\0', (dtv[-1].counter + 1) * sizeof (dtv_t));
> +
> + /* Re-initialize the TLS. */
> + _dl_allocate_tls_init (TLS_TPADJ (result), false);
Review:
Should we reset timerid just avoid it leaking across threads?
> +}
> +
> /* Code to allocate and deallocate a stack. */
> #include "allocatestack.c"
>
> @@ -644,6 +671,40 @@ report_thread_creation (struct pthread *pd)
> return false;
> }
>
> +/* Reset internal thread state as if the start thread routine was initially
> + called from pthread_create. It is used on POSIX timers to reset the
> + SIGEV_THREAD thread after a timer activation (as requires by POSIX on
Review:
Typo s/requires by POSIX on/required by POSIX in/g
> + Realtime Signal Generation and Delivery). */
> +void
> +__pthread_reset_state (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 pthread_reset_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;
> +
> + __pthread_init_stack (self);
> +
> + /* Reset to the expected initial signal mask. */
> + internal_signal_restore_set (&self->sigmask);
> +}
>
> int
> __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
> diff --git a/rt/Makefile b/rt/Makefile
> index 39a3e5620b3..2fe4d7ba6b1 100644
> --- a/rt/Makefile
> +++ b/rt/Makefile
> @@ -76,7 +76,9 @@ 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 \
> + tst-timer7
> tests-internal := tst-timer-sigmask
>
> tests-time64 := \
> @@ -98,6 +100,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 869194bde68..52f44bd5666 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 00000000000..4eee7421246
> --- /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");
Review:
Typo s/time_delete/timer_delete/g
> + }
> +
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/rt/tst-timer7.c b/rt/tst-timer7.c
> new file mode 100644
> index 00000000000..edfebfec1f9
> --- /dev/null
> +++ b/rt/tst-timer7.c
> @@ -0,0 +1,91 @@
> +/* Check if thread local storage is reset on each SIGEV_THREAD trigger.
> + 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 <array_length.h>
> +#include <semaphore.h>
> +#include <signal.h>
> +#include <string.h>
> +#include <time.h>
> +
> +#include <support/check.h>
> +
> +static sem_t sem;
> +
> +static __thread int var1;
> +#define VAR2_LEN 32
> +static __thread char var2[] = { [0 ... VAR2_LEN] = 0xcc };
> +
> +static const char var2_expected[] = { [0 ... VAR2_LEN] = 0xcc };
> +
> +static void
> +on_timer (union sigval sv)
> +{
> + TEST_COMPARE (var1, 0);
> + TEST_COMPARE_BLOB (var2, array_length (var2),
> + var2_expected, array_length (var2_expected));
> +
> + var1 = 1;
> + memset (var2, 0x00, array_length (var2));
> +
> + sem_post (&sem);
> +}
> +
> +#define NITERS 10
> +
> +static int
> +do_test (void)
> +{
> + const struct itimerspec its =
> + { .it_value = { .tv_nsec = 10000000 /* 0.01s */ },
> + .it_interval = { .tv_nsec = 10000000 /* 0.01s */ } };
> + const struct itimerspec its_stop = { 0 };
> +
> + sem_init (&sem, 0, 0);
> +
> + timer_t timerid;
> + struct sigevent ev =
> + {
> + .sigev_notify = SIGEV_THREAD,
> + .sigev_notify_function = on_timer,
> + };
> + 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");
> +
> + for (int i = 0; i < NITERS; i++)
> + {
> + if (sem_wait (&sem) != 0)
> + FAIL_EXIT1 ("sem_wait: %m");
> + }
> +
> + /* Disarm before deleting to minimise the chance of an in-flight
> + invocation racing with timer_delete. */
> + if (timer_settime (timerid, 0, &its_stop, NULL) == -1)
> + FAIL_EXIT1 ("timer_settime: %m");
> +
> + if (timer_delete (timerid) == -1)
> + FAIL_EXIT1 ("timer_delete: %m");
> +
> + sem_destroy (&sem);
> +
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/nptl/Makefile b/sysdeps/nptl/Makefile
> index 03c9c056401..47f5ba5824f 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 c09e57c5abd..31ac12812f3 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/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h
> index de432d40324..f1a18cdaf89 100644
> --- a/sysdeps/nptl/pthreadP.h
> +++ b/sysdeps/nptl/pthreadP.h
> @@ -667,6 +667,18 @@ int __pthread_attr_extension (struct pthread_attr *attr) attribute_hidden
> # define PTHREAD_STATIC_FN_REQUIRE(name) __asm (".globl " #name);
> #endif
>
> +struct pthread_reset_cleanup_args_t
> +{
> + struct pthread_unwind_buf *cleanup_jmp_buf;
> + jmp_buf jb;
> +};
> +
> +/* Reset internal thread state is if the start thread routine was initially
> + called from pthread_create. It should be used along pthread_cleanup_push
> + and pthread_cleanup_pop pthread_reset_cleanup_args_t. */
Review:
/* Reset the thread's internal state to a point as close to the initial call to
pthread_create. It is designed to be used as the argument to
pthread_cleanup_push along with a struct pthread_reset_cleanup_args_t pointer
in args with a valid cleanup_jmp_buf used to reset the threads own copy. */
> +void __pthread_reset_state (void *arg) attribute_hidden;
> +
> +
> /* Make a deep copy of the attribute *SOURCE in *TARGET. *TARGET is
> not assumed to have been initialized. Returns 0 on success, or a
> positive error code otherwise. */
> diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h
> index a8f1c87b463..02a086f7d5d 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 bea1e0e62da..c95b504d5f6 100644
> --- a/sysdeps/unix/sysv/linux/kernel-posix-cpu-timers.h
> +++ b/sysdeps/unix/sysv/linux/kernel-posix-cpu-timers.h
> @@ -20,6 +20,8 @@
> #define CPUCLOCK_SCHED 2
> #define CPUCLOCK_MAX 3
>
> +#include <sys/types.h>
> +
> static inline clockid_t
> make_process_cpuclock (unsigned int pid, clockid_t clock)
> {
> diff --git a/sysdeps/unix/sysv/linux/kernel-posix-timers.h b/sysdeps/unix/sysv/linux/kernel-posix-timers.h
> index 9b7859b1c65..a1abef7ae69 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 sign bit (INT_MIN) is set on timer_delete to
> + signal the helper thread to exit its sigwaitinfo loop. */
>
> 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;
Review:
Should this be an atomic Relaxed MO read?
We are in timer_delete() and reading, while timer_helper_thread is also reading.
While timer_helper_thread shouldn't write to timerid, it is safest if this is atomic.
> + }
> + return (uintptr_t) timerid;
> +}
> +
> +static inline void
> +timerid_signal_delete (kernel_timer_t *timerid)
> +{
Review:
Add "/* Relaxed MO is sufficient because this is followed by a signal which is full memory barrier. */"
Or this should be a Release MO.
> + 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 0889ce66f5e..3d80d63f269 100644
> --- a/sysdeps/unix/sysv/linux/timer_create.c
> +++ b/sysdeps/unix/sysv/linux/timer_create.c
> @@ -15,46 +15,148 @@
> 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 barrier;
> + struct sigevent *evp;
> +};
> +
> +static void *
> +timer_helper_thread (void *arg)
> +{
> + struct pthread *self = THREAD_SELF;
> + struct timer_helper_thread_args_t *args = arg;
> + struct pthread_reset_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;
OK. Copy the function, and sival value.
> +
> + __pthread_barrier_wait (&args->barrier);
OK. Required wait on timer_create.
> + /* timer_create syscall failed. */
> + if (self->exiting)
OK. Read of self->exiting without atomic.
OK. No UB because of barrier wait.
OK. Usually accessing exiting is done with exit_lock held.
OK. self->exiting is safe to access after timer_delete() (valid self-synchronizing).
> + 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 (__pthread_reset_state, &clargs);
> + thrfunc (sival);
> + pthread_cleanup_pop (1);
Review:
We always call the thread state reset?
Why is that?
> + }
> +
> + /* timer_delete sets the MSB and sends SIGTIMER to wake this thread. */
Review:
Add "Relaxed MO is sufficient because signaling this thread is a memory barrier."
or use Acquire MO to sequence-after the Relase MO store in timer_delete.
> + if (atomic_load_relaxed (&self->timerid) < 0)
OK. Relaxed load of timerid from the helper thread. Ordering has to be enforced
by another operation (signal syscall), and this is expected.
> + break;
> + }
> +
> + return NULL;
> +}
> +
Review:
Should describe that errno is set on error for timer_create_sigev_thread.
> +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 = { .evp = evp };
> + __pthread_barrier_init (&args.barrier, NULL, 2);
> +
> + pthread_t th;
> + int r = __pthread_create (&th, attr, timer_helper_thread, &args);
OK. Thread is created via timer_helper_thread.
> + if (r != 0)
> + {
> + __set_errno (r);
> + return -1;
> + }
> +
> + struct pthread *pthr = (struct pthread *)th;
> + /* SIGEV_THREAD_ID delivers the signal to a specific thread by TID.
> + SIGEV_SIGNAL is not combined here because SIGEV_THREAD_ID already
> + implies signal delivery; the kernel treats them as orthogonal bits
> + and the TID field alone is sufficient to route SIGTIMER correctly. */
> + 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 cannot use a negative timerid value after the
> + pthread_barrier_wait because we cannot distinguish between
> + a timer creation failure and a request to delete a timer if it happens
> + to arrive quickly (e.g. two timers are created in sequence,
> + where the 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.
Review:
Suggest s/to avoid pthread_kill to sned/to prevent pthread_kill from sending/g
> + Since the thread should not be user-visible, signal are only sent
Review:
Typo s/signal/signals/g
> + during timer_delete. */
> + pthr->exiting = true;
OK. Write happens and protected by barrier so no UB.
> + }
> + 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.barrier);
Review:
Earlier INLINE_SYSCALL_CALLL may set errno.
If INLINE_SYSCALL_CALL fails it sets errno, and we proceed to barrier wait.
The call to __pthread_barrier_wait *might* clobber errno, and so the returned
errno might be wrong, and then timer_create returned errno might be wrong.
Today in glibc I think it never clobbers, but a correct solution would be to
save and restore errno.
This requires timer_create syscall to fail, and pthread_barrier_wait to fail,
and us to have an implementation that sets errno.
Unlikely but possible?
> +
> + 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)
OK. timer_create
> {
> - {
> - 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 +166,34 @@ ___timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid)
>
> *timerid = kernel_timer_to_timerid (ktimerid);
> }
> - else
> + break;
> + case SIGEV_THREAD:
OK. This is the SIGEV_THREAD case.
> {
> - /* 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;
> -
> - nattr = (struct pthread_attr *) &newp->attr;
> - oattr = (struct pthread_attr *) evp->sigev_notify_attributes;
> -
> - nattr->schedparam = oattr->schedparam;
> - nattr->schedpolicy = oattr->schedpolicy;
> - nattr->flags = oattr->flags;
> - nattr->guardsize = oattr->guardsize;
> - nattr->stackaddr = oattr->stackaddr;
> - nattr->stacksize = oattr->stacksize;
> + int r = __pthread_attr_copy (&attr, evp->sigev_notify_attributes);
OK. Copy attributes.
> + if (r != 0)
> + {
> + __set_errno (r);
> + return -1;
> + }
> }
> + else
> + __pthread_attr_init (&attr);
> + __pthread_attr_setdetachstate (&attr, PTHREAD_CREATE_DETACHED);
OK. Detached.
>
> - /* In any case set the detach flag. */
> - __pthread_attr_setdetachstate (&newp->attr, PTHREAD_CREATE_DETACHED);
> + int r = timer_create_sigev_thread (syscall_clockid, evp, timerid,
> + &attr);
>
> - /* 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 } } };
> + __pthread_attr_destroy (&attr);
>
> - /* 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 9a6e74328e7..3422342b5ca 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,26 @@
> 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);
> + struct pthread *th = timerid_to_pthread (timerid);
> + kernel_timer_t ktimerid = timerid_to_kernel_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);
> - }
> + /* Delete the kernel timer first so no new events are generated
> + after this function returns. */
> + int ret = INLINE_SYSCALL_CALL (timer_delete, ktimerid);
> + if (ret != 0)
> + return ret;
>
> + /* Signal the helper thread to exit its sigwaitinfo loop. */
> + timerid_signal_delete (&th->timerid);
OK. Uses atomics.
> + /* 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);
OK. This is a memory barrier that the target thread will see the timerid update.
> 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 7ba6dd78ba7..00000000000
> --- a/sysdeps/unix/sysv/linux/timer_routines.c
> +++ /dev/null
> @@ -1,154 +0,0 @@
> -/* Copyright (C) 2003-2026 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);
> -}
--
Cheers,
Carlos.
More information about the Libc-alpha
mailing list