lutris-wrapper: reaping processes immediately

Rather than waiting for the initial process to exit before we start
reaping child processes, instead launch the initial process async
and use the same wait3->reap loop on the initial process as the
inherited processes.

Refactor util/monitor and util/process to stop emiting logging
information or attempting to track state changes since POSIX signals
don't give us enough opportunity to track this information reliably.
I'm sure some users will be happy to not see their terminal spammed.

bin/lutris-wrapper

@@ -84,11 +84,9 @@ def main():
     def hard_sig_handler(signum, _frame):
         log("Caught another signal, sending SIGKILL.")
         for _ in range(3):  # just in case we race a new process.
-            monitor.refresh_process_status()
-            children = monitor.children + monitor.ignored_children
             if not children:
                 break
-            for child in children:
+            for child in monitor.iterate_all_processes():
                 try:
                     os.kill(child.pid, signal.SIGKILL)
                 except ProcessLookupError:  # process already dead
@@ -99,8 +97,7 @@ def main():
         log("Caught signal %s" % signum)
         signal.signal(signal.SIGTERM, hard_sig_handler)
         signal.signal(signal.SIGINT, hard_sig_handler)
-        monitor.refresh_process_status()
-        for child in monitor.children:
+        for child in monitor.iterate_monitored_processes():
             log("passing along signal to PID %s" % child.pid)
             try:
                 os.kill(child.pid, signum)
@@ -108,26 +105,46 @@ def main():
                 pass
         log("--terminated processes--")
 
-    old_sigterm_handler = signal.signal(signal.SIGTERM, sig_handler)
-    old_sigint_handler = signal.signal(signal.SIGINT, sig_handler)
+    signal.signal(signal.SIGTERM, sig_handler)
+    signal.signal(signal.SIGINT, sig_handler)
 
     log("Running %s" % " ".join(args))
+    returncode = None
     try:
-        returncode = subprocess.run(args).returncode
+        initial_pid = subprocess.Popen(args).pid
     except FileNotFoundError:
         log("Failed to execute process. Check that the file exists")
         return
-    try:
+
+    log("Initial process has started with pid %d" % initial_pid)
+
+    while monitor.is_game_alive():
+        # Wait for a process to die.
+        try:
+            dead_pid, dead_returncode, _ = os.wait3(0)
+        except ChildProcessError:
+            # No processes remain. No need to check monitor.
+            break
+
         while True:
-            log("Waiting on children")
-            os.wait3(0)
-            if not monitor.refresh_process_status():
-                log("All children gone")
+            # Reap as many children as possible before checking
+            # our monitor list.
+            if dead_pid == initial_pid:
+                log("Initial process has died.")
+                returncode = dead_returncode
+
+            try:
+                dead_pid, dead_returncode, _ = os.wait3(os.WNOHANG)
+            except ChildProcessError:
+                # No processes remain.
+                break 
+
+            if dead_pid == 0:  # No more children to reap
                 break
-    except ChildProcessError:
-        # If the game itself has quit then
-        # this process has no children
-        pass
+
+    if returncode is None:
+        returncode = 0
+        log("Never found the initial process' return code. Weird?")
     log("Exit with returncode %s" % returncode)
     sys.exit(returncode)
 

lutris/util/monitor.py

@@ -2,7 +2,6 @@
 import os
 import shlex
 
-from lutris.util.log import logger
 from lutris.util.process import Process
 
 
@@ -57,9 +56,6 @@ class ProcessMonitor:
         self.exclude_processes = [
             x[0:15] for x in EXCLUDED_PROCESSES + self.parse_process_list(exclude_processes)
         ]
-        # Keep a copy of the monitored processes to allow comparisons
-        self.children = []
-        self.ignored_children = []
 
     @staticmethod
     def parse_process_list(process_list):
@@ -70,38 +66,9 @@ class ProcessMonitor:
             return shlex.split(process_list)
         return process_list
 
-    def iter_children(self, process, topdown=True):
-        """Iterator that yields all the children of a process"""
-        for child in process.children:
-            if topdown:
-                yield child
-            yield from self.iter_children(child, topdown=topdown)
-            if not topdown:
-                yield child
-
-    @staticmethod
-    def _log_changes(label, old, new):
-        newpids = {p.pid for p in new}
-        oldpids = {p.pid for p in old}
-        added = [p for p in new if p.pid not in oldpids]
-        removed = [p for p in old if p.pid not in newpids]
-        if added:
-            logger.debug("New %s processes: %s", label, ', '.join(map(str, added)))
-        if removed:
-            logger.debug("Dead %s processes: %s", label, ', '.join(map(str, removed)))
-
-    def refresh_process_status(self):
-        """Return status of a process"""
-        old_children, self.children = self.children, []
-        old_ignored_children, self.ignored_children = self.ignored_children, []
-
-        for child in self.iter_children(Process(os.getpid())):
-            if child.state == 'Z':  # should never happen anymore...
-                logger.debug("Unexpected zombie process %s", child)
-                try:
-                    os.wait3(os.WNOHANG)
-                except ChildProcessError:
-                    pass
+    def iterate_monitored_processes(self):
+        for child in Process(os.getpid()).iter_children():
+            if child.state == 'Z':
                 continue
 
             if (
@@ -109,11 +76,15 @@ class ProcessMonitor:
                     and child.name in self.exclude_processes
                     and child.name not in self.include_processes
             ):
-                self.ignored_children.append(child)
+                pass
             else:
-                self.children.append(child)
+                yield child
 
-        self._log_changes('ignored', old_ignored_children, self.ignored_children)
-        self._log_changes('monitored', old_children, self.children)
+    def iterate_all_processes(self):
+        return Process(os.getpid()).iter_children()
 
-        return len(self.children) > 0
+    def is_game_alive(self):
+        "Returns whether at least one nonexcluded process exists"
+        for child in self.iterate_monitored_processes():
+            return True
+        return False

lutris/util/process.py

@@ -94,3 +94,9 @@ class Process:
             for child_pid in self.get_children_pids_of_thread(tid):
                 _children.append(Process(child_pid))
         return _children
+
+    def iter_children(self):
+        """Iterator that yields all the children of a process"""
+        for child in self.children:
+            yield child
+            yield from child.iter_children()