subproc: reap processes after killing

Always try to release resources if possible.

Fixes #69

diff --git a/nsjail.cc b/nsjail.cc
index 830f554..0b57033 100644 (file)
--- a/nsjail.cc
+++ b/nsjail.cc
@@ -122,7 +122,7 @@ static int listenMode(nsjconf_t* nsjconf) {
        }
        for (;;) {
                if (sigFatal > 0) {
-                       subproc::killAll(nsjconf);
+                       subproc::killAndReapAll(nsjconf);
                        logs::logStop(sigFatal);
                        close(listenfd);
                        return EXIT_SUCCESS;
@@ -159,7 +159,7 @@ static int standaloneMode(nsjconf_t* nsjconf) {
                                subproc::displayProc(nsjconf);
                        }
                        if (sigFatal > 0) {
-                               subproc::killAll(nsjconf);
+                               subproc::killAndReapAll(nsjconf);
                                logs::logStop(sigFatal);
                                return (128 + sigFatal);
                        }

diff --git a/subproc.cc b/subproc.cc
index 7da5c2c..1502026 100644 (file)
--- a/subproc.cc
+++ b/subproc.cc
@@ -288,8 +288,39 @@ static void seccompViolation(nsjconf_t* nsjconf, siginfo_t* si) {
        }
 }
 
-int reapProc(nsjconf_t* nsjconf) {
+static int reapProc(nsjconf_t* nsjconf, pid_t pid, bool should_wait = false) {
        int status;
+
+       if (wait4(pid, &status, should_wait ? 0 : WNOHANG, NULL) == pid) {
+               cgroup::finishFromParent(nsjconf, pid);
+
+               std::string remote_txt = "[UNKNOWN]";
+               const pids_t* elem = getPidElem(nsjconf, pid);
+               if (elem) {
+                       remote_txt = elem->remote_txt;
+               }
+
+               if (WIFEXITED(status)) {
+                       LOG_I("PID: %d (%s) exited with status: %d, (PIDs left: %d)",
+                           pid, remote_txt.c_str(), WEXITSTATUS(status),
+                           countProc(nsjconf) - 1);
+                       removeProc(nsjconf, pid);
+                       return WEXITSTATUS(status);
+               }
+               if (WIFSIGNALED(status)) {
+                       LOG_I(
+                           "PID: %d (%s) terminated with signal: %s (%d), (PIDs left: %d)",
+                           pid, remote_txt.c_str(),
+                           util::sigName(WTERMSIG(status)).c_str(), WTERMSIG(status),
+                           countProc(nsjconf) - 1);
+                       removeProc(nsjconf, pid);
+                       return 128 + WTERMSIG(status);
+               }
+       }
+       return 0;
+}
+
+int reapProc(nsjconf_t* nsjconf) {
        int rv = 0;
        siginfo_t si;
 
@@ -304,33 +335,7 @@ int reapProc(nsjconf_t* nsjconf) {
                if (si.si_code == CLD_KILLED && si.si_status == SIGSYS) {
                        seccompViolation(nsjconf, &si);
                }
-
-               if (wait4(si.si_pid, &status, WNOHANG, NULL) == si.si_pid) {
-                       cgroup::finishFromParent(nsjconf, si.si_pid);
-
-                       std::string remote_txt = "[UNKNOWN]";
-                       const pids_t* elem = getPidElem(nsjconf, si.si_pid);
-                       if (elem) {
-                               remote_txt = elem->remote_txt;
-                       }
-
-                       if (WIFEXITED(status)) {
-                               LOG_I("PID: %d (%s) exited with status: %d, (PIDs left: %d)",
-                                   si.si_pid, remote_txt.c_str(), WEXITSTATUS(status),
-                                   countProc(nsjconf) - 1);
-                               removeProc(nsjconf, si.si_pid);
-                               rv = WEXITSTATUS(status);
-                       }
-                       if (WIFSIGNALED(status)) {
-                               LOG_I(
-                                   "PID: %d (%s) terminated with signal: %s (%d), (PIDs left: %d)",
-                                   si.si_pid, remote_txt.c_str(),
-                                   util::sigName(WTERMSIG(status)).c_str(), WTERMSIG(status),
-                                   countProc(nsjconf) - 1);
-                               removeProc(nsjconf, si.si_pid);
-                               rv = 128 + WTERMSIG(status);
-                       }
-               }
+               rv = reapProc(nsjconf, si.si_pid);
        }
 
        time_t now = time(NULL);
@@ -357,9 +362,14 @@ int reapProc(nsjconf_t* nsjconf) {
        return rv;
 }
 
-void killAll(nsjconf_t* nsjconf) {
-       for (const auto& p : nsjconf->pids) {
-               kill(p.pid, SIGKILL);
+void killAndReapAll(nsjconf_t* nsjconf) {
+       while (!nsjconf->pids.empty()) {
+               pid_t pid = nsjconf->pids.front().pid;
+               if (kill(pid, SIGKILL) == 0) {
+                       reapProc(nsjconf, pid, true);
+               } else {
+                       removeProc(nsjconf, pid);
+               }
        }
 }
 

diff --git a/subproc.h b/subproc.h
index c09240f..33e2b5c 100644 (file)
--- a/subproc.h
+++ b/subproc.h
@@ -36,7 +36,7 @@ namespace subproc {
 bool runChild(nsjconf_t* nsjconf, int fd_in, int fd_out, int fd_err);
 int countProc(nsjconf_t* nsjconf);
 void displayProc(nsjconf_t* nsjconf);
-void killAll(nsjconf_t* nsjconf);
+void killAndReapAll(nsjconf_t* nsjconf);
 /* Returns the exit code of the first failing subprocess, or 0 if none fail */
 int reapProc(nsjconf_t* nsjconf);
 int systemExe(const std::vector<std::string>& args, char** env);