On 01/23/2013 05:44 PM, Stephan Krempel wrote:
Hi there,
If the calling process has SIGALRM blocked, timeout simply never times out.
Find attached a small program to demonstrate the problem and a patch to
fix it by unblocking the necessary signal (at least on x86/linux).
The sigprocmask gnulib module is used if the sigaction
module is needed, so we should be able to use sigprocmask
unconditionally here. There is even an implementation
for windows included if needed.
If this is intended behavior, I would suggest to document it explicitly
to prevent people from tapping into the same pitfall that I hit.
I agree with your patch, and have attached an updated version.
Hmm I suppose we should also do this for SIGCHLD as a follow on from:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=73d46261
thanks,
Pádraig.
>From 72a6ea78f86bc19836d9fed4e5f1f72850e3f8a6 Mon Sep 17 00:00:00 2001
From: Stephan Krempel <krem...@par-tec.com>
Date: Fri, 25 Jan 2013 02:48:46 +0000
Subject: [PATCH] timeout: ensure a blocked SIGALRM from the parent is
unblocked
* src/timeout.c (unblock_signal): A new function to unblock a
specified signal, or warn if not possible.
(set_timeout): Ensure SIGALRM is unblocked before we setup the timer.
* tests/misc/timeout-blocked.pl: A new test for the issue.
* tests/local.mk: Reference the new test.
* NEWS: Mention the fix.
---
NEWS | 4 +++
src/timeout.c | 15 ++++++++++++
tests/local.mk | 1 +
tests/misc/timeout-blocked.pl | 48 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 68 insertions(+), 0 deletions(-)
create mode 100755 tests/misc/timeout-blocked.pl
diff --git a/NEWS b/NEWS
index 46d1aba..990b196 100644
--- a/NEWS
+++ b/NEWS
@@ -51,6 +51,10 @@ GNU coreutils NEWS -*- outline -*-
outputs a newline after the last number rather than a trailing separator.
[bug introduced in coreutils-8.20]
+ timeout now ensures that blocking of ALRM signals is not inherited from
+ its parent, which would cause timeouts to be ignored.
+ [the bug dates back to the initial implementation]
+
** Changes in behavior
df --total now prints '-' into the target column (mount point) of the
diff --git a/src/timeout.c b/src/timeout.c
index b163a0e..2ffd2b1 100644
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -102,6 +102,16 @@ static struct option const long_options[] =
{NULL, 0, NULL, 0}
};
+static void
+unblock_signal (int sig)
+{
+ sigset_t unblock_set;
+ sigemptyset (&unblock_set);
+ sigaddset (&unblock_set, sig);
+ if (sigprocmask (SIG_UNBLOCK, &unblock_set, NULL) != 0)
+ error (0, errno, _("warning: sigprocmask"));
+}
+
/* Start the timeout after which we'll receive a SIGALRM.
Round DURATION up to the next representable value.
Treat out-of-range values as if they were maximal,
@@ -110,6 +120,11 @@ static struct option const long_options[] =
static void
settimeout (double duration)
{
+
+ /* We configure timers below so that SIGALRM is sent on expiry.
+ Therefore ensure we don't inherit a mask blocking SIGALRM. */
+ unblock_signal (SIGALRM);
+
/* timer_settime() provides potentially nanosecond resolution.
setitimer() is more portable (to Darwin for example),
but only provides microsecond resolution and thus is
diff --git a/tests/local.mk b/tests/local.mk
index 82daee5..6043bb6 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -369,6 +369,7 @@ all_tests = \
tests/misc/tee-dash.sh \
tests/misc/test-diag.pl \
tests/misc/timeout.sh \
+ tests/misc/timeout-blocked.pl \
tests/misc/timeout-group.sh \
tests/misc/timeout-parameters.sh \
tests/misc/tr.pl \
diff --git a/tests/misc/timeout-blocked.pl b/tests/misc/timeout-blocked.pl
new file mode 100755
index 0000000..6f16ba4
--- /dev/null
+++ b/tests/misc/timeout-blocked.pl
@@ -0,0 +1,48 @@
+#!/usr/bin/perl
+# Test that timeout handles blocked SIGALRM from its parent.
+
+# Copyright (C) 2013 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program 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 General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+use strict;
+
+(my $ME = $0) =~ s|.*/||;
+
+eval { require POSIX; };
+$@
+ and CuSkip::skip "$ME: this script requires Perl's POSIX module\n";
+
+use POSIX qw(:signal_h);
+my $sigset = POSIX::SigSet->new(SIGALRM); # define the signals to block
+my $old_sigset = POSIX::SigSet->new; # where the old sigmask will be kept
+unless (defined sigprocmask(SIG_BLOCK, $sigset, $old_sigset)) {
+ CuSkip::skip "$ME: sigprocmask failed; skipped";
+}
+
+my @Tests =
+ (
+ # test-name, [option, option, ...] {OUT=>"expected-output"}
+ #
+
+ ['block-alrm', ".1 sleep 10", {EXIT => 124}],
+ );
+
+my $save_temps = $ENV{DEBUG};
+my $verbose = $ENV{VERBOSE};
+
+my $prog = 'timeout';
+my $fail = run_tests ($ME, $prog, \@Tests, $save_temps, $verbose);
+
+exit $fail;
--
1.7.6.4
