ash: jobs: Fix waitcmd busy loop

Upstream commit:

    Date: Tue, 2 Jun 2020 23:46:48 +1000
    jobs: Fix waitcmd busy loop

    We need to clear gotsigchld in waitproc because it is used as
    a loop conditional for the waitcmd case.  Without it waitcmd
    may busy loop after a SIGCHLD.

    This patch also changes gotsigchld into a volatile sig_atomic_t
    to prevent compilers from optimising its accesses away.

    Fixes: 6c691b3e5099 ("jobs: Only clear gotsigchld when waiting...")
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

This change also incorporates other changes to bring us closer to upstream.

function                                             old     new   delta
dowait                                               553     636     +83

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

1 files changed, 34 insertions, 57 deletions

diff --git a/shell/ash.c b/shell/ash.c
index 13470b2fa..07aa2da2e 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -4273,64 +4273,55 @@ sprint_status48(char *os, int status, int sigonly)
         return s - os;
 }
 
+#define DOWAIT_NONBLOCK 0
+#define DOWAIT_BLOCK    1
+#define DOWAIT_BLOCK_OR_SIG 2
+#if BASH_WAIT_N
+# define DOWAIT_JOBSTATUS 0x10   /* OR this to get job's exitstatus instead of pid */
+#endif
+
 static int
-wait_block_or_sig(int *status)
+waitproc(int block, int *status)
 {
-        int pid;
+        sigset_t oldmask;
+        int flags = block == DOWAIT_BLOCK ? 0 : WNOHANG;
+        int err;
 
-        do {
-                sigset_t mask;
+#if JOBS
+        if (doing_jobctl)
+                flags |= WUNTRACED;
+#endif
 
-                /* Poll all children for changes in their state */
+        do {
                 got_sigchld = 0;
-                /* if job control is active, accept stopped processes too */
-                pid = waitpid(-1, status, doing_jobctl ? (WNOHANG|WUNTRACED) : WNOHANG);
-                if (pid != 0)
-                        break; /* Error (e.g. EINTR, ECHILD) or pid */
+                do
+                        err = waitpid(-1, status, flags);
+                while (err < 0 && errno == EINTR);
 
-                /* Children exist, but none are ready. Sleep until interesting signal */
-#if 1
-                sigfillset(&mask);
-                sigprocmask2(SIG_SETMASK, &mask); /* mask is updated */
-                while (!got_sigchld && !pending_sig) {
-                        sigsuspend(&mask);
-                        /* ^^^ add "sigdelset(&mask, SIGCHLD);" before sigsuspend
-                         * to make sure SIGCHLD is not masked off?
-                         * It was reported that this:
-                         *      fn() { : | return; }
-                         *      shopt -s lastpipe
-                         *      fn
-                         *      exec ash SCRIPT
-                         * under bash 4.4.23 runs SCRIPT with SIGCHLD masked,
-                         * making "wait" commands in SCRIPT block forever.
-                         */
-                }
-                sigprocmask(SIG_SETMASK, &mask, NULL);
-#else /* unsafe: a signal can set pending_sig after check, but before pause() */
+                if (err || (err = -!block))
+                        break;
+
+                sigfillset(&oldmask);
+                sigprocmask2(SIG_SETMASK, &oldmask); /* mask is updated */
                 while (!got_sigchld && !pending_sig)
-                        pause();
-#endif
+                        sigsuspend(&oldmask);
+                sigprocmask(SIG_SETMASK, &oldmask, NULL);
+                //simpler, but unsafe: a signal can set pending_sig after check, but before pause():
+                //while (!got_sigchld && !pending_sig)
+                //      pause();
 
-                /* If it was SIGCHLD, poll children again */
         } while (got_sigchld);
 
-        return pid;
+        return err;
 }
 
-#define DOWAIT_NONBLOCK 0
-#define DOWAIT_BLOCK    1
-#define DOWAIT_BLOCK_OR_SIG 2
-#if BASH_WAIT_N
-# define DOWAIT_JOBSTATUS 0x10   /* OR this to get job's exitstatus instead of pid */
-#endif
-
 static int
 waitone(int block, struct job *job)
 {
         int pid;
         int status;
         struct job *jp;
-        struct job *thisjob;
+        struct job *thisjob = NULL;
 #if BASH_WAIT_N
         bool want_jobexitstatus = (block & DOWAIT_JOBSTATUS);
         block = (block & ~DOWAIT_JOBSTATUS);
@@ -4357,21 +4348,8 @@ waitone(int block, struct job *job)
          * SIG_DFL handler does not wake sigsuspend().
          */
         INT_OFF;
-        if (block == DOWAIT_BLOCK_OR_SIG) {
-                pid = wait_block_or_sig(&status);
-        } else {
-                int wait_flags = 0;
-                if (block == DOWAIT_NONBLOCK)
-                        wait_flags = WNOHANG;
-                /* if job control is active, accept stopped processes too */
-                if (doing_jobctl)
-                        wait_flags |= WUNTRACED;
-                /* NB: _not_ safe_waitpid, we need to detect EINTR */
-                pid = waitpid(-1, &status, wait_flags);
-        }
-        TRACE(("wait returns pid=%d, status=0x%x, errno=%d(%s)\n",
-                                pid, status, errno, strerror(errno)));
-        thisjob = NULL;
+        pid = waitproc(block, &status);
+        TRACE(("wait returns pid %d, status=%d\n", pid, status));
         if (pid <= 0)
                 goto out;
 
@@ -4466,7 +4444,6 @@ dowait(int block, struct job *jp)
         rpid = 1;
 
         do {
-                got_sigchld = 0;
                 pid = waitone(block, jp);
                 rpid &= !!pid;
 
@@ -4721,7 +4698,7 @@ waitcmd(int argc UNUSED_PARAM, char **argv)
                         job = getjob(*argv, 0);
                 }
                 /* loop until process terminated or stopped */
-                dowait(DOWAIT_BLOCK_OR_SIG, NULL);
+                dowait(DOWAIT_BLOCK_OR_SIG, job);
                 if (pending_sig)
                         goto sigout;
                 job->waited = 1;