← Commit list

91e11eba6

Denys Vlasenko

2020-09-29

SUCCESS Duration: 149s

Subject

ash: jobs: Fix waitcmd busy loop

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

shell/ash.c +34 -57

shell/ash.c+34 -57

@@ -4273,64 +4273,55 @@ sprint_status48(char *os, int status, int sigonly)
	return s - os;
	}

+	#define DOWAIT_NONBLOCK 0
+	#define DOWAIT_BLOCK 1
+	#define DOWAIT_BLOCK_OR_SIG 2
+	#if BASH_WAIT_N
+	# define DOWAIT_JOBSTATUS 0x10 /* OR this to get job's exitstatus instead of pid */
+	#endif
+
	static int
-	wait_block_or_sig(int *status)
+	waitproc(int block, int *status)
	{
-	int pid;
+	sigset_t oldmask;
+	int flags = block == DOWAIT_BLOCK ? 0 : WNOHANG;
+	int err;

-	do {
-	sigset_t mask;
+	#if JOBS
+	if (doing_jobctl)
+	flags \|= WUNTRACED;
+	#endif

-	/* Poll all children for changes in their state */
+	do {
	got_sigchld = 0;
-	/* if job control is active, accept stopped processes too */
-	pid = waitpid(-1, status, doing_jobctl ? (WNOHANG\|WUNTRACED) : WNOHANG);
-	if (pid != 0)
-	break; /* Error (e.g. EINTR, ECHILD) or pid */
+	do
+	err = waitpid(-1, status, flags);
+	while (err < 0 && errno == EINTR);

-	/* Children exist, but none are ready. Sleep until interesting signal */
-	#if 1
-	sigfillset(&mask);
-	sigprocmask2(SIG_SETMASK, &mask); /* mask is updated */
-	while (!got_sigchld && !pending_sig) {
-	sigsuspend(&mask);
-	/* ^^^ add "sigdelset(&mask, SIGCHLD);" before sigsuspend
-	* to make sure SIGCHLD is not masked off?
-	* It was reported that this:
-	* fn() { : \| return; }
-	* shopt -s lastpipe
-	* fn
-	* exec ash SCRIPT
-	* under bash 4.4.23 runs SCRIPT with SIGCHLD masked,
-	* making "wait" commands in SCRIPT block forever.
-	*/
-	}
-	sigprocmask(SIG_SETMASK, &mask, NULL);
-	#else /* unsafe: a signal can set pending_sig after check, but before pause() */
+	if (err \|\| (err = -!block))
+	break;
+
+	sigfillset(&oldmask);
+	sigprocmask2(SIG_SETMASK, &oldmask); /* mask is updated */
	while (!got_sigchld && !pending_sig)
-	pause();
-	#endif
+	sigsuspend(&oldmask);
+	sigprocmask(SIG_SETMASK, &oldmask, NULL);
+	//simpler, but unsafe: a signal can set pending_sig after check, but before pause():
+	//while (!got_sigchld && !pending_sig)
+	// pause();

-	/* If it was SIGCHLD, poll children again */
	} while (got_sigchld);

-	return pid;
+	return err;
	}

-	#define DOWAIT_NONBLOCK 0
-	#define DOWAIT_BLOCK 1
-	#define DOWAIT_BLOCK_OR_SIG 2
-	#if BASH_WAIT_N
-	# define DOWAIT_JOBSTATUS 0x10 /* OR this to get job's exitstatus instead of pid */
-	#endif
-
	static int
	waitone(int block, struct job *job)
	{
	int pid;
	int status;
	struct job *jp;
-	struct job *thisjob;
+	struct job *thisjob = NULL;
	#if BASH_WAIT_N
	bool want_jobexitstatus = (block & DOWAIT_JOBSTATUS);
	block = (block & ~DOWAIT_JOBSTATUS);
@@ -4357,21 +4348,8 @@ waitone(int block, struct job *job)
	* SIG_DFL handler does not wake sigsuspend().
	*/
	INT_OFF;
-	if (block == DOWAIT_BLOCK_OR_SIG) {
-	pid = wait_block_or_sig(&status);
-	} else {
-	int wait_flags = 0;
-	if (block == DOWAIT_NONBLOCK)
-	wait_flags = WNOHANG;
-	/* if job control is active, accept stopped processes too */
-	if (doing_jobctl)
-	wait_flags \|= WUNTRACED;
-	/* NB: _not_ safe_waitpid, we need to detect EINTR */
-	pid = waitpid(-1, &status, wait_flags);
-	}
-	TRACE(("wait returns pid=%d, status=0x%x, errno=%d(%s)\n",
-	pid, status, errno, strerror(errno)));
-	thisjob = NULL;
+	pid = waitproc(block, &status);
+	TRACE(("wait returns pid %d, status=%d\n", pid, status));
	if (pid <= 0)
	goto out;

@@ -4466,7 +4444,6 @@ dowait(int block, struct job *jp)
	rpid = 1;

	do {
-	got_sigchld = 0;
	pid = waitone(block, jp);
	rpid &= !!pid;

@@ -4721,7 +4698,7 @@ waitcmd(int argc UNUSED_PARAM, char **argv)
	job = getjob(*argv, 0);
	}
	/* loop until process terminated or stopped */
-	dowait(DOWAIT_BLOCK_OR_SIG, NULL);
+	dowait(DOWAIT_BLOCK_OR_SIG, job);
	if (pending_sig)
	goto sigout;
	job->waited = 1;