Message ID | 20171122201512.7FF5E414F10CF@oldenburg.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | sigwait: Do not fail with EINTR and return error code [BZ #22478] | expand |
On 22/11/2017 18:15, Florian Weimer wrote: > Since > > commit 8b0e795aaa445e9167aa07b282c5720b35342c07 > Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> > Date: Wed Nov 1 11:49:05 2017 -0200 > > Simplify Linux sig{timed}wait{info} implementations > > sigwait can fail with EINTR. Applications do not expect that, and the > error code is not documented in POSIX or the manual pages. > > This commit restores the previous behavior by retrying the system call > on EINTR. It also returns the error code, not -1, on the remaing > errors. > > 2017-11-22 Florian Weimer <fweimer@redhat.com> > > [BZ #22478] > * sysdeps/unix/sysv/linux/sigwait.c (__sigwait): Retry on EINTR. > Return error code, not -1. > * signal/tst-sigwait-eintr.c: New file. > * signal/Makefile (tests): Add tst-sigwait-eintr. Thanks for catching it. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > diff --git a/signal/Makefile b/signal/Makefile > index a6a1289437..c2dc719d70 100644 > --- a/signal/Makefile > +++ b/signal/Makefile > @@ -45,8 +45,8 @@ routines := signal raise killpg \ > allocrtsig sigtimedwait sigwaitinfo sigqueue \ > sighold sigrelse sigignore sigset > > -tests := tst-signal tst-sigset tst-sigsimple tst-raise tst-sigset2 > - > +tests := tst-signal tst-sigset tst-sigsimple tst-raise tst-sigset2 \ > + tst-sigwait-eintr \ > > include ../Rules > > diff --git a/signal/tst-sigwait-eintr.c b/signal/tst-sigwait-eintr.c > new file mode 100644 > index 0000000000..7149eb93c3 > --- /dev/null > +++ b/signal/tst-sigwait-eintr.c > @@ -0,0 +1,85 @@ > +/* Check that sigwait does not fail with EINTR (bug 22478). > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <signal.h> > +#include <stdio.h> > +#include <support/check.h> > +#include <support/xunistd.h> > +#include <time.h> > +#include <unistd.h> > + > +/* Handler for SIGUSR1. */ > +static void > +sigusr1_handler (int signo) > +{ > + TEST_VERIFY (signo == SIGUSR1); > +} > + > +/* Spawn a subprocess to send two signals: First SIGUSR1, then > + SIGUSR2. Return the PID of the process. */ > +static pid_t > +signal_sender (void) > +{ > + pid_t pid = xfork (); > + if (pid == 0) > + { > + static const struct timespec delay = { 1, }; > + if (nanosleep (&delay, NULL) != 0) > + FAIL_EXIT1 ("nanosleep: %m"); > + if (kill (getppid (), SIGUSR1) != 0) > + FAIL_EXIT1 ("kill (SIGUSR1): %m"); > + if (nanosleep (&delay, NULL) != 0) > + FAIL_EXIT1 ("nanosleep: %m"); > + if (kill (getppid (), SIGUSR2) != 0) > + FAIL_EXIT1 ("kill (SIGUSR2): %m"); > + _exit (0); > + } > + return pid; > +} > + > +static int > +do_test (void) > +{ > + if (signal (SIGUSR1, sigusr1_handler) == SIG_ERR) > + FAIL_EXIT1 ("signal (SIGUSR1): %m\n"); > + > + sigset_t sigs; > + sigemptyset (&sigs); > + sigaddset (&sigs, SIGUSR2); > + if (sigprocmask (SIG_BLOCK, &sigs, NULL) != 0) > + FAIL_EXIT1 ("sigprocmask (SIGBLOCK, SIGUSR2): %m"); > + pid_t pid = signal_sender (); > + int signo = 0; > + int ret = sigwait (&sigs, &signo); > + if (ret != 0) > + { > + support_record_failure (); > + errno = ret; > + printf ("error: sigwait failed: %m (%d)\n", ret); > + } > + TEST_VERIFY (signo == SIGUSR2); > + > + int status; > + xwaitpid (pid, &status, 0); > + TEST_VERIFY (status == 0); > + > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/sysdeps/unix/sysv/linux/sigwait.c b/sysdeps/unix/sysv/linux/sigwait.c > index e6fb32a610..443c3ad8e1 100644 > --- a/sysdeps/unix/sysv/linux/sigwait.c > +++ b/sysdeps/unix/sysv/linux/sigwait.c > @@ -17,13 +17,20 @@ > > #include <signal.h> > #include <sysdep-cancel.h> > +#include <errno.h> > > int > __sigwait (const sigset_t *set, int *sig) > { > siginfo_t si; > - if (__sigtimedwait (set, &si, 0) < 0) > - return -1; > + int ret; > + do > + ret = __sigtimedwait (set, &si, 0); > + /* Applications do not expect sigwait to return with EINTR, and the > + error code is not specified by POSIX. */ > + while (ret < 0 && errno == EINTR); > + if (ret < 0) > + return errno; > *sig = si.si_signo; > return 0; > } >
diff --git a/signal/Makefile b/signal/Makefile index a6a1289437..c2dc719d70 100644 --- a/signal/Makefile +++ b/signal/Makefile @@ -45,8 +45,8 @@ routines := signal raise killpg \ allocrtsig sigtimedwait sigwaitinfo sigqueue \ sighold sigrelse sigignore sigset -tests := tst-signal tst-sigset tst-sigsimple tst-raise tst-sigset2 - +tests := tst-signal tst-sigset tst-sigsimple tst-raise tst-sigset2 \ + tst-sigwait-eintr \ include ../Rules diff --git a/signal/tst-sigwait-eintr.c b/signal/tst-sigwait-eintr.c new file mode 100644 index 0000000000..7149eb93c3 --- /dev/null +++ b/signal/tst-sigwait-eintr.c @@ -0,0 +1,85 @@ +/* Check that sigwait does not fail with EINTR (bug 22478). + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <signal.h> +#include <stdio.h> +#include <support/check.h> +#include <support/xunistd.h> +#include <time.h> +#include <unistd.h> + +/* Handler for SIGUSR1. */ +static void +sigusr1_handler (int signo) +{ + TEST_VERIFY (signo == SIGUSR1); +} + +/* Spawn a subprocess to send two signals: First SIGUSR1, then + SIGUSR2. Return the PID of the process. */ +static pid_t +signal_sender (void) +{ + pid_t pid = xfork (); + if (pid == 0) + { + static const struct timespec delay = { 1, }; + if (nanosleep (&delay, NULL) != 0) + FAIL_EXIT1 ("nanosleep: %m"); + if (kill (getppid (), SIGUSR1) != 0) + FAIL_EXIT1 ("kill (SIGUSR1): %m"); + if (nanosleep (&delay, NULL) != 0) + FAIL_EXIT1 ("nanosleep: %m"); + if (kill (getppid (), SIGUSR2) != 0) + FAIL_EXIT1 ("kill (SIGUSR2): %m"); + _exit (0); + } + return pid; +} + +static int +do_test (void) +{ + if (signal (SIGUSR1, sigusr1_handler) == SIG_ERR) + FAIL_EXIT1 ("signal (SIGUSR1): %m\n"); + + sigset_t sigs; + sigemptyset (&sigs); + sigaddset (&sigs, SIGUSR2); + if (sigprocmask (SIG_BLOCK, &sigs, NULL) != 0) + FAIL_EXIT1 ("sigprocmask (SIGBLOCK, SIGUSR2): %m"); + pid_t pid = signal_sender (); + int signo = 0; + int ret = sigwait (&sigs, &signo); + if (ret != 0) + { + support_record_failure (); + errno = ret; + printf ("error: sigwait failed: %m (%d)\n", ret); + } + TEST_VERIFY (signo == SIGUSR2); + + int status; + xwaitpid (pid, &status, 0); + TEST_VERIFY (status == 0); + + return 0; +} + +#include <support/test-driver.c> diff --git a/sysdeps/unix/sysv/linux/sigwait.c b/sysdeps/unix/sysv/linux/sigwait.c index e6fb32a610..443c3ad8e1 100644 --- a/sysdeps/unix/sysv/linux/sigwait.c +++ b/sysdeps/unix/sysv/linux/sigwait.c @@ -17,13 +17,20 @@ #include <signal.h> #include <sysdep-cancel.h> +#include <errno.h> int __sigwait (const sigset_t *set, int *sig) { siginfo_t si; - if (__sigtimedwait (set, &si, 0) < 0) - return -1; + int ret; + do + ret = __sigtimedwait (set, &si, 0); + /* Applications do not expect sigwait to return with EINTR, and the + error code is not specified by POSIX. */ + while (ret < 0 && errno == EINTR); + if (ret < 0) + return errno; *sig = si.si_signo; return 0; }