From b4f7ccecd93ca9e64c3c3c774fdaefae83d7204a Mon Sep 17 00:00:00 2001
From: Peter Stephenson
Date: Sun, 26 Oct 2014 17:47:42 +0000
Subject: 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.
---
ChangeLog | 9 ++++
Doc/Zsh/builtins.yo | 16 ++++++
Doc/Zsh/options.yo | 8 +--
Src/exec.c | 2 -
Src/init.c | 1 -
Src/jobs.c | 138 ++++++++++++++++++++++++++++++++++++++++++++--------
Src/linklist.c | 4 ++
Src/signals.c | 14 +++---
8 files changed, 157 insertions(+), 35 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 48efd1c0d..c75b870de 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2014-10-26 Peter Stephenson
+
+ * 33531 (plus fix to test job pointer and removing
+ lastpid_status): Doc/Zsh/builtins.yo, Doc/Zsh/options.yo,
+ Src/exec.c, Src/init.c, Src/jobs.c, Src/linklist.c,
+ Src/signals.c: retain up to CHILD_MAX statuses of exited
+ background processes; remove old lastpid_status mechanism;
+ slightly improve safety of permanently allocated linked lists.
+
2014-10-24 Barton E. Schaefer
* 33526: Completion/Unix/Type/_path_files: fix path prefix
diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 46f40cc3a..edc335e8b 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -2059,6 +2059,22 @@ then all currently active child processes are waited for.
Each var(job) can be either a job specification or the process ID
of a job in the job table.
The exit status from this command is that of the job waited for.
+
+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 tt($!) immediately after the process has been
+started. There is a limit on the number of process IDs remembered by
+the shell; this is given by the value of the system configuration
+parameter tt(CHILD_MAX). When this limit is reached, older process IDs
+are discarded, least recently started processes first.
+
+Note there is no protection against the process ID wrapping, i.e. if the
+wait is not executed soon enough there is a chance the process waited
+for is the wrong one. A conflict implies both process IDs have been
+generated by the shell, as other processes are not recorded, and that
+the user is potentially interested in both, so this problem is intrinsic
+to process IDs.
)
findex(whence)
item(tt(whence) [ tt(-vcwfpams) ] var(name) ...)(
diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 068a253ac..452b258b4 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -1434,10 +1434,10 @@ shell is saved for output within a subshell (for example, within a
pipeline). When the option is set, the output of tt(jobs) is empty
until a job is started within the subshell.
-When the option is set, it becomes possible to use the tt(wait) builtin to
-wait for the last job started in the background (as given by tt($!)) even
-if that job has already exited. This works even if the option is turned
-on temporarily around the use of the tt(wait) builtin.
+In previous versions of the shell, it was necessary to enable
+tt(POSIX_JOBS) in order for the builtin command tt(wait) to return the
+status of background jobs that had already exited. This is no longer
+the case.
)
enditem()
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);
}
}
--
cgit 1.4.1