33531 with additions: retain status of exited background jobs.

Add linked list of unwaited-for background jobs.
Truncate at value of _SC_CHILD_MAX discarding oldest.
Remove old lastpid_status mechanism for latest exited process only.
Slightly tighten safety of permanently allocated linked lists so
that this doesn't compromise signal handling.

5 files changed, 128 insertions, 31 deletions

diff --git a/Src/exec.c b/Src/exec.c
index 2f896d8d5..5bbd4e15d 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2942,8 +2942,6 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	    close(synch[0]);
 	    if (how & Z_ASYNC) {
 		lastpid = (zlong) pid;
-		/* indicate it's possible to set status for lastpid */
-		lastpid_status = -2L;
 	    } else if (!jobtab[thisjob].stty_in_env && varspc) {
 		/* search for STTY=... */
 		Wordcode p = varspc;
diff --git a/Src/init.c b/Src/init.c
index 68d36125e..655166135 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1049,7 +1049,6 @@ setupvals(void)
     bufstack = znewlinklist();
     hsubl = hsubr = NULL;
     lastpid = 0;
-    lastpid_status = -1L;
 
     get_usage();
 
diff --git a/Src/jobs.c b/Src/jobs.c
index bd95afb7a..18bb648d9 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -104,15 +104,6 @@ int prev_errflag, prev_breaks, errbrk_saved;
 /**/
 int numpipestats, pipestats[MAX_PIPESTATS];
 
-/*
- * The status associated with the process lastpid.
- * -1 if not set and no associated lastpid
- * -2 if lastpid is set and status isn't yet
- * else the value returned by wait().
- */
-/**/
-long lastpid_status;
-
 /* Diff two timevals for elapsed-time computations */
 
 /**/
@@ -1309,14 +1300,6 @@ addproc(pid_t pid, char *text, int aux, struct timeval *bgtime)
 {
     Process pn, *pnlist;
 
-    if (pid == lastpid && lastpid_status != -2L) {
-	/*
-	 * The status for the previous lastpid is invalid.
-	 * Presumably process numbers have wrapped.
-	 */
-	lastpid_status = -1L;
-    }
-
     DPUTS(thisjob == -1, "No valid job in addproc.");
     pn = (Process) zshcalloc(sizeof *pn);
     pn->pid = pid;
@@ -1940,6 +1923,122 @@ maybeshrinkjobtab(void)
     unqueue_signals();
 }
 
+/*
+ * Definitions for the background process stuff recorded below.
+ * This would be more efficient as a hash, but
+ * - that's quite heavyweight for something not needed very often
+ * - we need some kind of ordering as POSIX allows us to limit
+ *   the size of the list to the value of _SC_CHILD_MAX and clearly
+ *   we want to clear the oldest first
+ * - cases with a long list of background jobs where the user doesn't
+ *   wait for a large number, and then does wait for one (the only
+ *   inefficient case) are rare
+ * - in the context of waiting for an external process, looping
+ *   over a list isn't so very inefficient.
+ * Enough excuses already.
+ */
+
+/* Data in the link list, a key (process ID) / value (exit status) pair. */
+struct bgstatus {
+    pid_t pid;
+    int status;
+};
+typedef struct bgstatus *Bgstatus;
+/* The list of those entries */
+LinkList bgstatus_list;
+/* Count of entries.  Reaches value of _SC_CHILD_MAX and stops. */
+long bgstatus_count;
+
+/*
+ * Remove and free a bgstatus entry.
+ */
+static void rembgstatus(LinkNode node)
+{
+    zfree(remnode(bgstatus_list, node), sizeof(struct bgstatus));
+    bgstatus_count--;
+}
+
+/*
+ * Record the status of a background process that exited so we
+ * can execute the builtin wait for it.
+ *
+ * We can't execute the wait builtin for something that exited in the
+ * foreground as it's not visible to the user, so don't bother recording.
+ */
+
+/**/
+void
+addbgstatus(pid_t pid, int status)
+{
+    static long child_max;
+    Bgstatus bgstatus_entry;
+
+    if (!child_max) {
+#ifdef _SC_CHILD_MAX
+	child_max = sysconf(_SC_CHILD_MAX);
+	if (!child_max) /* paranoia */
+#endif
+	{
+	    /* Be inventive */
+	    child_max = 1024L;
+	}
+    }
+
+    if (!bgstatus_list) {
+	bgstatus_list = znewlinklist();
+	/*
+	 * We're not always robust about memory failures, but
+	 * this is pretty deep in the shell basics to be failing owing
+	 * to memory, and a failure to wait is reported loudly, so test
+	 * and fail silently here.
+	 */
+	if (!bgstatus_list)
+	    return;
+    }
+    if (bgstatus_count == child_max) {
+	/* Overflow.  List is in order, remove first */
+	rembgstatus(firstnode(bgstatus_list));
+    }
+    bgstatus_entry = (Bgstatus)zalloc(sizeof(*bgstatus_entry));
+    if (!bgstatus_entry) {
+	/* See note above */
+	return;
+    }
+    bgstatus_entry->pid = pid;
+    bgstatus_entry->status = status;
+    if (!zaddlinknode(bgstatus_list, bgstatus_entry)) {
+	zfree(bgstatus_entry, sizeof(*bgstatus_entry));
+	return;
+    }
+    bgstatus_count++;
+}
+
+/*
+ * See if pid has a recorded exit status.
+ * Note we make no guarantee that the PIDs haven't wrapped, so this
+ * may not be the right process.
+ *
+ * This is only used by wait, which must only work on each
+ * pid once, so we need to remove the entry if we find it.
+ */
+
+static int getbgstatus(pid_t pid)
+{
+    LinkNode node;
+    Bgstatus bgstatus_entry;
+
+    if (!bgstatus_list)
+	return -1;
+    for (node = firstnode(bgstatus_list); node; incnode(node)) {
+	bgstatus_entry = (Bgstatus)getdata(node);
+	if (bgstatus_entry->pid == pid) {
+	    int status = bgstatus_entry->status;
+	    rembgstatus(node);
+	    return status;
+	}
+    }
+    return -1;
+}
 
 /* bg, disown, fg, jobs, wait: most of the job control commands are     *
  * here.  They all take the same type of argument.  Exception: wait can *
@@ -2085,10 +2184,7 @@ bin_fg(char *name, char **argv, Options ops, int func)
 		}
 		if (retval == 0)
 		    retval = lastval2;
-	    } else if (isset(POSIXJOBS) &&
-		       pid == lastpid && lastpid_status >= 0L) {
-		retval = (int)lastpid_status;
-	    } else {
+	    } else if ((retval = getbgstatus(pid)) < 0) {
 		zwarnnam(name, "pid %d is not a child of this shell", pid);
 		/* presumably lastval2 doesn't tell us a heck of a lot? */
 		retval = 1;
diff --git a/Src/linklist.c b/Src/linklist.c
index 1e364fb4e..3aa8125d9 100644
--- a/Src/linklist.c
+++ b/Src/linklist.c
@@ -118,6 +118,8 @@ znewlinklist(void)
     LinkList list;
 
     list = (LinkList) zalloc(sizeof *list);
+    if (!list)
+	return NULL;
     list->list.first = NULL;
     list->list.last = &list->node;
     list->list.flags = 0;
@@ -152,6 +154,8 @@ zinsertlinknode(LinkList list, LinkNode node, void *dat)
 
     tmp = node->next;
     node->next = new = (LinkNode) zalloc(sizeof *tmp);
+    if (!new)
+	return NULL;
     new->prev = node;
     new->dat = dat;
     new->next = tmp;
diff --git a/Src/signals.c b/Src/signals.c
index 2df69f96e..e72850516 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -522,14 +522,14 @@ wait_for_processes(void)
 	    get_usage();
 	}
 	/*
-	 * Remember the status associated with $!, so we can
-	 * wait for it even if it's exited.  This value is
-	 * only used if we can't find the PID in the job table,
-	 * so it doesn't matter that the value we save here isn't
-	 * useful until the process has exited.
+	 * 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 (pn != NULL && pid == lastpid && lastpid_status != -1L)
-	    lastpid_status = lastval2;
+	if (jn && !(jn->stat & (STAT_CURSH|STAT_BUILTIN)) &&
+	    jn - jobtab != thisjob)
+	    addbgstatus(pid, (int)lastval2);
     }
 }