Exit code 130 kills pipeline

Exit code 130 kills pipeline

[Roman Perepelitsa]

Is it expected that the following command produces no output?

  ( exit 130 ) | { sleep 1; echo hello }

Roman.

Re: Exit code 130 kills pipeline

[Peter Stephenson]

> On 17 June 2020 at 16:45 Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> Is it expected that the following command produces no output?
> 
>   ( exit 130 ) | { sleep 1; echo hello }

There's some slightly dodgy use of flags when testing for signals.

I'm not sure "or"ing in the 128 is really needed at all but to, keep
this minimal, all we need to do is test separately whether the process
actually got a signal.  If it didn't we've no business looking for
SIGINT or SIGQUIT.

pws

diff --git a/Src/jobs.c b/Src/jobs.c
index 8353f1152..0d4993554 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -442,7 +442,7 @@ update_job(Job jn)
     Process pn;
     int job;
     int val = 0, status = 0;
-    int somestopped = 0, inforeground = 0;
+    int somestopped = 0, inforeground = 0, signalled = 0;
 
     for (pn = jn->auxprocs; pn; pn = pn->next) {
 #ifdef WIFCONTINUED
@@ -464,12 +464,15 @@ update_job(Job jn)
 	    return;                        /* so no need to update job table entry         */
 	if (WIFSTOPPED(pn->status))        /* some processes are stopped                   */
 	    somestopped = 1;               /* so job is not done, but entry needs updating */
-	if (!pn->next)                     /* last job in pipeline determines exit status  */
+	if (!pn->next) {
+	    /* last job in pipeline determines exit status  */
 	    val = (WIFSIGNALED(pn->status) ?
 		   0200 | WTERMSIG(pn->status) :
 		   (WIFSTOPPED(pn->status) ?
 		    0200 | WEXITSTATUS(pn->status) :
 		    WEXITSTATUS(pn->status)));
+	    signalled = WIFSIGNALED(pn->status);
+	}
 	if (pn->pid == jn->gleader)        /* if this process is process group leader      */
 	    status = pn->status;
     }
@@ -564,7 +567,7 @@ update_job(Job jn)
 		}
 		/* If we have `foo|while true; (( x++ )); done', and hit
 		 * ^C, we have to stop the loop, too. */
-		if ((val & 0200) && inforeground == 1 &&
+		if (signalled && inforeground == 1 &&
 		    ((val & ~0200) == SIGINT || (val & ~0200) == SIGQUIT)) {
 		    if (!errbrk_saved) {
 			errbrk_saved = 1;
@@ -581,7 +584,7 @@ update_job(Job jn)
 		adjustwinsize(0);
 	    }
 	}
-    } else if (list_pipe && (val & 0200) && inforeground == 1 &&
+    } else if (list_pipe && signalled && inforeground == 1 &&
 	       ((val & ~0200) == SIGINT || (val & ~0200) == SIGQUIT)) {
 	if (!errbrk_saved) {
 	    errbrk_saved = 1;

Re: Exit code 130 kills pipeline

[Roman Perepelitsa]

On Wed, Jun 17, 2020 at 6:59 PM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> diff --git a/Src/jobs.c b/Src/jobs.c
> index 8353f1152..0d4993554 100644
> --- a/Src/jobs.c
> +++ b/Src/jobs.c
> [...]

Thanks, Peter. I confirm that this patch fixes the issue I was having.

Roman.

Re: Exit code 130 kills pipeline

[Peter Stephenson]

Not a direct part of this bug, but while I'm looking I might at least
mention it, though I suspect it's very minor in practice.

Just looking at the code that sets the return value:

	    /* last job in pipeline determines exit status  */
	    val = (WIFSIGNALED(pn->status) ?
		   0200 | WTERMSIG(pn->status) :
		   (WIFSTOPPED(pn->status) ?
		    0200 | WEXITSTATUS(pn->status) :
		    WEXITSTATUS(pn->status)));
	    signalled = WIFSIGNALED(pn->status);

The only case where the bottom bits are the signal is WIFSIGNALED();
that's the fix that's just gone in, so no problem with that one.

However, I notice the Linux manual page says WEXITSTATUS() is only
useful if WIFEXITED() is true for the status.  So the remaining
two cases could possibly do with a bit of surgery.  I'm guessing
WIFSTOPPED() usually reports a status of 128 + 0, and if the
process is stopped (not exited) I'm not sure if the user gets
to see the status anyway.  So this may be a non-issue.

pws

Re: Exit code 130 kills pipeline

[Mikael Magnusson]

On 6/17/20, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> Is it expected that the following command produces no output?
>
>   ( exit 130 ) | { sleep 1; echo hello }

No?
% ( exit 130 ) | { sleep 1; echo hello }
hello

-- 
Mikael Magnusson

Re: Exit code 130 kills pipeline

[Roman Perepelitsa]

On Wed, Jun 17, 2020 at 7:42 PM Mikael Magnusson <mikachu@gmail.com> wrote:
>
> On 6/17/20, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> > Is it expected that the following command produces no output?
> >
> >   ( exit 130 ) | { sleep 1; echo hello }
>
> No?

Good point. I should've specified the environment.

% docker run -it --rm zshusers/zsh:5.8 \
  zsh -c '( exit 130 ) | { sleep 1; echo hello }'

(no output)

Roman.

Re: Exit code 130 kills pipeline

[Mikael Magnusson]

On 6/17/20, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> On Wed, Jun 17, 2020 at 7:42 PM Mikael Magnusson <mikachu@gmail.com> wrote:
>>
>> On 6/17/20, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
>> > Is it expected that the following command produces no output?
>> >
>> >   ( exit 130 ) | { sleep 1; echo hello }
>>
>> No?
>
> Good point. I should've specified the environment.
>
> % docker run -it --rm zshusers/zsh:5.8 \
>   zsh -c '( exit 130 ) | { sleep 1; echo hello }'
>
> (no output)
>
> Roman.

I get no output from
zsh -c '( exit 130 ) | { sleep 1; echo hello }'
but i do get output from
zsh -ic '( exit 130 ) | { sleep 1; echo hello }'
and again no output from
zsh -fic '( exit 130 ) | { sleep 1; echo hello }'

so it's probably something weird :).

-- 
Mikael Magnusson