timeout: fix a small race that would ignore command exit

This fixes a regression from commit v8.26-39-g2f69dba

* src/timeout.c (block_cleanup_and_chld): Rename from block_cleanup
to indicate we also block SIGCHLD to avoid the race where SIGCHLD
fires between waitpid() polling and sigsuspend() waiting for a signal.
* NEWS: Mention the fix.

diff --git a/NEWS b/NEWS
index c3128e1451084946161176b3c46edb8b7760c95d..18922072abc894c50e42e500417e605079437a7d 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -12,7 +12,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   [bug introduced in coreutils-8.28]
 
   timeout will again notice its managed command exiting, even when
-  invoked with blocked CHLD signal.
+  invoked with blocked CHLD signal, or in a narrow window where
+  this CHLD signal from the exiting child was missed.  In each case
+  timeout would have then waited for the time limit to expire.
   [bug introduced in coreutils-8.27]
 
 ** Build-related

diff --git a/src/timeout.c b/src/timeout.c
index 9be70e01521d1c073820cfce9c3e044aa0036707..562781943edd84c4bdd767711f6ce4a4a512c727 100644 (file)
--- a/src/timeout.c
+++ b/src/timeout.c
@@ -366,19 +366,26 @@ install_cleanup (int sigterm)
   sigaction (sigterm, &sa, NULL); /* user specified termination signal.  */
 }
 
-/* Blocks all signals which were registered with cleanup
-   as signal handler.  Return original mask in OLD_SET.  */
+/* Block all signals which were registered with cleanup() as the signal
+   handler, so we never kill processes after waitpid() returns.
+   Also block SIGCHLD to ensure it doesn't fire between
+   waitpid() polling and sigsuspend() waiting for a signal.
+   Return original mask in OLD_SET.  */
 static void
-block_cleanup (int sigterm, sigset_t *old_set)
+block_cleanup_and_chld (int sigterm, sigset_t *old_set)
 {
   sigset_t block_set;
   sigemptyset (&block_set);
+
   sigaddset (&block_set, SIGALRM);
   sigaddset (&block_set, SIGINT);
   sigaddset (&block_set, SIGQUIT);
   sigaddset (&block_set, SIGHUP);
   sigaddset (&block_set, SIGTERM);
   sigaddset (&block_set, sigterm);
+
+  sigaddset (&block_set, SIGCHLD);
+
   if (sigprocmask (SIG_BLOCK, &block_set, old_set) != 0)
     error (0, errno, _("warning: sigprocmask"));
 }
@@ -508,7 +515,7 @@ main (int argc, char **argv)
       /* Ensure we don't cleanup() after waitpid() reaps the child,
          to avoid sending signals to a possibly different process.  */
       sigset_t cleanup_set;
-      block_cleanup (term_signal, &cleanup_set);
+      block_cleanup_and_chld (term_signal, &cleanup_set);
 
       while ((wait_result = waitpid (monitored_pid, &status, WNOHANG)) == 0)
         sigsuspend (&cleanup_set);  /* Wait with cleanup signals unblocked.  */