[BUG?] "wait <PID>" sometimes confused with multiple childs

[BUG?] "wait <PID>" sometimes confused with multiple childs

[Antoine C.]

Hello,

I have a problem with a script starting several subshells in background then checking their return code. It looks like the "wait <PID>" builtin is sometimes returning the error code of another child.

Here is a script reproducing this behavior. Several childs are started in a first loop with predefined return code, their PID saved, then we wait for each of them in a second loop and check their RC:
------start-----
#!/bin/zsh

while true; do
	pids=()
	for task in {1..16} ; do
		( sleep .1 ; exit $task ) &
		pids+=($!)
		# uncomment to prevent bug
		#sleep .1
	done
	for task in {1..16} ; do
		wait $pids[task]
		rc=$?
		if (( rc != task )) ; then
			echo "  FAIL $pids[task]" $rc $task
		fi
	done
	echo "done $pids"
done
------end-----

On my machine, I have many "FAIL ..." printed, around 1 "FAIL" every 2 "done".
If we comment out the "sleep" above, no more fails are printed. I also tried with bash (with a few syntax changes), and it is always working as expected.

Is it a bug or am I missing something?

Antoine

Re: [BUG?] "wait <PID>" sometimes confused with multiple childs

[Peter Stephenson]

On Mon, 12 Mar 2018 14:23:59 +0100
Antoine C. <acalando@free.fr> wrote:
> I have a problem with a script starting several subshells in
> background then checking their return code. It looks like the "wait
> <PID>" builtin is sometimes returning the error code of another child.
>
> Is it a bug or am I missing something?

It's a bug.  You hit a race where we record the status but don't
read it back properly.

Thanks, I had no problem investigating with that script.

pws

diff --git a/Src/jobs.c b/Src/jobs.c
index 503618f..330ee6b 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -2289,8 +2289,11 @@ bin_fg(char *name, char **argv, Options ops, int func)
 		     */
 		    retval = waitforpid(pid, 1);
 		}
-		if (retval == 0)
-		    retval = lastval2;
+		if (retval == 0) {
+		    if ((retval = getbgstatus(pid)) < 0) {
+			retval = lastval2;
+		    }
+		}
 	    } 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? */