Correctly handle exited jobs in process_mark_finished_children

This is effectively a pick of 2ebdcf82eebec230549b64dc740ea604c7772049
and the subsequent fixup. However we also avoid setting WNOHANG unless
waitpid() indicates a process was reaped.

Fixes #5438
This commit is contained in:
ridiculousfish 2019-01-20 15:04:27 -08:00
parent f4351eb0f3
commit e2f2dbf032

View File

@ -467,6 +467,11 @@ static bool process_mark_finished_children(bool block_on_fg) {
break; break;
} }
assert((*process)->pid != INVALID_PID && "Waiting by process on an invalid PID!"); assert((*process)->pid != INVALID_PID && "Waiting by process on an invalid PID!");
// Don't wait for completed jobs; see #5438.
if ((*process)->completed) {
process++;
continue;
}
pid = waitpid((*process)->pid, &status, options); pid = waitpid((*process)->pid, &status, options);
process++; process++;
} else { } else {
@ -475,14 +480,14 @@ static bool process_mark_finished_children(bool block_on_fg) {
pid = waitpid(-1 * j->pgid, &status, options); pid = waitpid(-1 * j->pgid, &status, options);
} }
// Never make two calls to waitpid(2) without WNOHANG (i.e. with "HANG") in a row,
// because we might wait on a non-stopped job that becomes stopped, but we don't refresh
// our view of the process state before calling waitpid(2) again here.
options |= WNOHANG;
if (pid > 0) { if (pid > 0) {
// A child process has been reaped // A child process has been reaped
handle_child_status(pid, status); handle_child_status(pid, status);
// Always set WNOHANG (that is, don't hang). Otherwise we might wait on a non-stopped job
// that becomes stopped, but we don't refresh our view of the process state before
// calling waitpid(2) again here.
options |= WNOHANG;
} else if (pid == 0 || errno == ECHILD) { } else if (pid == 0 || errno == ECHILD) {
// No killed/dead children in this particular process group // No killed/dead children in this particular process group
if (!wait_by_process) { if (!wait_by_process) {