From owner-freebsd-threads@FreeBSD.ORG Fri Nov 21 16:57:04 2014 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 9B7BB9E4; Fri, 21 Nov 2014 16:57:04 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 255A8B7A; Fri, 21 Nov 2014 16:57:03 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id sALGuwCq020767 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 21 Nov 2014 18:56:58 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua sALGuwCq020767 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id sALGuwN5020766; Fri, 21 Nov 2014 18:56:58 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 21 Nov 2014 18:56:58 +0200 From: Konstantin Belousov To: Eygene Ryabinkin Subject: Re: [CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case Message-ID: <20141121165658.GP17068@kib.kiev.ua> References: <20141120194925.GK17068@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: davidxu@FreeBSD.org, freebsd-threads@FreeBSD.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Nov 2014 16:57:04 -0000 On Fri, Nov 21, 2014 at 07:07:14PM +0300, Eygene Ryabinkin wrote: > The problem is that libthr's _sigaction() proxies sa_flags, so if > SA_RESETHAND was specified by the caller, it will also be passed to > the kernel when thr_sighandler() is installed. And since sa_flags are > equipped with SA_SIGINFO inside _sigaction(), thr_sighandler() and > handle_signal() always expect to be called with POSIX SA_SIGINFO > semantics. > > This isn't the case for SA_RESETHAND, because sigdflt() from > kern/kern_sig.c will be called before mach-dependent sv_sendsig > vector, so > {{{ > SIGDELSET(ps->ps_siginfo, sig) > }}} > will be issued. Thus, any code inside mach-dependent handler won't > see ps_siginfo, so will always use traditional semantics. This > explains why I see 0x10001 as "siginfo_t *info": it is just "int code" > for the traditional case and the signal in question is really produced > by userland, so it is SI_USER. Great, thank you for tracking this down. > > 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. 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. > > Had created a patch that adds 4th argument to sv_sendsig and fixes > this problem: > http://codelabs.ru/fbsd/patches/libthr/11-CURRENT-fix-SIGINFO-processing-with-RESETHAND.diff > This changes internal KABI, but hopefully sv_sendsig is an internal > kernel interface that isn't used by anything else outside kern_sig.c. > > 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@f.o. The test would require adoption to the framework. diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 5cdc2ce..2c3cc80 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -2863,14 +2863,14 @@ postsig(sig) kern_sigprocmask(td, SIG_BLOCK, &mask, NULL, SIGPROCMASK_PROC_LOCKED | SIGPROCMASK_PS_LOCKED); - if (SIGISMEMBER(ps->ps_sigreset, sig)) - sigdflt(ps, sig); td->td_ru.ru_nsignals++; if (p->p_sig == sig) { p->p_code = 0; p->p_sig = 0; } (*p->p_sysent->sv_sendsig)(action, &ksi, &returnmask); + if (SIGISMEMBER(ps->ps_sigreset, sig)) + sigdflt(ps, sig); } return (1); }