[PATCH] zargs and signal handling

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

Whilst experimenting with something else, I discovered that "zargs -P"
would often abandon its backgrounded jobs when receiving
terminal-driven signals.  This led me down several rat-holes trying to
figure out what was going on.  In fact I'm still not sure what all the
reasons were or how many of them were related to "setopt nomonitor"
(which is removed in the attached patch).

I discovered for example that even when INT and QUIT were explicitly
trapped, a shell function passed as the command for zargs to run would
execute with those signals either reset to their defaults or simply
ignored, and I haven't been able to work out the circumstances when
each of those cases occurs.  E.g., with

f() { sleep $1; print $1 }
zargs -n 1 -P 3 -- 2 3 4 5 6 -- f

Interrupting zargs with ^C would leave all the background "f" running,
waiting for "sleep".  On the other hand, with

zargs -n 1 -P 3 -- 2 3 4 5 6 -- /bin/sleep

the ^C terminated the sleeps as expected but attempting to stop zargs
with ^Z would cause zargs to return, leaving orphan stopped sleep
processes behind.  That part was due to "nomonitor".

Here then is a patch which:
-- as discussed in the "Removing subshell from zargs" thread, removes
the preliminary "wait" for all the process
-- as noted above, removes "nomonitor" (because it was only needed for
that "wait")
-- explicitly adds traps to exit for tty-generated signals plus TERM,
except that it ...
-- captures the global signal trap context and restores it in the
background jobs
-- wraps the whole thing in an "always" block to clean up local helper functions
-- updates the comments to note another buglet and drop support for zsh 4.x.

As further speculated in the other thread, this doesn't yet remove the
subshell around the whole mess.

I'd love to hear explanations of why the signal handling behaves as it
does.  Every time I thought I had the logic mapped onto subshell entry
conditions or whatever, it'd behave a different way the next time I
tried, until I ended up with the rather brute-force approach in the
patch.

diff --git a/Functions/Misc/zargs b/Functions/Misc/zargs
index 81916a3ac..782d6811e 100644
--- a/Functions/Misc/zargs
+++ b/Functions/Misc/zargs
@@ -39,9 +39,14 @@
 #
 # "Killed by a signal" is determined by the usual shell rule that $? is
 # the signal number plus 128, so zargs can be fooled by a command that
-# explicitly exits with 129+.  Also, zsh prior to 4.1.x returns 1 rather
-# than 127 for "command not found" so this function incorrectly returns
-# 123 in that case if used with zsh 4.0.x.
+# explicitly exits with 129+.  If the command passed to zargs is a shell
+# function which uses "exit" instead of "return", zsh interprets 129+ as
+# a signal sent to the process group and may terminate zargs with that
+# status.  This is avoided when running zargs -P 2 or greater.
+#
+# ZARGS_VERSION 1.5 is the last to support zsh 4.x.  Also, zsh prior to
+# 4.1.x returns 1 rather than 127 for "command not found" so zargs
+# incorrectly returned 123 in that case if used with zsh 4.0.x.
 #
 # Because of "wait" limitations, --max-procs spawns max-procs jobs, then
 # waits for all of those, then spawns another batch, etc.
@@ -71,6 +76,10 @@
 # * The use of SIGUSR1 and SIGUSR2 to change the number of parallel jobs
 #   is not supported.
 
+{ # Begin "always" block to reset locally defined functions
+
+local ZARGS_VERSION="1.8"
+
 # First, capture the current setopts as "sticky emulation"
 if zmodload zsh/parameter
 then
@@ -84,11 +93,20 @@ else
   emulate $(emulate -l) -c '_zarun() { eval "$@" }'
 fi
 
+local _zaTRAPS="$(trap)"
+_zatraps() {
+  # In children, these traps may be reset to default behavior, even
+  # if the calling shell has traps.  Restore to surrounding context,
+  # but assure that if zargs itself is signaled, children will exit.
+  [[ -o interactive ]] &&
+    function TRAP{HUP,INT,QUIT,TERM} { exit $((128 + $1)) }
+  [[ -n "$_zaTRAPS" ]] && eval "$_zaTRAPS"
+  unset _zaTRAPS
+}
+
 emulate -L zsh || return 1
 local -a opts eof n s l P i
 
-local ZARGS_VERSION="1.7"
-
 if zparseopts -a opts -D -- \
 	-eof::=eof e::=eof \
 	-exit x \
@@ -193,14 +211,14 @@ then (( c = $#command - 1 ))
 else command=( print -r -- )
 fi
 
-local wait bg
-local execute='
+local bg execute='
     if (( $opts[(I)-(-interactive|p)] ))
     then read -q "?$call?..." || continue
     elif (( $opts[(I)-(-verbose|t)] ))
     then print -u2 -r -- "$call"
     fi
     _zarun "{
+	_zatraps
 	\"\${call[@]}\"
     } $bg"'
 local ret=0 analyze='
@@ -263,12 +281,10 @@ fi
 
 if (( P != 1 && ARGC > 1 ))
 then
-    # These setopts are necessary for "wait" on multiple jobs to work.
-    setopt nonotify nomonitor
+    setopt nonotify	# Do not report each exiting job
     local -a _zajobs
     local j
     bg='& _zajobs+=( $! )'
-    wait='wait'
     analyze='
     for j in $_zajobs; do
       wait $j
@@ -316,4 +332,8 @@ return $ret
 
 )
 
+} always {
+  builtin unfunction _zarun _zatraps
+}
+
 # }