Removing subshell from zargs (see "zargs with -P intermittently failing")

Removing subshell from zargs (see "zargs with -P intermittently failing")

[Bart Schaefer]

Got my old MacBook Air (High Sierra) fired up.

With a simple shell function "f" and only "option C" of Jun-ichi's
patch applied, I can get

% zargs -n 1 -P 19 -- {1..40} -- f
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once
1: jobs.c:2254: addbgstatus is called more than once

Removing the "wait" (without PID) from the zargs loop is confirmed to
make these go away.

As mentioned on the other thread, this should mean that the subshell
is also not needed, because the parent shell will wait for the
specific jobs spawned by zargs without blocking on other background
jobs.  I can indeed confirm that this is OK, by e.g.:

% sleep 60 &
[1] 58019
% zargs -n 1 -P 19 -- {1..40} -- f
% jobs
[1]  + running    sleep 60

However, I also intermittently get this:

% zargs -n 1 -P 19 -- {1..40} -- f
% jobs
[1]    running    sleep 60
[9]  + done       { "${call[@]}"; }

Or even this:

% zargs -n 1 -P 19 -- {1..40} -- f
%
[1]    done       sleep 60
% jobs
[9]  + done       { "${call[@]}"; }

I've seen [7],[8],[9] show up this way in a few test passes.  The
embedded "wait $j" (for PID j) does appear to have worked and returned
the correct status, so I don't know how significant this is.  Any
ideas?  It demonstrates that a job spawned by zargs can be considered
the "current" or "previous" job when the subshell is removed, which
might not be desirable.

Re: Removing subshell from zargs (see "zargs with -P intermittently failing")

[Jun. T]

Sorry I missed this post. And I fear I will not be able to going into
any detail at least for a few days.

> 2022/05/30 5:56, Bart Schaefer <schaefer@brasslantern.com> wrote:
> 
> However, I also intermittently get this:
> 
> % zargs -n 1 -P 19 -- {1..40} -- f
> % jobs
> [1]    running    sleep 60
> [9]  + done       { "${call[@]}"; }
> 
> Or even this:
> 
> % zargs -n 1 -P 19 -- {1..40} -- f
> %
> [1]    done       sleep 60
> % jobs
> [9]  + done       { "${call[@]}"; }

(1) On my Mac, it seems that this does not happen if I start zsh by
'zsh -f' (but I don't know why).

(2) It seems we don't need the extra background job 'sleep 60 &'
to get the problem.

(3) I tried the following dirty patch for debugging:

diff --git a/Src/jobs.c b/Src/jobs.c
index a91ef787f..6e59b5b71 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -2391,7 +2391,8 @@ bin_fg(char *name, char **argv, Options ops, int func)
 		curmaxjob = maxjob;
 		ignorejob = thisjob;
 	    }
-	    for (job = 0; job <= curmaxjob; job++, jobptr++)
+	    for (job = 0; job <= curmaxjob; job++, jobptr++) {
+		zwarn("%d %d", job, jobptr->stat);
 		if (job != ignorejob && jobptr->stat) {
 		    if ((!OPT_ISSET(ops,'r') && !OPT_ISSET(ops,'s')) ||
 			(OPT_ISSET(ops,'r') && OPT_ISSET(ops,'s')) ||
@@ -2400,6 +2401,7 @@ bin_fg(char *name, char **argv, Options ops, int func)
 			(OPT_ISSET(ops,'s') && jobptr->stat & STAT_STOPPED))
 			printjob(jobptr, lng, 2);
 		}
+	    }
 	    unqueue_signals();
 	    return 0;
 	} else {   /* Must be BIN_WAIT, so wait for all jobs */


When the problem occurs, I get:

% zargs -n 1 -P 19 -- {1..40} -- f; jobs
zsh: 0 0
zsh: 1 17504	# thisjob = ignorejob
zsh: 2 0
zsh: 3 0
zsh: 4 0
zsh: 5 0
zsh: 6 0
zsh: 7 0
zsh: 8 2137
[8]  + done       { "${call[@]}"; }

2137 = 0x859
= STAT_CHANGED | STAT_DONE | STAT_LOCKED | STAT_INUSE | STAT_NOSTTY

maxjob = 8 here, but it should have been decremented to 1?
The only place maxjob is decremented is in freejob(), I guess.

Re: Removing subshell from zargs (see "zargs with -P intermittently failing")

[Bart Schaefer]

On Mon, May 30, 2022 at 9:34 AM Jun. T <takimoto-j@kba.biglobe.ne.jp> wrote:
>
> Sorry I missed this post. And I fear I will not be able to going into
> any detail at least for a few days.

No worries.

> (3) I tried the following dirty patch for debugging:
>
[...]
>
> When the problem occurs, I get:
>
> % zargs -n 1 -P 19 -- {1..40} -- f; jobs
> zsh: 0 0
> zsh: 1 17504    # thisjob = ignorejob

Aha.  Yes, this is what the original subshell + wait was intended to
prevent.  I think what's happened here is that "waid $j" was looking
for the exit of (what is here shown as) job 0, but job 1 exited first,
and zsh had to handle but disregard that exit in order to continue
waiting for the job whose status was wanted.  Doing a full "wait"
beforehand guarantees that we can return the status of any job without
having to handle intervening signals, and theoretically doesn't slow
us down because we eventually have to wait for all the jobs anyway.

> zsh: 8 2137
> [8]  + done       { "${call[@]}"; }
>
> maxjob = 8 here, but it should have been decremented to 1?
> The only place maxjob is decremented is in freejob(), I guess.

Yes, once a job is in (in this case) job table slot 8, new jobs have
to use 9 and up even if the jobs in slots 0 through 7 have already
exited.  Once the current maxjob slot is reclaimed, all slots before
it that point to exited jobs can be reclaimed too.

The question is why slot 8 wasn't reclaimed until "jobs" ran.  Likely
related in some way to the order in which the jobs exited.

Harmless, I guess, but potentially confusing enough that the subshell
wrapper should be left in place?  Given the behavior above, it would
also appear possible to fill up the parent shell job table by having
maxjobs get "stuck" at increasingly larger slots.

  Or else it's related to this:

    # These setopts are necessary for "wait" on multiple jobs to work.
    setopt nonotify nomonitor

Those options are local to the zargs function, of course, but perhaps
they should actually be inside the subshell?

Re: Removing subshell from zargs (see "zargs with -P intermittently failing")

[Bart Schaefer]

On Mon, May 30, 2022 at 11:39 AM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> Harmless, I guess, but potentially confusing enough that the subshell
> wrapper should be left in place?

One additional issue -- if the subshell is removed and you interrupt
zargs -P, you get something like this:

^C%
%
[9]    done       { "${call[@]}"; }
%
[21]  - done       { "${call[@]}"; }
%
[10]    done       { "${call[@]}"; }
%
[20]  - done       { "${call[@]}"; }
%
[11]    done       { "${call[@]}"; }
%
[19]  - done       { "${call[@]}"; }
%
[12]    done       { "${call[@]}"; }
%
[18]  - done       { "${call[@]}"; }
%
[13]    done       { "${call[@]}"; }
%
[17]  - done       { "${call[@]}"; }
%
[14]    done       { "${call[@]}"; }
%
[16]  - done       { "${call[@]}"; }
%
[15]  - done       { "${call[@]}"; }

All things considered, I think using the subshell is in fact preferable.