Date: Sun, 11 Jul 2010 15:59:05 -0700 From: Garrett Cooper <yanegomi@gmail.com> To: Kostik Belousov <kostikbel@gmail.com> Cc: hackers@freebsd.org Subject: Re: *sigpause hanging on 8.x+ Message-ID: <AANLkTin8xa-wgdB9QsPszN1pdqLJR7V6DNkQCEbjsPEH@mail.gmail.com> In-Reply-To: <AANLkTik79TZiA0CLubDgVpp8hY4M_C9LnQIIOKmMQ1In@mail.gmail.com> References: <AANLkTikfxu3BjQE4nnNJ-VeNSDy9vhfXEhY1XQ4jWtY4@mail.gmail.com> <20100711210843.GQ2408@deviant.kiev.zoral.com.ua> <AANLkTimjk7oJKlB7YCrZimMsTQtvKIxipZtfBfpIdSz3@mail.gmail.com> <20100711214016.GR2408@deviant.kiev.zoral.com.ua> <AANLkTik79TZiA0CLubDgVpp8hY4M_C9LnQIIOKmMQ1In@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Jul 11, 2010 at 3:35 PM, Garrett Cooper <yanegomi@gmail.com> wrote:
> 2010/7/11 Kostik Belousov <kostikbel@gmail.com>:
>> On Sun, Jul 11, 2010 at 02:30:01PM -0700, Garrett Cooper wrote:
>>> On Sun, Jul 11, 2010 at 2:08 PM, Kostik Belousov <kostikbel@gmail.com> =
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 <sys/param.h>
>>> >> =A0#include <signal.h>
>>> >> +#include <stdio.h>
>>> >> =A0#include <string.h>
>>> >> =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 <signal.h>
>>> >> #include <stdio.h>
>>> >>
>>> >> 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 <err.h>
>>> >> #include <signal.h>
>>> >>
>>> >> 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 <errno.h>
> #include <signal.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> #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);
>
> }
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTin8xa-wgdB9QsPszN1pdqLJR7V6DNkQCEbjsPEH>