Re: _lwp_create change

[christos%astron.com@localhost (Christos Zoulas)]

In article <20170420002926.GA17755%britannica.bec.de@localhost>,
Joerg Sonnenberger  <joerg%bec.de@localhost> wrote:
>On Wed, Apr 19, 2017 at 07:52:25PM -0400, Christos Zoulas wrote:
>> This issue came up recently with the go language signal tests failing.
>> The problem is that NetBSD copies the l_sigstk on lwp_creation and if
>> threads happen to get signals at the same time, then they share the
>> signal stack and they die (if sigaltstack is set). Go attempts to work
>> around this by blocking the signals (setting the signal mask on lwp
>> creation, and then calling sigaltstack() itself):
>
>As discussed previously, I think the patch makes the behavior worse and
>the Go implementation is kind of stupid as well. To correcly ensure that
>signals always use the signal stack:
>
>(1) Allocate a new signal stack.
>(2) Mask all signals.
>(3) Create the new thread.
>(4) Switch the *old* stack to the new signal stack.
>(5) Unmask signals again.
>
>Variant:
>(1) Allocate a new signal stack.
>(2) Mask all signals.
>(3) Switch the *old* stack to the new signal stack, remember the old
>    setting.
>(4) Create the new thread.
>(5) Switch the *old* stack back to the original signal stack.
>(6) Unmask signals again.
>
>This is significantly more flexible than the proposed hack and requires
>no code changes in the kernel. It does assume that no synchronous
>signals happen between step (2) and (5) [or (2) and (6) respectively],
>but if you want to segfault anyway, it likely doesn't really matter.
>In either case, the new thread starts with the correct signal stack
>without any race condition.

Actually the first scenario is:

(1.l1) get current context
(2.l1) Mask all signals.
(3.l1) Create the new thread.
(4.l1) Unmask all signals

(1.l2) get the current stack
(2.l2) set the new stack if needed
(3.l2) Unmask signals again.

_lwp_create is documented to use the signal mask from the context, but it does
not. This is a bug according to the documentation. If we allow both the stack
and the mask to be set from the context in lwp_create the above becomes (my
proposal):

(1.l1) get current context/set mask/stack in context for new thread.
(2.l1) Create the new thread.

(1.l2) Unmask signals 

If we just fix the sigmask bug:

(1.l1) get current context/set mask in context for new thread.
(2.l1) Create the new thread.

(1.l2) get the current stack
(2.l2) set the new stack if needed
(3.l2) Unmask signals again.

I.e. we save 2 syscalls on the parent thread.

christos