[CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case

[Eygene Ryabinkin]

Fri, Nov 21, 2014 at 06:56:58PM +0200, Konstantin Belousov wrote:
> On Fri, Nov 21, 2014 at 07:07:14PM +0300, Eygene Ryabinkin wrote:
> > So, it looks like a kernel bug (since if we request SA_SIGINFO, we
> > should get the proper handler to be called even for the SA_RESETHAND
> > case).  I see two possibilities:
> > 
> >  - invoke SA_RESETHAND processing inside mach-dependent code; that's
> >    a kind of ugly and makes mach-specific code to deal with the
> >    generic signal handling logics;
> > 
> >  - pass information about SA_SIGINFO "out-of-band" (not in
> >    ps->ps_siginfo).
> > 
> > We can't postpone sigdflt() to be called after signal being delivered,
> > since spec requires it to be done before calling user-space handler.
>
> No, userspace is not called from sv_sendsig(), it simply cannot work
> because nested signals would cause nesting on the kernel stack frame.
> sv_sendsig() only prepares the usermode signal stack frame and arranges
> the saved thread CPU state so that on return from kernel to usermode,
> the signal handler is executed.

So, if we rearrange calls to sv_sendsig() and sigdflt() there can't be
cases when the process being signalled will pick the signal before
sigdflt() will be called?  Just because XSI wants that handler reset
and clearing of SIGINFO happen before handler execution,
  http://pubs.opengroup.org/onlinepubs/009695399/functions/sigaction.html
So, I went into more trouble of not touching the order.

> And, amusingly, the only thing which sv_sendsig() methods need and which
> is also touched by sigdflt(), is ps_siginfo.  Simply rearranging the
> order of calls should be enough.  I put the patch at the end of message,
> it worked for me.

If there is no change in behaviour that will arise from rearranging
the order of calls to mach-dependent and mach-independent code,
I'd go a bit firther and unify some repeated code,
  http://codelabs.ru/fbsd/patches/libthr/11-CURRENT-fix-SIGINFO-processing-with-RESETHAND-v2.diff

Works for me too, just tested with the same Squid installation.

> > It works fine with virgin libthr and solves Squid restart problem.
> > Will try to install it to more test machines and see if it will work
> > as expected.  Seems like a kernel regression test like the attached
> > one will also be handy.
> 
> There are people who develop the regression suite(s) for FreeBSD.
> Might be, they would note your test, may be, you should somehow contact
> them and propose the inclusion of the test into suite.  I believe
> it is freebsd-testing at f.o.  The test would require adoption to the
> framework.

Yep, will try to catch this train.

Thanks!
-- 
Eygene Ryabinkin                                        ,,,^..^,,,
[ Life's unfair - but root password helps!           | codelabs.ru ]
[ 82FE 06BC D497 C0DE 49EC  4FF0 16AF 9EAE 8152 ECFB | freebsd.org ]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 358 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-threads/attachments/20141121/ddaac8f2/attachment.sig>