From 409a9b66b6bea230422a56537029a21f5cc1c4c0 Mon Sep 17 00:00:00 2001 From: Stephen Atherton Date: Mon, 8 Oct 2018 10:42:11 -0700 Subject: [PATCH 1/3] FDBMonitor will only kill its entire process group on exit if it started its own process group with setsid() (as with daemonize mode), otherwise it will kill its processes individually. --- fdbmonitor/fdbmonitor.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/fdbmonitor/fdbmonitor.cpp b/fdbmonitor/fdbmonitor.cpp index c1b57dfa6d..2987b2df8e 100644 --- a/fdbmonitor/fdbmonitor.cpp +++ b/fdbmonitor/fdbmonitor.cpp @@ -635,13 +635,15 @@ bool argv_equal(const char** a1, const char** a2) return true; } -void kill_process(uint64_t id) { +void kill_process(uint64_t id, bool wait = true) { pid_t pid = id_pid[id]; log_msg(SevInfo, "Killing process %d\n", pid); kill(pid, SIGTERM); - waitpid(pid, NULL, 0); + if(wait) { + waitpid(pid, NULL, 0); + } pid_id.erase(pid); id_pid.erase(id); @@ -1367,8 +1369,16 @@ int main(int argc, char** argv) { signal(SIGCHLD, SIG_IGN); sigprocmask(SIG_SETMASK, &normal_mask, NULL); - /* Send SIGHUP to all child processes */ - kill(0, SIGHUP); + /* If daemonized, setsid() was called earlier so we can just kill our entire new process group */ + if(daemonize) { + kill(0, SIGHUP); + } + else { + /* Otherwise kill each process individually but don't wait on them yet */ + for(auto i : id_pid) { + kill_process(i->first, false); + } + } /* Wait for all child processes (says POSIX.1-2001) */ /* POSIX.1-2001 specifies that if the disposition of SIGCHLD is set to SIG_IGN, then children that terminate do not become zombies and a call to wait() From 4488ac922c61a9aa8168fded6b019805e072f069 Mon Sep 17 00:00:00 2001 From: Stephen Atherton Date: Mon, 8 Oct 2018 10:51:10 -0700 Subject: [PATCH 2/3] Typo which was missed because of a local make issue causing success without recompile. --- fdbmonitor/fdbmonitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdbmonitor/fdbmonitor.cpp b/fdbmonitor/fdbmonitor.cpp index 2987b2df8e..84d5273399 100644 --- a/fdbmonitor/fdbmonitor.cpp +++ b/fdbmonitor/fdbmonitor.cpp @@ -1376,7 +1376,7 @@ int main(int argc, char** argv) { else { /* Otherwise kill each process individually but don't wait on them yet */ for(auto i : id_pid) { - kill_process(i->first, false); + kill_process(i.first, false); } } From 3bd66217c661e9deec7989f7d9171cc7cd5b1015 Mon Sep 17 00:00:00 2001 From: Stephen Atherton Date: Thu, 8 Nov 2018 16:21:50 -0800 Subject: [PATCH 3/3] Bug fix, kill_process() modifies the id <-> pid maps so to kill all ids one by one the loop iterator must be advanced before the kill. --- fdbmonitor/fdbmonitor.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fdbmonitor/fdbmonitor.cpp b/fdbmonitor/fdbmonitor.cpp index 84d5273399..0a5fa186c6 100644 --- a/fdbmonitor/fdbmonitor.cpp +++ b/fdbmonitor/fdbmonitor.cpp @@ -1375,8 +1375,11 @@ int main(int argc, char** argv) { } else { /* Otherwise kill each process individually but don't wait on them yet */ - for(auto i : id_pid) { - kill_process(i.first, false); + auto i = id_pid.begin(); + auto iEnd = id_pid.end(); + while(i != iEnd) { + // Must advance i before calling kill_process() which erases the entry at i + kill_process((i++)->first, false); } }