TRAPCHLD and wait

TRAPCHLD and wait

[Bart Schaefer]

The manual says:

     It is possible to wait for recent processes (specified by process
     ID, not by job) that were running in the background even if the
     process has exited.  Typically the process ID will be recorded by
     capturing the value of the variable $! immediately after the
     process has been started.

This does work in the most obvious cases:

% { sleep 10; exit 23 } & bang=$!
[1] 560524
% sleep 20
[1]  + exit 23    { sleep 10; exit 23; }
% wait $bang
% print $?
23
%

However, you can't do that wait in a CHLD trap, even though it still
works after the trap has run:

% TRAPCHLD() {
  wait $bang
  q=$?
  zle -I
  print $q
  zle -R
}
% { sleep 10; exit 21 } & bang=$!
[1] 560661
%
[1]  + exit 21    { sleep 10; exit 21; }
%
TRAPCHLD:wait:1: pid 560661 is not a child of this shell
127
% wait $bang
% print $?
21
%

I suspect this has to do with order of operations in the signal
handlers but thought I'd ask for clues before digging in myself.

Re: TRAPCHLD and wait

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

On Sun, Dec 3, 2023 at 10:02 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> % TRAPCHLD() {
>   wait $bang
>   q=$?
>   zle -I
>   print $q
>   zle -R
> }
> %
> TRAPCHLD:wait:1: pid 560661 is not a child of this shell
> 127
>
> I suspect this has to do with order of operations in the signal
> handlers but thought I'd ask for clues before digging in myself.

I got impatient.  Anyone see a problem with the attached patch?
C03traps (and everything else) still passes.

[-- Attachment #2: trap-wait.txt --]
[-- Type: text/plain, Size: 1911 bytes --]

diff --git a/Src/jobs.c b/Src/jobs.c
index a3b9f667a..1bdf7a65c 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -660,6 +660,25 @@ update_job(Job jn)
     }
 }
 
+/**/
+void
+update_bg_job(Job jn, pid_t pid, int status)
+{
+    /*
+     * Accumulate a list of older jobs.  We only do this for
+     * background jobs, which is something in the job table
+     * that's not marked as in the current shell or as shell builtin
+     * and is not equal to the current foreground job.
+     */
+    if (jn && !(jn->stat & (STAT_CURSH|STAT_BUILTIN)) &&
+	jn - jobtab != thisjob) {
+	if (WIFEXITED(status))
+	    addbgstatus(pid, WEXITSTATUS(status));
+	else if (WIFSIGNALED(status))
+	    addbgstatus(pid, 0200 | WTERMSIG(status));
+    }
+}
+
 /* set the previous job to something reasonable */
 
 /**/
diff --git a/Src/signals.c b/Src/signals.c
index a61368554..b1a843e2c 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -556,9 +556,11 @@ wait_for_processes(void)
 		    jn->gleader = 0;
 		}
 	    }
+	    update_bg_job(jn, pid, status);
 	    update_job(jn);
 	} else if (findproc(pid, &jn, &pn, 1)) {
 	    pn->status = status;
+	    update_bg_job(jn, pid, status);
 	    update_job(jn);
 	} else {
 	    /* If not found, update the shell record of time spent by
@@ -567,19 +569,7 @@ wait_for_processes(void)
 	     * terminates.
 	     */
 	    get_usage();
-	}
-	/*
-	 * Accumulate a list of older jobs.  We only do this for
-	 * background jobs, which is something in the job table
-	 * that's not marked as in the current shell or as shell builtin
-	 * and is not equal to the current foreground job.
-	 */
-	if (jn && !(jn->stat & (STAT_CURSH|STAT_BUILTIN)) &&
-	    jn - jobtab != thisjob) {
-	    if (WIFEXITED(status))
-		addbgstatus(pid, WEXITSTATUS(status));
-	    else if (WIFSIGNALED(status))
-		addbgstatus(pid, 0200 | WTERMSIG(status));
+	    update_bg_job(jn, pid, status);
 	}
 
 	unqueue_signals();