Re: [musl] [PATCH] fix thread leak on timer_create(SIGEV_THREAD) failure

[Rich Felker <dalias@libc.org>]

On Thu, Sep 08, 2022 at 12:18:56PM +0300, Alexey Izbyshev wrote:
> After commit 5b74eed3b301e2227385f3bf26d3bb7c2d822cf8 the timer thread
> doesn't check whether timer_create() actually created the timer,
> proceeding to wait for a signal that might never arrive.  We can't fix
> this by simply checking for a negative timer_id after
> pthread_barrier_wait() because we have no way to distinguish a timer
> creation failure and a request to delete a timer with INT_MAX id if it
> happens to arrive quickly (a variation of this bug existed before
> 5b74eed3b301e2227385f3bf26d3bb7c2d822cf8, where the timer would be
> leaked in this case).  So (ab)use cancel field of pthread_t instead.
> ---
>  src/time/timer_create.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/time/timer_create.c b/src/time/timer_create.c
> index 4bef2390..cd32c945 100644
> --- a/src/time/timer_create.c
> +++ b/src/time/timer_create.c
> @@ -43,6 +43,8 @@ static void *start(void *arg)
>  	union sigval val = args->sev->sigev_value;
>  
>  	pthread_barrier_wait(&args->b);
> +	if (self->cancel)
> +		return 0;
>  	for (;;) {
>  		siginfo_t si;
>  		while (sigwaitinfo(SIGTIMER_SET, &si) < 0);
> @@ -113,8 +115,10 @@ int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_t *restrict
>  		ksev.sigev_signo = SIGTIMER;
>  		ksev.sigev_notify = SIGEV_THREAD_ID;
>  		ksev.sigev_tid = td->tid;
> -		if (syscall(SYS_timer_create, clk, &ksev, &timerid) < 0)
> +		if (syscall(SYS_timer_create, clk, &ksev, &timerid) < 0) {
>  			timerid = -1;
> +			td->cancel = 1;
> +		}
>  		td->timer_id = timerid;
>  		pthread_barrier_wait(&args.b);
>  		if (timerid < 0) return -1;
> -- 
> 2.37.2

I'm not really happy with overloading td->cancel like this, but it's
probably the best fix at present. The long-term direction for this
functionality is hopefully removing the use of kernel timer objects
entirely for SIGEV_THREAD timers and instead implementing them with
clock_nanosleep, which would eliminate the possibility of
SYS_timer_create failure, the possibility of leak, the td->timer_id
member, the SIGTIMER reserved signal, and basically all the hacks
going on here.

Rich