On 2022-09-12 17:22, Rich Felker wrote:
> 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.

I'm not terribly happy too, but short of pulling the real 
pthread_cancel() here, the only other reasonable fix that I could come 
up with was to make the timer thread allocate the space for the deleted 
flag on its stack, like in the attached patch, but it felt a bit heavy.

Alexey