From 547d772d4115cece72c8ea4ca9c6b438135c3599 Mon Sep 17 00:00:00 2001 From: martin Date: Wed, 28 Apr 2021 09:58:42 +0000 Subject: [PATCH] Pull up following revision(s) (requested by kre in ticket #1259): bin/sh/jobs.h: revision 1.24 bin/sh/eval.c: revision 1.182 bin/sh/jobs.c: revision 1.110 Related to PR bin/48875 Correct an issue found by Oguz and reported in e-mail (on the bug-bash list initially!) with the code changed to deal with PR bin/48875 With: sh -c 'echo start at $SECONDS; (sleep 3 & (sleep 1& wait) ); echo end at $SECONDS' The shell should say "start at 0\nend at 1\n", but instead (before this fix, in -9 and HEAD, but not -8) does "start at 0\nend at 3\n" (Not in -8 as the 48875 changes were never pulled up)> There was an old problem, fixed years ago, which cause the same symptom, related to the way the jobs table was cleared (or not) in subshells, and it seemed like that might have resurfaced. But not so, the issue here is the sub-shell elimination, which was part of the 48875 "fix" (not really, it wasn't really a bug, just sub-optimal and unexpected behaviour). What the shell actually has been running in this case is: sh -c 'echo start at $SECONDS; (sleep 3 & sleep 1& wait ); echo end at $SECONDS' as the inner subshell was deemed unnecessary - all its parent would do is wait for its exit status, and then exit with that status - we may as well simply replace the current sub-shell with the new one, let it do its thing, and we're done... But not here, the running "sleep 3" will remain a child of that merged sub-shell, and the "wait" will thus wait for it, along with the sleep 1 which is all it should be seeing. For now, fix this by not eliminating a sub-shell if there are existing unwaited upon children in the current one. It might be possible to simply disregard the old child for the purposes of wait (and "jobs", etc, all cmds which look at the jobs table) but the bookkeeping required to make that work reliably is likely to take some time to get correct... Along with this fix comes a fix to DEBUG mode shells, which, in situations like this, could dump core in the debug code if the relevant tracing was enabled, and add a new trace for when the jobs table is cleared (which was added predating the discovery of the actual cause of this issue, but seems worth keeping.) Neither of these changes have any effect on shells compiled normally. XXX pullup -9 --- bin/sh/eval.c | 6 +++--- bin/sh/jobs.c | 34 +++++++++++++++++++++++++++++++--- bin/sh/jobs.h | 3 ++- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/bin/sh/eval.c b/bin/sh/eval.c index 1a227c75eaf1..f250893e150d 100644 --- a/bin/sh/eval.c +++ b/bin/sh/eval.c @@ -1,4 +1,4 @@ -/* $NetBSD: eval.c,v 1.175.2.2 2019/12/26 20:16:47 martin Exp $ */ +/* $NetBSD: eval.c,v 1.175.2.3 2021/04/28 09:58:42 martin Exp $ */ /*- * Copyright (c) 1993 @@ -37,7 +37,7 @@ #if 0 static char sccsid[] = "@(#)eval.c 8.9 (Berkeley) 6/8/95"; #else -__RCSID("$NetBSD: eval.c,v 1.175.2.2 2019/12/26 20:16:47 martin Exp $"); +__RCSID("$NetBSD: eval.c,v 1.175.2.3 2021/04/28 09:58:42 martin Exp $"); #endif #endif /* not lint */ @@ -548,7 +548,7 @@ evalsubshell(union node *n, int flags) flushout(outx); } INTOFF; - if ((!backgnd && flags & EV_EXIT && !have_traps()) || + if ((!backgnd && flags & EV_EXIT && !have_traps() && !anyjobs()) || forkshell(jp = makejob(n, 1), n, backgnd?FORK_BG:FORK_FG) == 0) { if (backgnd) flags &=~ EV_TESTED; diff --git a/bin/sh/jobs.c b/bin/sh/jobs.c index 4b367d04c4dc..5cffe3fc9ef1 100644 --- a/bin/sh/jobs.c +++ b/bin/sh/jobs.c @@ -1,4 +1,4 @@ -/* $NetBSD: jobs.c,v 1.106.2.1 2020/02/10 18:54:14 martin Exp $ */ +/* $NetBSD: jobs.c,v 1.106.2.2 2021/04/28 09:58:42 martin Exp $ */ /*- * Copyright (c) 1991, 1993 @@ -37,7 +37,7 @@ #if 0 static char sccsid[] = "@(#)jobs.c 8.5 (Berkeley) 5/4/95"; #else -__RCSID("$NetBSD: jobs.c,v 1.106.2.1 2020/02/10 18:54:14 martin Exp $"); +__RCSID("$NetBSD: jobs.c,v 1.106.2.2 2021/04/28 09:58:42 martin Exp $"); #endif #endif /* not lint */ @@ -768,7 +768,7 @@ waitcmd(int argc, char **argv) VTRACE(DBG_WAIT, ("wait %s%s%sfound %d candidates (last %s)\n", any ? "-n " : "", *argptr ? *argptr : "", argptr[0] && argptr[1] ? "... " : " ", found, - job ? (job->ref ? job->ref : "") : "none")); + job && job->used ? (job->ref ? job->ref : "") : "none")); /* * If we were given a list of jobnums: @@ -1033,6 +1033,32 @@ getjob(const char *name, int noerror) } +/* + * Find out if there are any running (that is, unwaited upon) + * background children of the current shell. + * + * Return 1/0 (yes, no). + * + * Needed as we cannot optimise away sub-shell creation if + * we have such a child, or a "wait" in that sub-shell would + * observe the already existing job. + */ +int +anyjobs(void) +{ + struct job *jp; + int i; + + if (jobs_invalid) + return 0; + + for (i = njobs, jp = jobtab ; --i >= 0 ; jp++) { + if (jp->used) + return 1; + } + + return 0; +} /* * Return a new job structure, @@ -1045,6 +1071,8 @@ makejob(union node *node, int nprocs) struct job *jp; if (jobs_invalid) { + VTRACE(DBG_JOBS, ("makejob(%p, %d) clearing jobtab (%d)\n", + (void *)node, nprocs, njobs)); for (i = njobs, jp = jobtab ; --i >= 0 ; jp++) { if (jp->used) freejob(jp); diff --git a/bin/sh/jobs.h b/bin/sh/jobs.h index a587e9e689b1..c8bdf19ba5c4 100644 --- a/bin/sh/jobs.h +++ b/bin/sh/jobs.h @@ -1,4 +1,4 @@ -/* $NetBSD: jobs.h,v 1.23 2018/09/11 03:30:40 kre Exp $ */ +/* $NetBSD: jobs.h,v 1.23.2.1 2021/04/28 09:58:42 martin Exp $ */ /*- * Copyright (c) 1991, 1993 @@ -99,6 +99,7 @@ int waitforjob(struct job *); int stoppedjobs(void); void commandtext(struct procstat *, union node *); int getjobpgrp(const char *); +int anyjobs(void); #if ! JOBS #define setjobctl(on) /* do nothing */