diff --git a/src/builtin_disown.cpp b/src/builtin_disown.cpp index 3c9822df7..510044803 100644 --- a/src/builtin_disown.cpp +++ b/src/builtin_disown.cpp @@ -36,7 +36,7 @@ static int disown_job(const wchar_t *cmd, parser_t &parser, io_streams_t &stream // within the context of a subjob which will cause the parent job to crash in exec_job(). // Instead, we set a flag and the parser removes the job from the jobs list later. j->mut_flags().disown_requested = true; - if (pgid) add_disowned_pgid(*pgid); + add_disowned_job(j); return STATUS_CMD_OK; } diff --git a/src/proc.cpp b/src/proc.cpp index b4558eb55..268ae2224 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -326,13 +326,23 @@ bool job_t::has_external_proc() const { /// we exit. Poll these from time-to-time to prevent zombie processes from happening (#5342). static owning_lock> s_disowned_pids; -void add_disowned_pgid(pid_t pgid) { +void add_disowned_job(job_t *j) { + if (j == nullptr) return; + // NEVER add our own (or an invalid) pgid as they are not unique to only // one job, and may result in a deadlock if we attempt the wait. - if (pgid != getpgrp() && pgid > 0) { + auto pgid = j->get_pgid(); + if (pgid && *pgid != getpgrp() && *pgid > 0) { // waitpid(2) is signalled to wait on a process group rather than a // process id by using the negative of its value. - s_disowned_pids.acquire()->push_back(pgid * -1); + s_disowned_pids.acquire()->push_back(*pgid * -1); + } else { + // Instead, add the PIDs of any external processes + for (auto &process : j->processes) { + if (process->pid) { + s_disowned_pids.acquire()->push_back(process->pid); + } + } } } @@ -341,12 +351,14 @@ static void reap_disowned_pids() { auto disowned_pids = s_disowned_pids.acquire(); auto try_reap1 = [](pid_t pid) { int status; - int ret = waitpid(pid, &status, WNOHANG) > 0; - if (ret) { + int ret = waitpid(pid, &status, WNOHANG); + if (ret > 0) { FLOGF(proc_reap_external, "Reaped disowned PID or PGID %d", pid); } return ret; }; + // waitpid returns 0 iff the PID/PGID in question has not changed state; remove the pid/pgid + // if it has changed or an error occurs (presumably ECHILD because the child does not exist) disowned_pids->erase(std::remove_if(disowned_pids->begin(), disowned_pids->end(), try_reap1), disowned_pids->end()); } diff --git a/src/proc.h b/src/proc.h index 934eab2b5..f9ce8d50b 100644 --- a/src/proc.h +++ b/src/proc.h @@ -551,9 +551,9 @@ void hup_jobs(const job_list_t &jobs); /// \return 1 if transferred, 0 if no transfer was necessary, -1 on error. int terminal_maybe_give_to_job_group(const job_group_t *jg, bool continuing_from_stopped); -/// Add a pid to the list of pids we wait on even though they are not associated with any jobs. -/// Used to avoid zombie processes after disown. -void add_disowned_pgid(pid_t pgid); +/// Add a job to the list of PIDs/PGIDs we wait on even though they are not associated with any +/// jobs. Used to avoid zombie processes after disown. +void add_disowned_job(job_t *j); bool have_proc_stat();