[musl] [timer] timer_delete function async problem

[musl] [timer] timer_delete function async problem

[zuotina]

[-- Attachment #1: Type: text/plain, Size: 675 bytes --]

Hi everrone


Problem:
When I created SIGEV_THREAD timer, then start it by timer_settime. like this
the notify callback in the helper thread 'start' will be run when timer expiration.
But when I delete the timer, the notify callback will be run all the same.
This is not what i want. In actual use,  I encountered a problem.


I found that the 'timer_delete' function returns immediately after called.
The timer may not perform the delete action.


In addition, the SIGEV_SIGNAL timer can be deleted after called the function. 
So i think the function has different semantics for different types.


Is there a way to implement synchronously ?


Looking forward to your reply.

[-- Attachment #2: Type: text/html, Size: 1025 bytes --]

Re: [musl] [timer] timer_delete function async problem

[Szabolcs Nagy]

* zuotina <zuotingyang@126.com> [2020-02-15 18:54:23 +0800]:
> Problem:
> When I created SIGEV_THREAD timer, then start it by timer_settime. like this
> the notify callback in the helper thread 'start' will be run when timer expiration.
> But when I delete the timer, the notify callback will be run all the same.
> This is not what i want. In actual use,  I encountered a problem.

write an example program that shows the problem

> 
> 
> I found that the 'timer_delete' function returns immediately after called.
> The timer may not perform the delete action.
> 
> 
> In addition, the SIGEV_SIGNAL timer can be deleted after called the function. 
> So i think the function has different semantics for different types.
> 
> 
> Is there a way to implement synchronously ?
> 
> 
> Looking forward to your reply.

Re: [musl] [timer] timer_delete function async problem

[Rich Felker]

On Sat, Feb 15, 2020 at 06:54:23PM +0800, zuotina wrote:
> Hi everrone
> 
> 
> Problem:
> When I created SIGEV_THREAD timer, then start it by timer_settime. like this
> the notify callback in the helper thread 'start' will be run when timer expiration.
> But when I delete the timer, the notify callback will be run all the same.
> This is not what i want. In actual use,  I encountered a problem.
> 
> 
> I found that the 'timer_delete' function returns immediately after called.
> The timer may not perform the delete action.

Logically the timer is deleted before the timer_delete function
returns. If the handler thread is still running a handler at the time,
that will necessarily continue intil it exits; the specification makes
no provision for timer_delete doing anything to already-running
handler threads. Since the operations are inherently unordered, it's
possible that a new handler physically starts running after the call
to timer_delete is made but before the signal is sent; as far as I can
tell this is not observable by the application (since there is no
ordering).

> In addition, the SIGEV_SIGNAL timer can be deleted after called the function. 
> So i think the function has different semantics for different types.

But doing so does not stop the signal handler from running (and
fundamentally couldn't). So I don't understand what your concern is.
Is it just that the kernel timer resource still exists until the
handler thread finishes? I don't think that's visible to the
application except possibly in limiting the number of timers that can
be created.

> Is there a way to implement synchronously ?

At some point I plan to drop use of kernel timer resources entirely
for SIGEV_THREAD timers, since they can be implemented *more easily*,
with fewer hacks (no SIGTIMER at all! we can get rid of this reserved
RT signal) with just a loop performing clock_nanosleep. If/when this
change is made, there will be no kernel timer resource involed at all,
so if I understand what you're asking, I guess that would give the
behavior you want. (?)

If I'm not understanding what you're asking for, could you send a
minimal testcase program demonstrating how you observe a behavior you
consider wrong without the test invoking any UB?

Rich

[musl] Re:Re: [musl] [timer] timer_delete function async problem

[zuotina]

[-- Attachment #1: Type: text/plain, Size: 3444 bytes --]

The pseudo-code example program as follows. There are two processes A and B.
process A:
void timer_handler(union sigval arg)
{
pctx = (struct ctx *)arg.sival_ptr;
if (pctx == NULL)
return;
// Here may be scheduled to timer_release of thread B.
// When scheduled again, the pctx was illeagal， so panic.
lock(pctx->lock);
/* do something with pctx */
unlock(pctx->lock);
}
void create_timer_function()
{
struct sigevent sig;
sig.sigev_notify = SIGEV_THREAD;
sig.sigev_value.sival_ptr = pctx; 
sig.sigev_notify_function = timer_handler;
timer_create(CLOCK_REALTIME, &sig, &pctx->timerid);
}


process B:
void timer_release()
{
lock(pctx->lock);
timer_delete(pctx->timerid);
unlock(pctx->lock);
// here will do something...
free(pctx);
}


After the call to timer_delete is made but before the signal is sent, 
a new handler which is not already-running will be coming at uncertain time.
I've tried synchronization and mutual between 'timer_release' and 'timer_handler', 
neither can be resolved. 
How to solve the panic in the example, hope to give some suggestions.









At 2020-02-16 01:27:26, "Rich Felker" <dalias@libc.org> wrote:
>On Sat, Feb 15, 2020 at 06:54:23PM +0800, zuotina wrote:
>> Hi everrone
>> 
>> 
>> Problem:
>> When I created SIGEV_THREAD timer, then start it by timer_settime. like this
>> the notify callback in the helper thread 'start' will be run when timer expiration.
>> But when I delete the timer, the notify callback will be run all the same.
>> This is not what i want. In actual use,  I encountered a problem.
>> 
>> 
>> I found that the 'timer_delete' function returns immediately after called.
>> The timer may not perform the delete action.
>
>Logically the timer is deleted before the timer_delete function
>returns. If the handler thread is still running a handler at the time,
>that will necessarily continue intil it exits; the specification makes
>no provision for timer_delete doing anything to already-running
>handler threads. Since the operations are inherently unordered, it's
>possible that a new handler physically starts running after the call
>to timer_delete is made but before the signal is sent; as far as I can
>tell this is not observable by the application (since there is no
>ordering).
>
>> In addition, the SIGEV_SIGNAL timer can be deleted after called the function. 
>> So i think the function has different semantics for different types.
>
>But doing so does not stop the signal handler from running (and
>fundamentally couldn't). So I don't understand what your concern is.
>Is it just that the kernel timer resource still exists until the
>handler thread finishes? I don't think that's visible to the
>application except possibly in limiting the number of timers that can
>be created.
>
>> Is there a way to implement synchronously ?
>
>At some point I plan to drop use of kernel timer resources entirely
>for SIGEV_THREAD timers, since they can be implemented *more easily*,
>with fewer hacks (no SIGTIMER at all! we can get rid of this reserved
>RT signal) with just a loop performing clock_nanosleep. If/when this
>change is made, there will be no kernel timer resource involed at all,
>so if I understand what you're asking, I guess that would give the
>behavior you want. (?)
>
>If I'm not understanding what you're asking for, could you send a
>minimal testcase program demonstrating how you observe a behavior you
>consider wrong without the test invoking any UB?
>
>Rich

[-- Attachment #2: Type: text/html, Size: 5043 bytes --]

Re: [musl] Re:Re: [musl] [timer] timer_delete function async problem

[Rich Felker]

On Mon, Feb 17, 2020 at 03:12:03PM +0800, zuotina wrote:
> The pseudo-code example program as follows. There are two processes A and B.
> process A:
> void timer_handler(union sigval arg)
> {
> pctx = (struct ctx *)arg.sival_ptr;
> if (pctx == NULL)
> return;
> // Here may be scheduled to timer_release of thread B.
> // When scheduled again, the pctx was illeagal， so panic.
> lock(pctx->lock);
> /* do something with pctx */
> unlock(pctx->lock);
> }
> void create_timer_function()
> {
> struct sigevent sig;
> sig.sigev_notify = SIGEV_THREAD;
> sig.sigev_value.sival_ptr = pctx; 
> sig.sigev_notify_function = timer_handler;
> timer_create(CLOCK_REALTIME, &sig, &pctx->timerid);
> }
> 
> 
> process B:
> void timer_release()
> {
> lock(pctx->lock);
> timer_delete(pctx->timerid);
> unlock(pctx->lock);
> // here will do something...
> free(pctx);
> }
> 
> 
> After the call to timer_delete is made but before the signal is sent, 
> a new handler which is not already-running will be coming at uncertain time.
> I've tried synchronization and mutual between 'timer_release' and 'timer_handler', 
> neither can be resolved. 
> How to solve the panic in the example, hope to give some suggestions.

As written, the code has use-after-free, independent of what
timer_delete does. It's always possible that the timer thread is at
the entry point of timer_handler when thread B calls timer_delete.
Then thread B frees an object which the timer thread is accessing.
Nothing timer_delete could do could prevent this situation from
arising.

At first glance, it seems like this is a fundamental design flaw in
the SIGEV_THREAD timer API -- there's no way to determine whether
there's still (at least) one thread that will run, since the state of
having started but not executed any application code in the handler
yet is not observable/distinguishable from not having run at all. If
this analysis is correct, then it seems like the timer handler thread
must use at least one object of permanent lifetime (i.e. that's never
freed) to synchronize and determine if it can access other objects.

If you can assume (as is true in the musl implementation, but doesn't
seem to be guaranteed) that at most one timer handler thread for a
given timer is runnable at once, then you can simply have the timer
handler thread be responsible for deleting the timer and freeing the
associated data object based on a flag that another thread set inside
the object (under appropriate locking).

If you can't assume this, you can make it true by not using
auto-rearming timers (i.e. no it_interval) and having the timer thread
re-arm itself (it_value) every time it runs.

None of this is nice, and suggests that SIGEV_THREAD timers (and the
POSIX timers API in general) is poorly designed and difficult to use
safely. Really, the right solution here is just making your own thread
that calls clock_nanosleep (probably with TIMER_ABSTIME) in a loop and
runs the code you want each time the sleep finishes. This is far
easier to get right and doesn't have complex lifetime issues to deal
with.

Rich