Re: Bug: Incorrect signal behavior in multi-threaded processes

[Dan Bonachea <dobonachea@lbl.gov>]

> A minimal test program is copied below and also available here:
> https://upc-bugs.lbl.gov/bugzilla/attachment.cgi?id=589

> It's worth noting POSIX 1003.1-2016 sec XRAT.B.2.4.1 (p.3577)
> specifically requires that any given signal should be delivered to
> exactly one thread. Also the spec for abort (p.565) requires the
> signal to be delivered as if by `raise(SIGABRT)` (p.1765) aka.
> `pthread_kill(pthread_self(),SIGABRT)` (p.1657), which implies
> any registered SIGABRT handler should run only on the thread
> which called abort().

Poking around further, I find that replacing the signal generation
code in the test program for all cases with :

  pthread_kill(pthread_self(),sigid)

generates compliant signal delivery behavior!

This reveals that Cygwin is theoretically capable of correctly
delivering signals to a selected "non-primordial" thread; but the
various forms of signal generation exercised in the original test are
apparently not leading to correct use of that internal mechanism.

To review, the POSIX 1003.1-2017 specification for abort() says:

   The SIGABRT signal shall be sent to the calling process as if by means
   of raise() with the argument SIGABRT.

and the specification for raise() says:

    The effect of the raise() function shall be equivalent to calling:
    pthread_kill(pthread_self(), sig);

but this appears to NOT currently be the case in Cygwin.
The current implementation of raise() in winsup/cygwin/signal.cc:

 300 extern "C" int
 301 raise (int sig)
 302 {
 303   return kill (myself->pid, sig);
 304 }

I believe this is the root cause of the observed misbehaviors with
both raise() and abort(). The Cygwin implementation of raise(sig) is
incorrectly generating a process-scope signal (discarding thread
information) rather than sending the signal to the *calling* thread,
as required by POSIX, via the same mechanism as
pthread_kill(pthread_self(),sig).

If the implementation of raise() in libc was internally replaced with
pthread_kill(pthread_self(), sig), I believe that should resolve two
of the three failure modes we've seen. I have no idea what negative
consequences (if any) there may be to that proposed change.

It's worth noting that an end user could potentially deploy a
(fragile) partial workaround by macro-defining abort and raise to
pthread_kill; but that notably would fail to capture calls made from
within libc (such as the abort() call made from
cygwin/assert.cc:__assert_func() when an invocation of assert() from
<assert.h> fails).

The remaining failure mode is a SIGSEGV generated from a programming
error (e.g. null pointer dereference) on a non-primordial thread. This
should ideally be fixed to deliver a pthread_kill() to the offending
thread, instead of the current process-wide abnormal termination that
ignores signal handlers. I agree with Madison that there is probably
no user-level workaround to cover this case at all, and I don't know
what may be required in the Win API to make this happen correctly.

Thoughts?
-D

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple