*sigpause hanging on 8.x+
Garrett Cooper
yanegomi at gmail.com
Sun Jul 11 22:35:01 UTC 2010
2010/7/11 Kostik Belousov <kostikbel at 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 at 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 testsuite
>> >> to FreeBSD, and one of the tests tests out sigpause. Unfortunately the
>> >> 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=$PWD:/usr/src/lib/libc/../libthr ~/test_sigpause
>> >> 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 amd64
>> >> VM:
>> >> FreeBSD starr-bastion.localdomain 8.0-STABLE FreeBSD 8.0-STABLE #0
>> >> r207913: Tue May 11 06:21:57 UTC 2010
>> >> root at starr-bastion.localdomain:/usr/obj/usr/src/sys/GENERIC i386
>> >>
>> >> Index: compat-43/sigcompat.c
>> >> ===================================================================
>> >> --- compat-43/sigcompat.c (revision 206173)
>> >> +++ compat-43/sigcompat.c (working copy)
>> >> @@ -36,6 +36,7 @@
>> >> #include "namespace.h"
>> >> #include <sys/param.h>
>> >> #include <signal.h>
>> >> +#include <stdio.h>
>> >> #include <string.h>
>> >> #include "un-namespace.h"
>> >> #include "libc_private.h"
>> >> @@ -102,7 +103,9 @@
>> >> {
>> >> sigset_t set;
>> >>
>> >> + printf("before sigemptyset\n");
>> >> sigemptyset(&set);
>> >> + printf("before _sigsuspend\n");
>> >> set.__bits[0] = mask;
>> >> return (_sigsuspend(&set));
>> >> }
>> >> @@ -111,10 +114,16 @@
>> >> xsi_sigpause(int sig)
>> >> {
>> >> sigset_t set;
>> >> + int rc;
>> >>
>> >> + printf("before sigemptyset\n");
>> >> sigemptyset(&set);
>> >> + printf("before sigaddset\n");
>> >> sigaddset(&set, sig);
>> >> - return (_sigsuspend(&set));
>> >> + printf("before _sigsuspend\n");
>> >> + rc = (_sigsuspend(&set));
>> >> + printf("after _sigsuspend\n");
>> >> + return rc;
>> >> }
>> >>
>> >> int
>> >>
>> >> $ cat ~/test_sigpause.c
>> >> #include <signal.h>
>> >> #include <stdio.h>
>> >>
>> >> int
>> >> main (void)
>> >> {
>> >> printf("0\n");
>> >> fflush(stdout);
>> >> (void) sigpause(1);
>> >> return 0;
>> >> }
>> >> $ cat ~/test_sigsuspend.c
>> >> #include <err.h>
>> >> #include <signal.h>
>> >>
>> >> int
>> >> main (void)
>> >> {
>> >> sigset_t oset;
>> >> sigset_t nset;
>> >> if (sigprocmask(1, &nset, &oset) == -1)
>> >> err(1, "sigprocmask(-1, &nset, &oset)");
>> >> if (sigprocmask(-1, &nset, &oset) == -1)
>> >> err(1, "sigprocmask(-1, &nset, &oset)");
>> >> return (sigsuspend(&nset));
>> >> }
>> >
>> > It seems I got a sigmask for sigpause inside the xsi_sigpause() backward.
>> > On the other hand, I do not understand what is your issue with sigpause().
>>
>> 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.
Much obliged :)... FWIW sigprocmask fails to do the right thing in
detecting the signal number:
$ ~/test_sigprocmask
signo = -1 result not sane (0 != -1, errno: 0 != EINVAL)
signo = 0 result not sane (0 != -1, errno: 0 != EINVAL)
signo = 1 result sane
signo = 9 result sane
signo = 17 result sane
signo = 65 result sane
signo = 64 result sane
signo = 66 result not sane (0 != -1, errno: 0 != EINVAL)
Would this fix that?
Index: sys/kern/kern_sig.c
===================================================================
--- sys/kern/kern_sig.c (revision 206173)
+++ sys/kern/kern_sig.c (working copy)
@@ -988,6 +988,9 @@
struct proc *p;
int error;
+ if (!_SIG_VALID(how))
+ return (-EINVAL);
+
p = td->td_proc;
if (!(flags & SIGPROCMASK_PROC_LOCKED))
PROC_LOCK(p);
I'll look for more low-hanging fruit.
>> > diff --git a/lib/libc/compat-43/sigcompat.c b/lib/libc/compat-43/sigcompat.c
>> > index c3ba30a..bab9d5c 100644
>> > --- a/lib/libc/compat-43/sigcompat.c
>> > +++ b/lib/libc/compat-43/sigcompat.c
>> > @@ -111,9 +111,12 @@ int
>> > xsi_sigpause(int sig)
>> > {
>> > sigset_t set;
>> > + int error;
>> >
>> > - sigemptyset(&set);
>> > - sigaddset(&set, sig);
>> > + error = _sigprocmask(SIG_BLOCK, NULL, &set);
>> > + if (error != 0)
>> > + return (error);
>> > + sigdelset(&set, sig);
>> > return (_sigsuspend(&set));
>> > }
>>
>> Doesn't this violate the restore clause noted in the manpage?
>>
>> The xsi_sigpause() function removes sig from the signal mask of the call-
>> ing process and suspend the calling process until a signal is received.
>> The xsi_sigpause() function restores the signal mask of the process to
>> its 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.
Yeah, I was wrong here:
The sigsuspend() system call temporarily changes the blocked signal mask
to the set to which sigmask points, and then waits for a signal to
arrive; on return the previous set of masked signals is restored. The
signal mask set is usually empty to indicate that all signals are to be
unblocked for the duration of the call.
> Also note that the sigprocmask call does not change process mask.
Not so sure about this though:
The sigprocmask() system call examines and/or changes the current signal
mask (those signals that are blocked from delivery). Signals are blocked
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.
Here's the test app I wrote and executed above, just for future reference:
Thanks!
-Garrett
$ cat ~/test_sigprocmask.c
#include <errno.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#define TEST_SIGPROCMASK_POS(signo) do { \
printf("signo = %d ", signo); \
rc = sigprocmask(-1, NULL, &oset); \
if (rc != 0) { \
printf("result not sane (%d != 0, errno: %d)\n", \
rc, errno); \
} else \
printf("result sane\n"); \
} while (0)
#define TEST_SIGPROCMASK_NEG(signo) do { \
printf("signo = %d ", signo); \
rc = sigprocmask(-1, NULL, &oset); \
if (rc != -1 || errno != EINVAL) { \
printf("result not sane (%d != -1, " \
"errno: %d != EINVAL)\n", \
rc, errno); \
} else \
printf("result sane\n"); \
} while (0)
int
main(void)
{
sigset_t oset;
int rc;
TEST_SIGPROCMASK_NEG(-1);
TEST_SIGPROCMASK_NEG(0);
TEST_SIGPROCMASK_POS(SIGHUP);
/* The system quietly disallows SIGKILL or SIGSTOP to be blocked. */
TEST_SIGPROCMASK_POS(SIGKILL);
TEST_SIGPROCMASK_POS(SIGSTOP);
TEST_SIGPROCMASK_POS(SIGRTMIN);
TEST_SIGPROCMASK_POS(SIGRTMIN-1);
TEST_SIGPROCMASK_NEG(SIGRTMIN+1);
return (0);
}
More information about the freebsd-hackers
mailing list