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

On 06/07/26 19:09, Carlos O'Donell wrote:
> On 5/14/26 7:14 AM, Adhemerval Zanella wrote:
>> The current timer_create SIGEV_THREAD implementation has some
>> downsides:
> 
> Three issues:
> 
> 1. timerid_clear is unused but still defined. Please delete.
> 2. Incomplete sentence "(or force a fresh allocation if)"?

Ack.

> 3. Conformance of pthread_cancel vs notification thread. See POC.
> 
>>    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 is indeed the most serious issue. It prevents the use of SIGEV_THREAD
> in functional safety contexts.
> 
>>    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. When we delete the timer the helper thread is exited.
> 
>>    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. We drop all of that.
> 
>>    4. timer_create does not propagate all thread attributes to the new
>>       thread (BZ 27895).
> 
> OK. We fix that.
> 
>>    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.
> 
>> 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).
> 
> Unfortunately it is not UB if another thread can prove the thread is
> alive, and you can, which creates a conformance issue (POC included).
> 
> For example:
> 
> thread A
> -> timer_create, SIGEV_THREAD
> -> timer fires
> 
>     thread S
>     -> sigwaitinfo returns.
>     -> run handler
>         -> Call pthread_self
>         -> Pass result to thread B    thread B
>         -> enable async cancel        -> Read pthread_t
>         -> while (1);            -> pthread_cancel on S
> 
> It's possible the handler is passing data to other worker threads
> and is sharing enough information to guarnatee that it is alive (and
> that pthread_t is valid despite detached state), and needs asynchronous
> cancellation, at which point it is valid to act on pthread_cancel().
> 
> The implementation prevents asynchronous cancellation from being used
> because of the re-used same-signal, with the pthread_cancel() call not
> acting.
> 
> Do we need another internal signal for this?
> 
> Or can we just reject pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, ...)
> for the helper thread and document this issue?
> 
> A signal needs to be delivered to interrupt the process, but to do that
> requires unblocking a signal. We have SIGSETXID unblocked. Could
> cancellation requests also be sent via the multiplexed mechansim?

Indeed this is not strickly UB, so the commit message is misleading. And I agree 
that this is a regression that prevents the asynchronous cancellation.

I think there is no need to reserver *another* internal signal, but at same time
it does result in a clear implementation.  This new scheme where the helper thread
does both the sigwaitinfo *and* runs thrfunc directly would require to eager-install
the sigcancel handler at the timer creation.  It couples the timer_create with some
pthread internal code, but it is doable.

I will send a new version.

> 
> Assisted-by: LLM
> ~~~
> /* POSIX requires SIGEV_THREAD notifications to behave as if a new thread
>    was created for each delivery.  Such a thread must honor asynchronous
>    cancellation once the notify function enables it.
> 
>    glibc's SIGEV_THREAD helper blocks all signals except SIGSETXID at thread
>    creation (see timer_create_sigev_thread), which prevents SIGCANCEL from
>    being delivered.  This test demonstrates the resulting non-conformance:
>    pthread_cancel on the pthread_t obtained from pthread_self inside the
>    notify function has no effect and the test times out.  */
> 
> #include <errno.h>
> #include <pthread.h>
> #include <semaphore.h>
> #include <signal.h>
> #include <time.h>
> #include <unistd.h>
> 
> #include <support/check.h>
> 
> static pthread_t handler_thread;
> static sem_t handler_started;
> 
> static void
> on_timer (union sigval sv)
> {
>   handler_thread = pthread_self ();
>   if (sem_post (&handler_started) != 0)
>     FAIL_EXIT1 ("sem_post from handler: %m");
> 
>   TEST_COMPARE (pthread_setcancelstate (PTHREAD_CANCEL_ENABLE, NULL), 0);
>   TEST_COMPARE (pthread_setcanceltype (PTHREAD_CANCEL_ASYNCHRONOUS, NULL), 0);
> 
>   while (1)
>     ;
> }
> 
> static int
> do_test (void)
> {
>   if (sem_init (&handler_started, 0, 0) != 0)
>     FAIL_EXIT1 ("sem_init: %m");
> 
>   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");
> 
>   struct itimerspec its =
>     { .it_value = { .tv_nsec = 10000000 /* 0.01s */ } };
>   if (timer_settime (timerid, 0, &its, NULL) == -1)
>     FAIL_EXIT1 ("timer_settime: %m");
> 
>   if (sem_wait (&handler_started) != 0)
>     FAIL_EXIT1 ("sem_wait: %m");
> 
>   /* Give the handler time to enter the busy loop.  */
>   usleep (10000);
> 
>   if (pthread_cancel (handler_thread) != 0)
>     FAIL_EXIT1 ("pthread_cancel: %m");
> 
>   /* A conforming implementation cancels the notify thread; detect that by
>      waiting until pthread_kill no longer finds it.  With the current glibc
>      SIGEV_THREAD helper this never happens and the test times out.  */
>   for (;;)
>     {
>       int err = pthread_kill (handler_thread, 0);
>       if (err == ESRCH)
>     break;
>       if (err != 0)
>     FAIL_EXIT1 ("pthread_kill: %m");
>       usleep (10000);
>     }
> 
>   struct itimerspec its_stop = { 0 };
>   if (timer_settime (timerid, 0, &its_stop, NULL) == -1)
>     FAIL_EXIT1 ("timer_settime (disarm): %m");
>   if (timer_delete (timerid) == -1)
>     FAIL_EXIT1 ("timer_delete: %m");
> 
>   if (sem_destroy (&handler_started) != 0)
>     FAIL_EXIT1 ("sem_destroy: %m");
> 
>   return 0;
> }
> 
> #define TIMEOUT 3
> #include <support/test-driver.c>
> ~~~

Thanks, I will use this a base for a regression testcase. >