On 13/11/2021 09:46, Andreas Schwab wrote:
On Nov 12 2021, Pádraig Brady wrote:
I've never seen this, and I can't see the race.
There is an obvious race: if env needs more than .1 seconds to set the
SIGINT handler.
Indeed.
That fits the pattern where we need a certain delay to pass.
So we can use our retry_delay_ helper here.
I'll apply the attached later.
thanks,
Pádraig
From dd460b0a7ac029a1809ca2ef363701b431e613be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 13 Nov 2021 12:15:17 +0000
Subject: [PATCH] tests: avoid rare false failure in env-signal-handler.sh
* tests/misc/env-signal-handler.sh: Use retry_delay_ to
avoid a false failure under load, where env hasn't setup
the SIGINT handling before timeout(1) sends the SIGINT.
Fixes https://bugs.gnu.org/51793
---
tests/misc/env-signal-handler.sh | 64 +++++++++++++-------------------
1 file changed, 26 insertions(+), 38 deletions(-)
diff --git a/tests/misc/env-signal-handler.sh b/tests/misc/env-signal-handler.sh
index aa6ec8dac..c4179fe9e 100755
--- a/tests/misc/env-signal-handler.sh
+++ b/tests/misc/env-signal-handler.sh
@@ -82,62 +82,50 @@ compare /dev/null err4 || fail=1
# env test - block signal handler
env --block-signal true || fail=1
+env_ignore_delay_()
+{
+ local delay="$1"
+
+ # The first 'env' is just to ensure timeout is not a shell built-in.
+ env timeout --verbose --kill-after=.1 --signal=INT $delay \
+ env $env_opt \
+ sleep 10 > /dev/null 2>outt
+ # check only the first two lines from stderr, which are printed by timeout.
+ # (operating systems might add more messages, like "killed").
+ sed -n '1,2p' outt > out || framework_failure_
+ compare exp out
+}
+
# Baseline test - ignore signal handler
# -------------------------------------
-# Kill 'sleep' after 1 second with SIGINT - it should terminate (as SIGINT's
-# default action is to terminate a program).
-# (The first 'env' is just to ensure timeout is not the shell's built-in.)
-env timeout --verbose --kill-after=.1 --signal=INT .1 \
- sleep 10 > /dev/null 2>err5
-
-printf "timeout: sending signal INT to command 'sleep'\n" > exp-err5 \
- || framework_failure_
-
-compare exp-err5 err5 || fail=1
-
+# Kill 'sleep' after 1 second with SIGINT - it should terminate
+# (as SIGINT's default action is to terminate a program).
+cat <<\EOF >exp || framework_failure_
+timeout: sending signal INT to command 'env'
+EOF
+env_opt='' retry_delay_ env_ignore_delay_ .1 6 || fail=1
# env test - ignore signal handler
# --------------------------------
-# Use env to silence (ignore) SIGINT - "seq" should continue running
+# Use env to silence (ignore) SIGINT - "sleep" should continue running
# after timeout sends SIGINT, and be killed after 1 second using SIGKILL.
-
-cat>exp-err6 <<EOF
+cat <<\EOF >exp || framework_failure_
timeout: sending signal INT to command 'env'
timeout: sending signal KILL to command 'env'
EOF
-
-env timeout --verbose --kill-after=.1 --signal=INT .1 \
- env --ignore-signal=INT \
- sleep 10 > /dev/null 2>err6t
-
-# check only the first two lines from stderr, which are printed by timeout.
-# (operating systems might add more messages, like "killed").
-sed -n '1,2p' err6t > err6 || framework_failure_
-
-compare exp-err6 err6 || fail=1
-
+env_opt='--ignore-signal=INT' retry_delay_ env_ignore_delay_ .1 6 || fail=1
# env test - ignore signal handler (2)
# ------------------------------------
-# Repeat the previous test with "--ignore-signals" and no signal names,
+# Repeat the previous test with "--ignore-signal" and no signal names,
# i.e., all signals.
-
-env timeout --verbose --kill-after=.1 --signal=INT .1 \
- env --ignore-signal \
- sleep 10 > /dev/null 2>err7t
-
-# check only the first two lines from stderr, which are printed by timeout.
-# (operating systems might add more messages, like "killed").
-sed -n '1,2p' err7t > err7 || framework_failure_
-
-compare exp-err6 err7 || fail=1
-
+env_opt='--ignore-signal' retry_delay_ env_ignore_delay_ .1 6 || fail=1
# env test --list-signal-handling
env --default-signal --ignore-signal=INT --list-signal-handling true \
2> err8t || fail=1
sed 's/(.*)/()/' err8t > err8 || framework_failure_
-env printf 'INT (): IGNORE\n' > exp-err8
+env printf 'INT (): IGNORE\n' > exp-err8 || framework_failure_
compare exp-err8 err8 || fail=1
--
2.26.2
