From owner-freebsd-threads@FreeBSD.ORG Fri Nov 21 16:07:21 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 3672BF66; Fri, 21 Nov 2014 16:07:21 +0000 (UTC) Received: from 0.mx.codelabs.ru (0.mx.codelabs.ru [144.206.233.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C6AA0334; Fri, 21 Nov 2014 16:07:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=codelabs.ru; s=three; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=0Naqu0nVafz1FtfCYPX2VdrqJUecWdH1tErmzG3pKug=; b=mkgakdqGz8+v9kYlh9n9YtwGh917mlID7bsLbSr3qXf9b3AYa/bRIQbm2aWuTyW7seH515/OoNTn5Gz2xjC82/wgl9MXMs9W5erEV2YvKs9EIqUzCOikqndKX4NHL8raM0kvhVqNRL7OphxH+Cd13dk/nL6sKhNYMOM002YaQj6fHJ92+DSRZTID9UYjgWZto4hTi7VNqk3DKrV2YD3X7C62ZF78PwbEmuen+5dfS/IeyjTa3KKjIZvmD9sKI6+aIVkzToleK7cWoRoTRF83ieAnQ6TXs4w/+QjFHKZn0ToPdinBOJE/xAGZfiu0MAzO4UWSAF5zdnKV5nj31uGoLgvYJuvg8WNBPgSEmyg1NWbyRnsJOqB+rHDJc+3ndcduM9/Fw0On68traMwOUIDRN9aE5aTOvaaojFBGPqEbl6+YT20mhIzI/YmZ6as531KJo0kZGqr0UwXMbjZa9iT3PTZWe5s8J6BdvkYbLW3hDC1/izQ9BgkpoC75g4JclifqO9Vfm/wYaQ2RF3aEGuNefYJqUO7NqMFI6+4/gGEsxz83nhiM0n3bI0BNcu6/2Sar01782fX8rJRv2cGSHOyK+TFwd+S4YWNzfuWgNMF870bWsr3hnnByMpl7AiJaZgnqv3P6kelLpS8E/WiJJ07+z1DvCpnBAtk2Zq1IM6qqyTY=; Received: from void.codelabs.ru (void.codelabs.ru [144.206.233.66]) by 0.mx.codelabs.ru with esmtpsa (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) id 1Xrqjs-000Ina-US; Fri, 21 Nov 2014 20:07:17 +0400 Date: Fri, 21 Nov 2014 19:07:14 +0300 From: Eygene Ryabinkin To: Konstantin Belousov Subject: Re: [CFR][PATCH] Call proper signal handler from libthr for non-SA_SIGINFO case Message-ID: References: <20141120194925.GK17068@kib.kiev.ua> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="LXqH7sWsIplhrbjF" Content-Disposition: inline In-Reply-To: <20141120194925.GK17068@kib.kiev.ua> Sender: rea@codelabs.ru X-Content-Filtered-By: Mailman/MimeDel 2.1.18-1 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:07:21 -0000 --LXqH7sWsIplhrbjF Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Konstantin, good day. Thu, Nov 20, 2014 at 09:49:25PM +0200, Konstantin Belousov wrote: > On Thu, Nov 20, 2014 at 09:32:19PM +0300, Eygene Ryabinkin wrote: > > Was hacking on Squid that used to dump core when signal 15 was > > delivered to it via 'squid -k shutdown' and found out that the reason > > for core dumps is that thr_sighandler() is _sometimes_ passed 0x10001 > > as the value of "info" argument despite that _sigaction() always arms > > flags for __sys_sigaction() with SA_SIGINFO. > >=20 > > But looking at handle_signal() I had discovered that if libthr client > > wants no SA_SIGINFO, then actp->sa_sigaction() will be called, though > > specs, > > http://pubs.opengroup.org/onlinepubs/009695399/functions/sigaction.ht= ml > > say that in this case sa_handler should be used. And this is consistent > > with the non-threaded case. > > The sa_sigaction and sa_handler live in the same bits, look at the > union in the definition of the struct sigaction in sys/signal.h. It is > the same pointer, cast to different functions signature depending on > SA_SIGSINFO. Sorry, should have been studied sigaction(2) more carefully: everything turned out to be written there. So, libthr is fine here. > > The origin of 0x10001 as info isn't yet clear to me, though I have > > a feeling that it is SI_USER that is slipping somewhere to the wrong > > place. Will dig further. > > It is not clear to me what your problem is, at all. 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. 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. 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. --=20 Eygene Ryabinkin ,,,^..^,,, [ Life's unfair - but root password helps! | codelabs.ru ] [ 82FE 06BC D497 C0DE 49EC 4FF0 16AF 9EAE 8152 ECFB | freebsd.org ] --LXqH7sWsIplhrbjF Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iL4EABEKAGYFAlRvYzJfFIAAAAAALgAoaXNzdWVyLWZwckBub3RhdGlvbnMub3Bl bnBncC5maWZ0aGhvcnNlbWFuLm5ldDgyRkUwNkJDRDQ5N0MwREU0OUVDNEZGMDE2 QUY5RUFFODE1MkVDRkIACgkQFq+eroFS7PuBvAD/XrhsUYpvMRgUJXW6W22OGN9R EKZYqQhKw8IyFYl1NLkA/0FOEjikQJWMA1SH6BB3h+3osS7e0AQkS2cwOAulpUM7 =RnhA -----END PGP SIGNATURE----- --LXqH7sWsIplhrbjF--