Dear Maintainer,

After a stopped pipeline job is resumed via the 'bg' builtin command,
the status of every command in the pipeline except the first is
incorrectly displayed. For example:

stretch% sleep 10 | sleep 15 | sleep 20
^Z
zsh: suspended  sleep 10 | sleep 15 | sleep 20
stretch% bg
[1]  + continued  sleep 10 | sleep 15 | sleep 20
stretch% jobs -l
[1]  + 1556 running    sleep 10 |
       1557 unknown signal (core dumped)                 sleep 15 |
       1558 unknown signal (core dumped)                 sleep 20
stretch%

The same thing happens regardless of what the commands are, as long as
there are at least two commands with a pipe petween them. Despite the
status, the jobs are definitely running:

stretch% cp /dev/null /tmp/emptyfile && tail -F /tmp/emptyfile | sed
's/foo/quux/'
^Z
zsh: suspended  tail -F /tmp/emptyfile | sed 's/foo/quux/'
stretch% bg
[1]  - continued  tail -F /tmp/emptyfile | sed 's/foo/quux/'
stretch% jobs -l
[1]  + 1583 running    tail -F /tmp/emptyfile |
       1584 unknown signal (core dumped)                 sed
's/foo/quux/'
stretch% echo foobar > /tmp/emptyfile
stretch% quuxbar

After the job completes, the status is correctly displayed as 'done':

stretch% kill 1583
stretch%
[1]  + terminated  tail -F /tmp/emptyfile |
       done        sed 's/foo/quux/'

While sending SIGCONT to the processes directly usually has the same
effect, by sending the signals individually in a certain order it's
actually possible to coax the correct behavior out:

stretch% sleep 120 | cat | cat
^Z
zsh: suspended  sleep 120 | cat | cat
stretch% jobs -l
[1]  + 1613 suspended  sleep 120 |
       1614 suspended  cat |
       1615 suspended  cat
stretch% kill -CONT 1615 ; jobs -l
[1]  + 1613 suspended  sleep 120 |
       1614 suspended  cat |
       1615 running    cat
stretch% kill -CONT 1614 ; jobs -l
[1]  + 1613 suspended  sleep 120 |
       1614 running    cat |
       1615 running    cat
stretch% kill -CONT 1613 ; jobs -l
[1]  + 1613 running    sleep 120 |
       1614 running    cat |
       1615 running    cat
stretch%

Trying a different order seems to suggest that it happens to the
reader of a pipe if it wakes up (receives SIGCONT) after the writer,
but not if the writer is still suspended:

stretch% jobs -l
[1]  + 1653 suspended  sleep 120 |
       1654 suspended  cat |
       1655 suspended  cat
stretch% kill -CONT 1654; jobs -l
[1]  + 1653 suspended  sleep 120 |
       1654 running    cat |
       1655 suspended  cat
stretch% kill -CONT 1655; jobs -l
[1]  + 1653 suspended  sleep 120 |
       1654 running    cat |
       1655 unknown signal (core dumped)                 cat

This is fixed in upstream zsh version 5.4 and newer. It's almost a one-liner;
does it belong in debian/patches for the stretch release?

Here's the upstream commit:

commit 12d950ba0cc345d047c94c9d94325dbfe47fc79d
Author: Barton E. Schaefer <schaefer@zsh.org>
Date:   Thu Feb 23 16:19:07 2017 -0800

    40624: conditionally handle WIFCONTINUED to properly set SP_RUNNING process status

diff --git a/ChangeLog b/ChangeLog
index d3ce452bc..8a0e059a2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2017-02-23  Barton E. Schaefer  <schaefer@zsh.org>
+
+       * 40624 (cf. Danek Duvall, 40563): Src/signals.c: conditionally
+       handle WIFCONTINUED to properly set SP_RUNNING process status
+
 2017-02-23  Jun-ichi Takimoto <takimoto-j@kba.biglobe.ne.jp>

        * 40604: configure.ac, Src/watch.c: revert to the old method if
diff --git a/Src/signals.c b/Src/signals.c
index a7176771a..68a7ae34d 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -522,6 +522,11 @@ wait_for_processes(void)
 #if defined(HAVE_WAIT3) && defined(HAVE_GETRUSAGE)
                struct timezone dummy_tz;
                gettimeofday(&pn->endtime, &dummy_tz);
+#ifdef WIFCONTINUED
+               if (WIFCONTINUED(status))
+                   pn->status = SP_RUNNING;
+               else
+#endif
                pn->status = status;
                pn->ti = ru;
 #else

Hi Nathan,

Nathan Dorfman wrote:

Thanks for the information. Marking as fixed with the according
version. (5.4 never was uploaded to Debian, hence closing as fixed
with 5.4.1-1.)

Hrm, should be possible. I though wonder if the IMHO rather small
impact of the issue validates a stable update.

Any other opinions?

Indeed looks small.

Daniel, Frank: Do we have to expect any side effects from
cherry-picking that commit for 5.3.1?

		Regards, Axel

Axel Beckert wrote on Thu, Oct 19, 2017 at 09:32:58 +0200:
-1 $version» command.  To indicate that a version is free of the bug is
«fixed -1 $version» as Axel did.

I'm not familiar with this part of the codebase, but it looks safe
enough.  Maybe upload it with urgency=low to maximise testing?

Cheers,

Daniel

Oops, thanks.

I too, was just wondering, not insisting by any means.

Cheers,
-nd.

Axel Beckert wrote:

The signals code has always been hairy  (as with any code base I'd say).
That change  looks right to  me (with my  limited knowledge of  the code
paths in question), and possible side effects should be harmless.

No guarantees, of course. :)


Regards, Frank