From owner-freebsd-hackers@FreeBSD.ORG Sun Jul 11 22:59:07 2010 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0445C1065672 for ; Sun, 11 Jul 2010 22:59:07 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id BB2B48FC0C for ; Sun, 11 Jul 2010 22:59:06 +0000 (UTC) Received: by iwn35 with SMTP id 35so5040687iwn.13 for ; Sun, 11 Jul 2010 15:59:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=85mZuks3s0hWrYfZMLs8gqrRunMfnaOfxKcV5p2z3yk=; b=p6r9mzU2S/otqxiQ8bqyVCOC6Tprd2x60j19CjrkcskQtmVowdPkOCEUbdRQiIjc/b SitnySsrECUQHI4pqcrSafLQ6k390FF6s3MV89AIL7o70Fn4slAyLZPa1G5POv9aIXbL dRf5L15ODTHH9yHVWqBqARg79/gjwkZYbOF+g= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=WaSeIfitR96z4GE2urM/Jfba1eFSMANDf4A0tk9TEjtlBavoKLCMf4OHrXcocm2W8Q u+5ADWg/JBPXIkbSSJvhhJ4+PYeeq1Vh957yhtdHTbWIKuOULfl+yFosA0FkaBx7qf/s D/jxc66S5vGU2CqcCuoFDFS6+Smr+s/BzUEtM= MIME-Version: 1.0 Received: by 10.231.191.138 with SMTP id dm10mr1451296ibb.126.1278889145852; Sun, 11 Jul 2010 15:59:05 -0700 (PDT) Received: by 10.231.192.147 with HTTP; Sun, 11 Jul 2010 15:59:05 -0700 (PDT) In-Reply-To: References: <20100711210843.GQ2408@deviant.kiev.zoral.com.ua> <20100711214016.GR2408@deviant.kiev.zoral.com.ua> Date: Sun, 11 Jul 2010 15:59:05 -0700 Message-ID: From: Garrett Cooper To: Kostik Belousov Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: hackers@freebsd.org Subject: Re: *sigpause hanging on 8.x+ X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 11 Jul 2010 22:59:07 -0000 On Sun, Jul 11, 2010 at 3:35 PM, Garrett Cooper wrote: > 2010/7/11 Kostik Belousov : >> On Sun, Jul 11, 2010 at 02:30:01PM -0700, Garrett Cooper wrote: >>> On Sun, Jul 11, 2010 at 2:08 PM, Kostik Belousov = wrote: >>> > On Sun, Jul 11, 2010 at 12:39:39PM -0700, Garrett Cooper wrote: >>> >> So, long story short... I've basically ported the open posix testsui= te >>> >> to FreeBSD, and one of the tests tests out sigpause. Unfortunately t= he >>> >> sucker hangs on my dev box at home. >>> >> >>> >> I've written a short testcase that demonstrates this. It prints out: >>> >> >>> >> $ ~/test_sigpause >>> >> 0 >>> >> >>> >> And proceeds to be unresponsive to signals (except SIGSTOP / SIGKILL= , >>> >> as expected). >>> >> >>> >> When I monkey around with libc's compat4.3 stuff a bit, this is what= comes up: >>> >> >>> >> $ env LD_LIBRARY_PATH=3D$PWD:/usr/src/lib/libc/../libthr ~/test_sigp= ause >>> >> 0 >>> >> before sigemptyset >>> >> before _sigsuspend >>> >> >>> >> So it's getting stuck after calling _sigsuspend. >>> >> >>> >> I tried the same thing on a i386 8-STABLE VM and it hangs as well. >>> >> >>> >> I tried applying similar printfs in libthr but it's not hitting that >>> >> code at all (it's now responding to SIGTERM though, which is >>> >> interesting, but not too interesting to me). >>> >> >>> >> I also wrote similar code that exercised the functionality in >>> >> sigsuspend, by calling sigprocmask beforehand, and it works. >>> >> >>> >> Thoughts? >>> >> >>> >> -Garrett >>> >> >>> >> Dev machine: >>> >> FreeBSD bayonetta.local 9.0-CURRENT FreeBSD 9.0-CURRENT #1 >>> >> r206173:209901M: Sun Jul 11 04:18:42 PDT 2010 >>> >> root@:/usr/obj/usr/src/sys/BAYONETTA =A0amd64 >>> >> VM: >>> >> FreeBSD starr-bastion.localdomain 8.0-STABLE FreeBSD 8.0-STABLE #0 >>> >> r207913: Tue May 11 06:21:57 UTC 2010 >>> >> root@starr-bastion.localdomain:/usr/obj/usr/src/sys/GENERIC =A0i386 >>> >> >>> >> Index: compat-43/sigcompat.c >>> >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>> >> --- compat-43/sigcompat.c =A0 =A0 (revision 206173) >>> >> +++ compat-43/sigcompat.c =A0 =A0 (working copy) >>> >> @@ -36,6 +36,7 @@ >>> >> =A0#include "namespace.h" >>> >> =A0#include >>> >> =A0#include >>> >> +#include >>> >> =A0#include >>> >> =A0#include "un-namespace.h" >>> >> =A0#include "libc_private.h" >>> >> @@ -102,7 +103,9 @@ >>> >> =A0{ >>> >> =A0 =A0 =A0 sigset_t set; >>> >> >>> >> + =A0 =A0 printf("before sigemptyset\n"); >>> >> =A0 =A0 =A0 sigemptyset(&set); >>> >> + =A0 =A0 printf("before _sigsuspend\n"); >>> >> =A0 =A0 =A0 set.__bits[0] =3D mask; >>> >> =A0 =A0 =A0 return (_sigsuspend(&set)); >>> >> =A0} >>> >> @@ -111,10 +114,16 @@ >>> >> =A0xsi_sigpause(int sig) >>> >> =A0{ >>> >> =A0 =A0 =A0 sigset_t set; >>> >> + =A0 =A0 int rc; >>> >> >>> >> + =A0 =A0 printf("before sigemptyset\n"); >>> >> =A0 =A0 =A0 sigemptyset(&set); >>> >> + =A0 =A0 printf("before sigaddset\n"); >>> >> =A0 =A0 =A0 sigaddset(&set, sig); >>> >> - =A0 =A0 return (_sigsuspend(&set)); >>> >> + =A0 =A0 printf("before _sigsuspend\n"); >>> >> + =A0 =A0 rc =3D (_sigsuspend(&set)); >>> >> + =A0 =A0 printf("after _sigsuspend\n"); >>> >> + =A0 =A0 return rc; >>> >> =A0} >>> >> >>> >> =A0int >>> >> >>> >> $ cat ~/test_sigpause.c >>> >> #include >>> >> #include >>> >> >>> >> int >>> >> main (void) >>> >> { >>> >> =A0 =A0 =A0 =A0 printf("0\n"); >>> >> =A0 =A0 =A0 =A0 fflush(stdout); >>> >> =A0 =A0 =A0 =A0 (void) sigpause(1); >>> >> =A0 =A0 =A0 =A0 return 0; >>> >> } >>> >> $ cat ~/test_sigsuspend.c >>> >> #include >>> >> #include >>> >> >>> >> int >>> >> main (void) >>> >> { >>> >> =A0 =A0 =A0 =A0 sigset_t oset; >>> >> =A0 =A0 =A0 =A0 sigset_t nset; >>> >> =A0 =A0 =A0 =A0 if (sigprocmask(1, &nset, &oset) =3D=3D -1) >>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "sigprocmask(-1, &nset, &oset= )"); >>> >> =A0 =A0 =A0 =A0 if (sigprocmask(-1, &nset, &oset) =3D=3D -1) >>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err(1, "sigprocmask(-1, &nset, &oset= )"); >>> >> =A0 =A0 =A0 =A0 return (sigsuspend(&nset)); >>> >> } >>> > >>> > It seems I got a sigmask for sigpause inside the xsi_sigpause() backw= ard. >>> > On the other hand, I do not understand what is your issue with sigpau= se(). >>> >>> The negative testcase from the open posix testsuite was setup so that >>> setting sigpause(-1) would return -1 with EINVAL, according to the >>> sig* manpages (-1 is an invalid signal of course). That isn't being >>> triggered with either function today. >>> >>> 0 seems a bit wonky too (it's an invalid signal number). >>> >>> My bet is that values greater than SIGRTMAX aren't interpreted properly= either. >> >> I will add these checks, thanks. > > =A0 =A0Much obliged :)... FWIW sigprocmask fails to do the right thing in > detecting the signal number: > > $ ~/test_sigprocmask > signo =3D -1 result not sane (0 !=3D -1, errno: 0 !=3D EINVAL) > signo =3D 0 result not sane (0 !=3D -1, errno: 0 !=3D EINVAL) > signo =3D 1 result sane > signo =3D 9 result sane > signo =3D 17 result sane > signo =3D 65 result sane > signo =3D 64 result sane > signo =3D 66 result not sane (0 !=3D -1, errno: 0 !=3D EINVAL) > > =A0 =A0Would this fix that? > > Index: sys/kern/kern_sig.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/kern/kern_sig.c (revision 206173) > +++ sys/kern/kern_sig.c (working copy) > @@ -988,6 +988,9 @@ > =A0 =A0 =A0 =A0struct proc *p; > =A0 =A0 =A0 =A0int error; > > + =A0 =A0 =A0 if (!_SIG_VALID(how)) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (-EINVAL); > + > =A0 =A0 =A0 =A0p =3D td->td_proc; > =A0 =A0 =A0 =A0if (!(flags & SIGPROCMASK_PROC_LOCKED)) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0PROC_LOCK(p); *snickers* no, that definitely doesn't fix the problem: $ ~/test_sigprocmask signo =3D -1 result not sane (-1 !=3D -1, errno: -22 !=3D EINVAL) signo =3D 0 result not sane (-1 !=3D -1, errno: -22 !=3D EINVAL) signo =3D 1 result not sane (-1 !=3D 0, errno: -22) signo =3D 9 result not sane (-1 !=3D 0, errno: -22) signo =3D 17 result not sane (-1 !=3D 0, errno: -22) signo =3D 65 result not sane (-1 !=3D 0, errno: -22) signo =3D 64 result not sane (-1 !=3D 0, errno: -22) signo =3D 66 result not sane (-1 !=3D -1, errno: -22 !=3D EINVAL) > =A0 =A0I'll look for more low-hanging fruit. > >>> > diff --git a/lib/libc/compat-43/sigcompat.c b/lib/libc/compat-43/sigc= ompat.c >>> > index c3ba30a..bab9d5c 100644 >>> > --- a/lib/libc/compat-43/sigcompat.c >>> > +++ b/lib/libc/compat-43/sigcompat.c >>> > @@ -111,9 +111,12 @@ int >>> > =A0xsi_sigpause(int sig) >>> > =A0{ >>> > =A0 =A0 =A0 =A0sigset_t set; >>> > + =A0 =A0 =A0 int error; >>> > >>> > - =A0 =A0 =A0 sigemptyset(&set); >>> > - =A0 =A0 =A0 sigaddset(&set, sig); >>> > + =A0 =A0 =A0 error =3D _sigprocmask(SIG_BLOCK, NULL, &set); >>> > + =A0 =A0 =A0 if (error !=3D 0) >>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (error); >>> > + =A0 =A0 =A0 sigdelset(&set, sig); >>> > =A0 =A0 =A0 =A0return (_sigsuspend(&set)); >>> > =A0} >>> >>> Doesn't this violate the restore clause noted in the manpage? >>> >>> =A0 =A0 =A0The xsi_sigpause() function removes sig from the signal mask= of the call- >>> =A0 =A0 =A0ing process and suspend the calling process until a signal i= s received. >>> =A0 =A0 =A0The xsi_sigpause() function restores the signal mask of the = process to >>> =A0 =A0 =A0its original state before returning. >>> >>> So if I had a sigset defined above with sig, then redefined it, I >>> would be whacking the previous handler by passing in NULL to >>> _sigprocmask, correct? If so, sigpause has issues too in its >>> implementation. >> No, not correct. Read the description of sigsuspend. > > =A0 =A0Yeah, I was wrong here: > > =A0 =A0 The sigsuspend() system call temporarily changes the blocked sign= al mask > =A0 =A0 to the set to which sigmask points, and then waits for a signal t= o > =A0 =A0 arrive; on return the previous set of masked signals is restored.= =A0The > =A0 =A0 signal mask set is usually empty to indicate that all signals are= to be > =A0 =A0 unblocked for the duration of the call. > >> Also note that the sigprocmask call does not change process mask. > > =A0 =A0Not so sure about this though: > > =A0 =A0 The sigprocmask() system call examines and/or changes the current= signal > =A0 =A0 mask (those signals that are blocked from delivery). =A0Signals a= re blocked > =A0 =A0 if they are members of the current signal mask set. > >>> There's also some interesting SIGDELSET action going on in libthr's >>> copy of _sigsuspend's with SIGCANCEL (apparently that's the unofficial >>> alias for SIGRTMIN as defined by libthr), but that's a sidenote for >>> the actual issue seen here. > > =A0 =A0Here's the test app I wrote and executed above, just for future re= ference: > Thanks! > -Garrett > > $ cat ~/test_sigprocmask.c > #include > #include > #include > #include > > #define TEST_SIGPROCMASK_POS(signo) do { =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("signo =3D %d ", signo); =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rc =3D sigprocmask(-1, NULL, &oset); =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (rc !=3D 0) { =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("result not sane (%= d !=3D 0, errno: %d)\n", \ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rc, errno); =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("result sane\n"); = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \ > =A0 =A0 =A0 =A0} while (0) > > #define TEST_SIGPROCMASK_NEG(signo) do { =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("signo =3D %d ", signo); =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rc =3D sigprocmask(-1, NULL, &oset); =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (rc !=3D -1 || errno !=3D EINVAL) { =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("result not sane (%= d !=3D -1, " =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"errno: %d !=3D EI= NVAL)\n", =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rc, errno); =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("result sane\n"); = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\ > =A0 =A0 =A0 =A0} while (0) > > int > main(void) > { > =A0 =A0 =A0 =A0sigset_t oset; > =A0 =A0 =A0 =A0int rc; > > =A0 =A0 =A0 =A0TEST_SIGPROCMASK_NEG(-1); > =A0 =A0 =A0 =A0TEST_SIGPROCMASK_NEG(0); > =A0 =A0 =A0 =A0TEST_SIGPROCMASK_POS(SIGHUP); > =A0 =A0 =A0 =A0/* The system quietly disallows SIGKILL or SIGSTOP to be b= locked. */ > =A0 =A0 =A0 =A0TEST_SIGPROCMASK_POS(SIGKILL); > =A0 =A0 =A0 =A0TEST_SIGPROCMASK_POS(SIGSTOP); > =A0 =A0 =A0 =A0TEST_SIGPROCMASK_POS(SIGRTMIN); > =A0 =A0 =A0 =A0TEST_SIGPROCMASK_POS(SIGRTMIN-1); > =A0 =A0 =A0 =A0TEST_SIGPROCMASK_NEG(SIGRTMIN+1); > > =A0 =A0 =A0 =A0return (0); > > } >