Re: [PATCH] coroutine-sigaltstack: Keep SIGUSR2 handler up

[Laszlo Ersek <lersek@redhat.com>]

On 01/22/21 19:29, Laszlo Ersek wrote:

> OK, I'll try my hand at it; I hope I won't be eating my words.

The more I look at it, the less comfortable I am with the existent code.

For example, I don't understand why the original commit -- 3194c8ceeba0,
"coroutine: adding sigaltstack method (.c source)", 2012-03-12 --
wrapped the sigsuspend() in a loop, dependent on the "tr_called" flag.
That looks like a platform bug workaround -- it suggests that
sigsuspend() could wake spuriously, i.e., not in response to the pending
SIGUSR2.

That seems bogus, per POSIX, given that all signals except SIGUSR2 are
included in the mask passed to sigsuspend().

https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigsuspend.html

Also, the comment says, "the signal can be delivered the first time
sigsuspend() is called", which is misleading -- the signal *IS*
delivered the first time sigsuspend() is called, given that we call
pthread_kill(pthread_self()) just before, with SIGUSR2 masked. So by the
time we reach sigsuspend(), the signal is pending.

(The synchronous nature of pthread_kill(pthread_self()) is confirmed by:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/raise.html

which explains (a) the equivalence of raise() with
pthread_kill(pthread_self()), and (b) the fact that raise() does not
return until after the signal handler does, if a signal handler is
called. Given that delivery is dependent on generation, and delivery is
synchronous per description, generation *cannot* be asynchronous.)

All of this makes me super uncomfortable with the code. Either the
platform(s) where it was written & tested did not conform to POSIX, or
the original author missed something, or *I* am missing something. In
each case, I should not be modifying this code; I'm flying blind.

I'm drifting towards an overhaul of coroutine-sigaltstack, based on my
personal understanding of POSIX, but given that I can absolutely not
*test* coroutine-sigaltstack on the platforms where it actually matters,
an "overhaul" by me would be reckless.

I didn't expect these skeletons when I first read Max's "Thread safety
of coroutine-sigaltstack" email :/

Max, after having worked on top of your patch for a few hours, I
officially endorse your mutex approach. I can't encourage you or myself
to touch this code, in good conscience. It's not that it's "bad"; it's
inexplicable and (to me) untestable.

Laszlo