Re: TRAPCHLD and wait

[Bart Schaefer <schaefer@brasslantern.com>]

[-- 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();