Re: [musl] Re: [RFC PATCH] x86/vdso/32: Add AT_SYSINFO cancellation helpers

[Ingo Molnar]

* Rich Felker <dalias@xxxxxxxx> wrote:

> > So instead of a sticky cancellation flag, we could introduce a sticky 
> > cancellation signal.
> > 
> > A 'sticky signal' is not cleared from signal_pending() when the signal handler 
> > executes, but it's automatically blocked so no signal handler recursion 
> > occurs. (A sticky signal could still be cleared via a separate mechanism, by 
> > the cancellation cleanup code.)
> > 
> > Such a 'sticky cancellation signal' would, in the racy situation, cause new 
> > blocking system calls to immediately return with -EINTR. Non-blocking syscalls 
> > could still be used. (So the cancellation signal handler itself would still 
> > have access to various fundamental system calls.)
> > 
> > I think this would avoid messy coupling between the kernel's increasingly more 
> > varied system call entry code and C libraries.
> > 
> > Sticky signals could be requested via a new SA_ flag.
> > 
> > What do you think?
> 
> This still doesn't address the issue that the code making the syscall needs to 
> be able to control whether it's cancellable or not. Not only do some syscalls 
> whose public functions are cancellation points need to be used internally in 
> non-cancellable ways; there's also the pthread_setcancelstate interface that 
> allows deferring cancellation so that it's possible to call functions which are 
> cancellation points without invoking cancellation.

I don't think there's a problem - but I might be wrong:

One way I think it would work is the following: a sticky signal is not the 
cancellation flag - it's a helper construct to implement the flag in user-space in 
a race-free way.

Say you have RT signal-32 as the cancellation signal, and it's a sticky signal.

When pthread_cancel() wants to cancel another thread, it first (atomically) sets 
the desired cancel state of the target thread. If that state signals that the 
thread is cancellable right now, and that we initiated its cancellation, then we 
send signal-32. I.e. the signal only ever gets sent if the thread is in a 
cancellable state.

libc internal functions and the pthread_setcancelstate() API can temporarily 
change the cancel state of a thread to non-cancellable - but pthread_cancel() 
observes those state transitions.

The 'sticky' nature of signal-32 will make a difference in the following race 
condition, if the cancellation flag is checked before a system call by the C 
library, and signal-32 arrives before the system call is executed. In that case 
the 'sticky' nature of the signal makes sure that all subsequent system calls 
return immediately.

The sticky signal is only ever sent when the thread is in cancellable state - and 
if the target thread notices the cancellation request before the signal arrives, 
it first waits for its arrival before executing any new system calls (as part of 
the teardown, etc.).

So the C library never has to do complex work with a sticky signal pending.

Does that make more sense to you?

> From my standpoint the simplest and cleanest solution is for vdso to provide a 
> predicate function that takes a ucontext_t and returns true/false for whether it 
> represents a state prior to entering (or reentering, for restart state) the vdso 
> syscall. If vdso exports this symbol libc can use vdso syscall with 
> cancellation. If not, it can just fallback to straight inline syscall like now.

Offering such a flag pushes unreasonable conceptual overhead into the vDSO proper 
in the long run: right now it might be easy to implement because the code paths 
are relatively simple and we can generate the flag passively via RIP checking - 
but if the vDSO grows more complex interfaces in the future, we'd essentially have 
to track our entry/exit state dynamically which sucks ...

I think the real solution is to push all such overhead to the cancellation API 
side: it can track its state, and it can use sticky signals to make sure blocking 
system calls return immediately once a cancellation is in progress.

Thanks,

	Ingo