[zsh] make the wait built-in work for already exited processes (RHBZ#768548)
Kamil Dudka
kdudka at fedoraproject.org
Mon Nov 10 14:48:44 UTC 2014
commit 7e97ffec17ed7ffed7319945cb909e67ebb01ba0
Author: Kamil Dudka <kdudka at redhat.com>
Date: Mon Nov 10 15:38:40 2014 +0100
make the wait built-in work for already exited processes (RHBZ#768548)
zsh-5.0.7-wait-for-exitted.patch | 377 ++++++++++++++++++++++++++++++++++++++
zsh.spec | 14 ++-
2 files changed, 389 insertions(+), 2 deletions(-)
---
diff --git a/zsh-5.0.7-wait-for-exitted.patch b/zsh-5.0.7-wait-for-exitted.patch
new file mode 100644
index 0000000..e92993f
--- /dev/null
+++ b/zsh-5.0.7-wait-for-exitted.patch
@@ -0,0 +1,377 @@
+From 223ac53797d33b0473323efc0d5a44d1dceaf746 Mon Sep 17 00:00:00 2001
+From: Peter Stephenson <p.w.stephenson at ntlworld.com>
+Date: Sun, 26 Oct 2014 17:47:42 +0000
+Subject: [PATCH 1/2] 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.
+
+Upstream-commit: b4f7ccecd93ca9e64c3c3c774fdaefae83d7204a
+Signed-off-by: Kamil Dudka <kdudka at redhat.com>
+---
+ 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 +++---
+ 7 files changed, 148 insertions(+), 35 deletions(-)
+
+diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
+index 46f40cc..edc335e 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 068a253..452b258 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 d0fadd6..a9c4688 100644
+--- a/Src/exec.c
++++ b/Src/exec.c
+@@ -2941,8 +2941,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 c26d887..6666f98 100644
+--- a/Src/init.c
++++ b/Src/init.c
+@@ -1036,7 +1036,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 bd95afb..18bb648 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 1e364fb..3aa8125 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 2df69f9..e728505 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);
+ }
+ }
+
+--
+2.1.0
+
+
+From 2d59469450ba80b69449dc2777f0fc0673e0fbd6 Mon Sep 17 00:00:00 2001
+From: Peter Stephenson <p.w.stephenson at ntlworld.com>
+Date: Sun, 26 Oct 2014 19:04:47 +0000
+Subject: [PATCH 2/2] 33542: test logic for waiting for already exited
+ processes
+
+Upstream-commit: 9a551ca85999ff329714fd2cca138ce2f7d3c3d9
+Signed-off-by: Kamil Dudka <kdudka at redhat.com>
+---
+ Test/A05execution.ztst | 29 +++++++++++++++++++++++++++--
+ 1 file changed, 27 insertions(+), 2 deletions(-)
+
+diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst
+index ca97f4f..589815f 100644
+--- a/Test/A05execution.ztst
++++ b/Test/A05execution.ztst
+@@ -190,9 +190,9 @@
+ print "${pipestatus[@]}")
+ ZTST_hashmark
+ done | sort | uniq -c | sed 's/^ *//'
+-0:Check whether `$pipestatus[]' behaves.
++0:Check whether '$pipestatus[]' behaves.
+ >2048 2 1 0
+-F:This test checks for a bug in `$pipestatus[]' handling. If it breaks then
++F:This test checks for a bug in '$pipestatus[]' handling. If it breaks then
+ F:the bug is still there or it reappeared. See workers-29973 for details.
+
+ { setopt MONITOR } 2>/dev/null
+@@ -244,3 +244,28 @@ F:anonymous function, and a descriptor leak when backgrounding a pipeline
+ >autoload_redir () {
+ > print Autoloaded ksh style
+ >} > autoload.log
++
++# This tests that we record the status of processes that have already exited
++# for when we wait for them.
++#
++# Actually, we don't guarantee here that the jobs have already exited, but
++# the order of the waits means it's highly likely we do need to recall a
++# previous status, barring accidents which shouldn't happen very often. In
++# other words, we rely on the test working repeatedly rather than just
++# once. The monitor option is irrelevant to the logic, so we'll make
++# our job easier by turning it off.
++ unsetopt monitor
++ (exit 1) &
++ one=$!
++ (exit 2) &
++ two=$!
++ (exit 3) &
++ three=$!
++ wait $three
++ print $?
++ wait $two
++ print $?
++ wait $one
++1:The status of recently exited background jobs is recorded
++>3
++>2
+--
+2.1.0
+
diff --git a/zsh.spec b/zsh.spec
index ecda18a..ec41af9 100644
--- a/zsh.spec
+++ b/zsh.spec
@@ -3,7 +3,7 @@
Summary: Powerful interactive shell
Name: zsh
Version: 5.0.7
-Release: 1%{?dist}
+Release: 2%{?dist}
License: MIT
URL: http://zsh.sourceforge.net/
Group: System Environment/Shells
@@ -27,7 +27,13 @@ Patch0: zsh-serial.patch
Patch4: zsh-4.3.6-8bit-prompts.patch
Patch5: zsh-test-C02-dev_fd-mock.patch
-Patch12: http://ausil.fedorapeople.org/aarch64/zsh/zsh-aarch64.patch
+
+# make the wait built-in work for already exited processes (#1162198)
+Patch6: zsh-5.0.7-wait-for-exitted.patch
+
+# XXX: useless because %%configure will overwrite the patched config.{guess,sub}
+Patch12: zsh-aarch64.patch
+
BuildRequires: coreutils sed ncurses-devel libcap-devel
BuildRequires: texinfo texi2html gawk hostname
Requires(post): info grep
@@ -66,6 +72,7 @@ This package contains the Zsh manual in html format.
#%patch2 -p1
%patch4 -p1
%patch5 -p1
+%patch6 -p1
%patch12 -p1
@@ -185,6 +192,9 @@ fi
%doc Doc/*.html
%changelog
+* Mon Nov 10 2014 Kamil Dudka <kdudka at redhat.com> - 5.0.7-2
+- make the wait built-in work for already exited processes (#1162198)
+
* Wed Oct 08 2014 Dominic Hopf <dmaphy at fedoraproject.org> - 5.0.7-1
- Update to latest upstream release: Zsh 5.0.7
More information about the scm-commits
mailing list