From: Dominique Martinet <[email protected]>
'wait -n' had (at least) a couple of bugs:
1/ Waiting for jobs with non-zero status
if waiting for a command that returns non-zero, dowait() would keep
waiting for the next command.
That means something like the following waits 2 seconds instead of 1:
time busybox sh -c 'sh -c "sleep 1; false" & sleep 2 & wait -n'
2/ Waiting for jobs with multiple processes
While fixing the above, I noticed this comment:
/* wait -n waits for one _job_, not one _process_.
* date; sleep 3 & sleep 2 | sleep 1 & wait -n; date
* should wait for 2 seconds. Not 1 or 3.
*/
It turns out, that does not work:
time busybox sh -c 'sleep 3 & sleep 2 | sleep 1 & wait -n'
The current code returns -1 if a job is not finished, and dowait() stops
looping immediately on -1 since commit 8d5f465a206e ("ash: jobs: Fix
infinite loop in waitproc"), breaking this behaviour.
A test was added testing both of these cases, outputing the following
errors before patch:
```
Test1
Test2
Background1: BUG!
Test2 exit code was not 1
Test3
Test3 exit code was not 0
Test3 finished in less than 2s
sh: can't kill pid 2074018: No such process
Ok:1
```
This patch splits dowait() into two functions because the variant that
wants the pid is different enough to benefit from specialization.
function old new delta
waitone - 449 +449
static.dowait - 80 +80
waitcmd 277 334 +57
dowait 608 - -608
------------------------------------------------------------------------------
(add/remove: 2/1 grow/shrink: 1/0 up/down: 586/-608) Total: -22 bytes
text data bss dec hex filename
785950 14292 1800 802042 c3cfa busybox_old
785928 14292 1800 802020 c3ce4 busybox_unstripped
Fixes: 8d5f465a206e ("ash: jobs: Fix infinite loop in waitproc")
Tested-by: Wolfgang Zekoll <[email protected]>
Signed-off-by: Dominique Martinet <[email protected]>
---
v1 -> v2:
- apply Wolfgang's suggestion for test wording, add his tested-by
Config.in | 7 ----
shell/ash.c | 59 +++++++++++++++--------------
shell/ash_test/ash-misc/wait8.right | 4 ++
shell/ash_test/ash-misc/wait8.tests | 47 +++++++++++++++++++++++
4 files changed, 81 insertions(+), 36 deletions(-)
create mode 100644 shell/ash_test/ash-misc/wait8.right
create mode 100755 shell/ash_test/ash-misc/wait8.tests
diff --git a/Config.in b/Config.in
index 6b925b517dd5..ad0cd1e26f01 100644
--- a/Config.in
+++ b/Config.in
@@ -204,13 +204,6 @@ config FEATURE_INSTALLER
busybox at runtime to create hard links or symlinks for all the
applets that are compiled into busybox.
-config FEATURE_VERSION
- bool "Support --version"
- default y
- depends on BUSYBOX
- help
- Enable 'busybox --version' support.
-
config INSTALL_NO_USR
bool "Don't use /usr"
default n
diff --git a/shell/ash.c b/shell/ash.c
index 841ffe8808fc..4f5ca0d02fb7 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -4418,9 +4418,6 @@ sprint_status48(char *os, int status, int sigonly)
#define DOWAIT_NONBLOCK 0 /* waitpid() will use WNOHANG and won't wait
for signals */
#define DOWAIT_BLOCK 1 /* waitpid() will NOT use WNOHANG */
#define DOWAIT_CHILD_OR_SIG 2 /* waitpid() will use WNOHANG and if got 0,
will wait for signals, then loop back */
-#if BASH_WAIT_N
-# define DOWAIT_JOBSTATUS 0x10 /* OR this to get job's exitstatus instead of
pid */
-#endif
static int
waitproc(int block, int *status)
@@ -4472,10 +4469,6 @@ static int waitone(int block, struct job *job)
int status;
struct job *jp;
struct job *thisjob = NULL;
-#if BASH_WAIT_N
- bool want_jobexitstatus = (block & DOWAIT_JOBSTATUS);
- block = (block & ~DOWAIT_JOBSTATUS);
-#endif
TRACE(("dowait(0x%x) called\n", block));
@@ -4501,7 +4494,7 @@ static int waitone(int block, struct job *job)
pid = waitproc(block, &status);
TRACE(("wait returns pid %d, status=%d\n", pid, status));
if (pid <= 0)
- goto out;
+ return pid;
for (jp = curjob; jp; jp = jp->prev_job) {
int jobstate;
@@ -4554,13 +4547,6 @@ static int waitone(int block, struct job *job)
out:
INTON;
-#if BASH_WAIT_N
- if (want_jobexitstatus) {
- pid = -1;
- if (thisjob && thisjob->state == JOBDONE)
- pid = thisjob->ps[thisjob->nprocs - 1].ps_status;
- }
-#endif
if (thisjob && thisjob == job) {
char s[48 + 1];
int len;
@@ -4572,32 +4558,47 @@ static int waitone(int block, struct job *job)
out2str(s);
}
}
- return pid;
+
+ status = INT_MAX;
+ if (thisjob && thisjob->state == JOBDONE)
+ status = thisjob->ps[thisjob->nprocs - 1].ps_status;
+
+ return status;
}
-static int dowait(int block, struct job *jp)
+static int dowait_status(void)
+{
+ int status, rstatus = -ENOENT;
+ int block = DOWAIT_CHILD_OR_SIG;
+
+ do {
+ status = waitone(block, NULL);
+ if (status >= 0 && status < INT_MAX) {
+ rstatus = status;
+ block = DOWAIT_NONBLOCK;
+ }
+ } while (status >= 0);
+
+ return rstatus;
+}
+
+static void dowait(int block, struct job *jp)
{
smallint gotchld = *(volatile smallint *)&gotsigchld;
- int rpid;
- int pid;
+ int status;
if (jp && jp->state != JOBRUNNING)
block = DOWAIT_NONBLOCK;
if (block == DOWAIT_NONBLOCK && !gotchld)
- return 1;
-
- rpid = 1;
+ return;
do {
- pid = waitone(block, jp);
- rpid &= !!pid;
+ status = waitone(block, jp);
- if (!pid || (jp && jp->state != JOBRUNNING))
+ if (!status || (jp && jp->state != JOBRUNNING))
block = DOWAIT_NONBLOCK;
- } while (pid >= 0);
-
- return rpid;
+ } while (status >= 0);
}
#if JOBS
@@ -4801,7 +4802,7 @@ waitcmd(int argc UNUSED_PARAM, char **argv)
* the trap is executed."
*/
#if BASH_WAIT_N
- status = dowait(DOWAIT_CHILD_OR_SIG | DOWAIT_JOBSTATUS,
NULL);
+ status = dowait_status();
#else
dowait(DOWAIT_CHILD_OR_SIG, NULL);
#endif
diff --git a/shell/ash_test/ash-misc/wait8.right
b/shell/ash_test/ash-misc/wait8.right
new file mode 100644
index 000000000000..5d29b8cc7dbf
--- /dev/null
+++ b/shell/ash_test/ash-misc/wait8.right
@@ -0,0 +1,4 @@
+Test1 (wait with zero exit status of bg process)
+Test2 (wait with non-zero exit status of bg process)
+Test3 (pipeline wait for whole job)
+Ok:0
diff --git a/shell/ash_test/ash-misc/wait8.tests
b/shell/ash_test/ash-misc/wait8.tests
new file mode 100755
index 000000000000..d29490260001
--- /dev/null
+++ b/shell/ash_test/ash-misc/wait8.tests
@@ -0,0 +1,47 @@
+# wait -n tests
+
+# should never wait for this one
+sleep 5 && echo "Background1: BUG!" &
+bg=$!
+
+# normal process
+echo "Test1 (wait with zero exit status of bg process)"
+sleep 1 &
+wait -n
+rc=$?
+if [ "$rc" != 0 ]; then
+ echo "Test1 exit code was not 0: $rc"
+fi
+
+# non-zero return code
+echo "Test2 (wait with non-zero exit status of bg process)"
+sh -c 'sleep 1; false' &
+pid2=$!
+wait -n
+rc=$?
+if [ "$rc" != 1 ]; then
+ echo "Test2 exit code was not 1: $rc"
+fi
+
+wait $pid2
+rc=$?
+if [ "$rc" != 1 ]; then
+ echo "Test2 exit code was not 1 on recheck: $rc"
+fi
+
+# pipeline... not too fast
+echo "Test3 (pipeline wait for whole job)"
+d1=$(date +%s)
+sleep 2 | sleep 1 &
+wait -n
+rc=$?
+if [ "$rc" != 0 ]; then
+ echo "Test3 exit code was not 0: $rc"
+fi
+d2=$(date +%s)
+if [ "$d2" -lt "$((d1 + 2))" ]; then
+ echo "Test3 finished in less than 2s"
+fi
+
+kill $bg
+echo Ok:$?
--
2.47.3
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
