proc: disown PIDs, not just PGIDs

add_disowned_pgid skipped jobs that have a PGID equal to the running
process. However, this includes processes started in config.fish or when
job control is turned off, so they never get waited on.

Instead, refactor this function to add_disowned_job, and add either the PGID or
all the PIDs of the job to the list of disowned PIDs/PGIDs.

Fixes #7183.

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;
 }

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<std::vector<pid_t>> 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;
+        int ret = waitpid(pid, &status, WNOHANG);
-        if (ret) {
+        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());
 }

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.
+/// Add a job to the list of PIDs/PGIDs we wait on even though they are not associated with any
-/// Used to avoid zombie processes after disown.
+/// jobs. Used to avoid zombie processes after disown.
-void add_disowned_pgid(pid_t pgid);
+void add_disowned_job(job_t *j);
 
 bool have_proc_stat();