This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] sigwait: Do not fail with EINTR and return error code [BZ #22478]
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Thu, 23 Nov 2017 08:07:18 -0200
- Subject: Re: [PATCH] sigwait: Do not fail with EINTR and return error code [BZ #22478]
- Authentication-results: sourceware.org; auth=none
- References: <20171122201512.7FF5E414F10CF@oldenburg.str.redhat.com>
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;
> }
>