X-Recipient: archive-cygwin AT delorie DOT com
DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id
	:list-unsubscribe:list-subscribe:list-archive:list-post
	:list-help:sender:mime-version:references:in-reply-to:from:date
	:message-id:subject:to:cc:content-type; q=dns; s=default; b=n37L
	EeO3qeAyH60NOjq+hyEHkfZlUIi/BApa/CeUfFZaTzzRLFtnr3JjjVfBdIl1Xv+5
	ekBVgoO9WjgWBf9vvOjcVdGaV/+pMW2HpyN/i7NRoCx5eUVkriID/vNIc+B/UHSQ
	SV07QfTfb6SKXq5yQ4ilY0u7iDjPWTUYAuohIGc=
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id
	:list-unsubscribe:list-subscribe:list-archive:list-post
	:list-help:sender:mime-version:references:in-reply-to:from:date
	:message-id:subject:to:cc:content-type; s=default; bh=ryi9uQR1OG
	PkgXbiu7zyyrzHGVA=; b=unWCEuPXce9U24CDWvf37PT5EswhxseCayid1BgtZG
	b2Z9PFQmNwAo4VecdK7BqQo0ghGrtloQWNj+TrUZpsM+mC3EKKanNrKTq6cEyw+0
	lZKeG30ib8BkOQnh1YrFRXdrNSOFxlOKcJhWUPNZML2YxKv8bKuW/opp/ZRu7K2J
	4=
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
List-Id: <cygwin.cygwin.com>
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com
Authentication-Results: sourceware.org; auth=none
X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=termination, HX-Received:14e, specification, delivering
X-HELO: fe4.lbl.gov
X-Ironport-SBRS: 3.4
MIME-Version: 1.0
References: <CAJTO8-bD0jPR3Vyr84Z_4BUdh3pPiZ-9ekeMBFUOFJRn5e0wQQ AT mail DOT gmail DOT com> <CAOTD34bXKQSjbce_+qNb-9GaFu88Q3o_Qw0t4oC9yK4+A7eNiA AT mail DOT gmail DOT com> <CAJTO8-ZLqP=_d-zQtFDv=syz4mfsrDRM1YBryrcL_Rp+Wt5q=w AT mail DOT gmail DOT com> <CAOTD34Z_gYcf_X3RDVqL61ME0ZDfti=ToPqm9cxoyTOisz94qg AT mail DOT gmail DOT com>
In-Reply-To: <CAOTD34Z_gYcf_X3RDVqL61ME0ZDfti=ToPqm9cxoyTOisz94qg@mail.gmail.com>
From: Dan Bonachea <dobonachea AT lbl DOT gov>
Date: Tue, 29 Jan 2019 18:21:28 -0500
Message-ID: <CAJTO8-b3bQX-ruNvhgi5QujW_g6hAKiMZjodNKqRY87qmXbjpQ@mail.gmail.com>
Subject: Re: Bug: Incorrect signal behavior in multi-threaded processes
To: "E. Madison Bray" <erik DOT m DOT bray AT gmail DOT com>
Cc: cygwin AT cygwin DOT com, gasnet-devel AT lbl DOT gov
Content-Type: text/plain; charset="UTF-8"

> 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