From 8730b482a7e383d9beb6b5195f709b43dc332530 Mon Sep 17 00:00:00 2001
From: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Date: Sun, 18 Nov 2018 15:18:18 -0600
Subject: [PATCH] Prevent zombie processes after disowned child procs exit

Closes #5346.
---
 src/builtin_disown.cpp |  9 +++++++--
 src/proc.cpp           | 15 +++++++++++++++
 src/proc.h             |  3 +++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/builtin_disown.cpp b/src/builtin_disown.cpp
index 7fda7ed11..382c95099 100644
--- a/src/builtin_disown.cpp
+++ b/src/builtin_disown.cpp
@@ -31,8 +31,13 @@ static int disown_job(const wchar_t *cmd, parser_t &parser, io_streams_t &stream
         streams.err.append_format(fmt, cmd, j->job_id, j->command_wcstr());
     }
 
-    if (parser.job_remove(j)) return STATUS_CMD_OK;
-    return STATUS_CMD_ERROR;
+    pid_t pgid = j->pgid;
+    if (!parser.job_remove(j)) {
+        return STATUS_CMD_ERROR;
+    }
+
+    add_disowned_pgid(pgid);
+    return STATUS_CMD_OK;
 }
 
 /// Builtin for removing jobs from the job list.
diff --git a/src/proc.cpp b/src/proc.cpp
index d12562c0b..5e0523187 100644
--- a/src/proc.cpp
+++ b/src/proc.cpp
@@ -349,6 +349,14 @@ io_chain_t job_t::all_io_redirections() const {
 
 typedef unsigned int process_generation_count_t;
 
+/// A list of pids/pgids that have been disowned. They are kept around until either they exit or
+/// we exit. Poll these from time-to-time to prevent zombie processes from happening (#5342).
+static std::vector<pid_t> s_disowned_pids;
+
+void add_disowned_pgid(pid_t pgid) {
+    s_disowned_pids.push_back(pgid * -1);
+}
+
 /// A static value tracking how many SIGCHLDs we have seen, which is used in a heurstic to
 /// determine if we should call waitpid() at all in `process_mark_finished_children`.
 static volatile process_generation_count_t s_sigchld_generation_cnt = 0;
@@ -496,6 +504,13 @@ static bool process_mark_finished_children(bool block_on_fg) {
         }
     }
 
+    // Poll disowned processes/process groups, but do nothing with the result. Only used to avoid
+    // zombie processes. Entries have already be converted to negative for process groups.
+    int status;
+    s_disowned_pids.erase(std::remove_if(s_disowned_pids.begin(), s_disowned_pids.end(),
+                [&status](pid_t pid) { return waitpid(pid, &status, WNOHANG) > 0; }),
+            s_disowned_pids.end());
+
     // Yes, the below can be collapsed to a single line, but it's worth being explicit about it with
     // the comments. Fret not, the compiler will optimize it. (It better!)
     if (jobs_skipped) {
diff --git a/src/proc.h b/src/proc.h
index 645e40ed5..f5d1abc95 100644
--- a/src/proc.h
+++ b/src/proc.h
@@ -408,6 +408,9 @@ bool terminal_give_to_job(const job_t *j, bool restore_attrs);
 /// Returns the pid to restore after running the builtin, or -1 if there is no pid to restore.
 pid_t terminal_acquire_before_builtin(int job_pgid);
 
+/// 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);
 
 /// 0 should not be used; although it is not a valid PGID in userspace,
 ///   the Linux kernel will use it for kernel processes.