[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ea3783d881ea63fb6207362783e12ddf@ispras.ru>
Date: Mon, 12 Sep 2022 17:57:35 +0300
From: Alexey Izbyshev <izbyshev@...ras.ru>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fix thread leak on timer_create(SIGEV_THREAD)
failure
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
View attachment "timer-create-thread-leak.patch" of type "text/x-diff" (2064 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
