[PATCH 15/17] Move process watching to iutil.

David Shea dshea at redhat.com
Sun Sep 21 19:37:07 UTC 2014


Change the process watching methods to use Popen objects. Convert X and
metacity startup to use startProgram and watchProcess. This also moves
startMetacityWM into doStartupX11Actions because there isn't much left
of the former.
---
 anaconda                             | 166 +++++------------------------------
 pyanaconda/iutil.py                  |  94 ++++++++++++++++++++
 tests/pyanaconda_tests/iutil_test.py |  19 ++++
 3 files changed, 136 insertions(+), 143 deletions(-)

diff --git a/anaconda b/anaconda
index d17cc1f..f9807cd 100755
--- a/anaconda
+++ b/anaconda
@@ -44,85 +44,11 @@ if ("debug=1" in proc_cmdline) or ("debug" in proc_cmdline):
     cov.start()
 
 
-import atexit, sys, os, time, subprocess, signal, errno
-
-# Install a global SIGCHLD handler to keep track of things that should be
-# running for as long as anaconda does. The dictionary is of the form
-# {pid: name, ...}. The handler will raise ExitError (defined below), so if
-# not caught a SIGCHLD from a watched process will halt anaconda.
-forever_pids = {}
-
-class ExitError(RuntimeError):
-    pass
-
-def sigchld_handler(num, frame):
-    # Check whether anything in the list of processes being watched has
-    # exited. We don't want to call waitpid(-1), since that would break
-    # anything else using wait/waitpid (like the subprocess module).
-    exited_pids = []
-    exn_message = []
-
-    for child_pid in forever_pids:
-        try:
-            pid_result, status = os.waitpid(child_pid, os.WNOHANG)
-        except OSError as e:
-            if e.errno == errno.ECHILD:
-                continue
-
-        if pid_result:
-            proc_name = forever_pids[child_pid]
-            exited_pids.append(child_pid)
-
-            if os.WIFEXITED(status):
-                status_str = "with status %s" % os.WEXITSTATUS(status)
-            elif os.WIFSIGNALED(status):
-                status_str = "on signal %s" % os.WTERMSIG(status)
-            else:
-                status_str = "with unknown status code %s" % status
-
-            exn_message.append("%s exited %s" % (proc_name, status_str))
-
-    for child_pid in exited_pids:
-        del forever_pids[child_pid]
-
-    if exn_message:
-        raise ExitError(", ".join(exn_message))
-
-signal.signal(signal.SIGCHLD, sigchld_handler)
-
-# Fork a new process and add it to forever_pids. The return values are the
-# the same as os.fork, but neither the parent nor the child will return
-# until the process is watched.
-def start_watched_pid(proc_name):
-    readpipe, writepipe = os.pipe()
-    childpid = os.fork()
-    if childpid == 0:
-        # No need for the write pipe in the child
-        os.close(writepipe)
-
-        # Wait for the parent to signal that it's ready to return
-        os.read(readpipe, 1)
-
-        # Ready to go
-        os.close(readpipe)
-        return childpid
-    else:
-        # No need for the read pipe in the parent
-        os.close(readpipe)
-
-        # Add the pid to the list of watched pids
-        forever_pids[childpid] = proc_name
-
-        # Signal to the child that we're ready to return
-        # D is for Done
-        os.write(writepipe, 'D')
-        os.close(writepipe)
-        return childpid
+import atexit, sys, os, time, subprocess, signal
 
 def exitHandler(rebootData, storage):
     # Clear the list of watched PIDs.
-    global forever_pids
-    forever_pids = {}
+    iutil.unwatchAllProcesses()
 
     # stop and save coverage here b/c later the file system may be unavailable
     if coverage is not None:
@@ -196,56 +122,29 @@ def startX11():
         if x11_started[0]:
             return
         log.error("Timeout trying to start the X server")
-        raise ExitError("Timeout trying to start the X server")
+        raise iutil.ExitError("Timeout trying to start the X server")
+
+    # preexec_fn to add the SIGUSR1 handler in the child
+    def sigusr1_preexec():
+        signal.signal(signal.SIGUSR1, signal.SIG_IGN)
 
     try:
         old_sigusr1_handler = signal.signal(signal.SIGUSR1, sigusr1_handler)
         old_sigalrm_handler = signal.signal(signal.SIGALRM, sigalrm_handler)
 
-        childpid = start_watched_pid("Xorg")
+        # Open /dev/tty5 for stdout and stderr redirects
+        xfd = open("/dev/tty5", "a")
 
-        if not childpid:
-            # after this point the method should never return (or throw an exception
-            # outside)
-            try:
-                # dup /dev/tty5 to stdout and stderr
-                xfd = os.open("/dev/tty5", os.O_WRONLY | os.O_APPEND)
-                os.dup2(xfd, sys.stdout.fileno())
-                os.dup2(xfd, sys.stderr.fileno())
-
-                # Close all other file descriptors
-                try:
-                    maxfd = os.sysconf("SC_OPEN_MAX")
-                except ValueError:
-                    maxfd = 1024
-
-                os.closerange(3, maxfd)
-
-                # Replace the SIGUSR1 handler with SIG_IGN as described above
-                signal.signal(signal.SIGUSR1, signal.SIG_IGN)
-
-                # Run it
-                os.execlp("Xorg", "Xorg", "-br",
-                          "-logfile", "/tmp/X.log",
-                          ":1", "vt6", "-s", "1440", "-ac",
-                          "-nolisten", "tcp", "-dpi", "96",
-                          "-noreset")
-
-                # We should never get here
-                raise OSError(0, "Unable to exec")
-            except BaseException as e:
-                # catch all possible exceptions
-                # Reopen the logger in case it was closed by closerange.
-                # Use a different name because assigning to variables across a
-                # fork confuses the hell out of python.
-                child_log = logging.getLogger("anaconda")
-                child_log.error("Problems running Xorg: %s", e)
-                os._exit(1)
-
-        # Parent process
         # Start the timer
         signal.alarm(60)
 
+        childproc = iutil.startProgram(["Xorg", "-br", "-logfile", "/tmp/X.log",
+                                        ":1", "vt6", "-s", "1440", "-ac",
+                                        "-nolisten", "tcp", "-dpi", "96",
+                                        "-noreset"], stdout=xfd, stderr=xfd,
+                                        preexec_fn=sigusr1_preexec)
+        iutil.watchProcess(childproc, "Xorg")
+
         # Wait for SIGUSR1
         while not x11_started[0]:
             signal.pause()
@@ -258,28 +157,17 @@ def startX11():
         signal.signal(signal.SIGUSR1, old_sigusr1_handler)
         signal.signal(signal.SIGALRM, old_sigalrm_handler)
 
-def startMetacityWM():
+# function to handle X startup special issues for anaconda
+def doStartupX11Actions():
+    """Start window manager"""
+
     # When metacity actually connects to the X server is unknowable, but
     # fortunately it doesn't matter. metacity does not need to be the first
     # connection to Xorg, and if anaconda starts up before metacity, metacity
     # will just take over and maximize the window and make everything right,
     # fingers crossed.
-
-    childpid = start_watched_pid("metacity")
-    if not childpid:
-        # after this point the method should never return (or throw an exception
-        # outside)
-        try:
-            returncode = iutil.execWithRedirect('metacity', ["--display", ":1", "--sm-disable"])
-        except BaseException as e:
-            # catch all possible exceptions
-            log.error("Problems running the window manager: %s", e)
-            os._exit(1)
-
-        log.info("The window manager has terminated.")
-        os._exit(returncode)
-
-    return childpid
+    childproc = iutil.startProgram(["metacity", "--display", ":1", "--sm-disable"])
+    iutil.watchProcess(childproc, "metacity")
 
 def startAuditDaemon():
     childpid = os.fork()
@@ -293,14 +181,6 @@ def startAuditDaemon():
     # auditd will turn into a daemon so catch the immediate child pid now:
     os.waitpid(childpid, 0)
 
-# function to handle X startup special issues for anaconda
-def doStartupX11Actions():
-    """Start window manager"""
-
-    # now start up the window manager
-    wm_pid = startMetacityWM()
-    log.info("Starting window manager, pid %s.", wm_pid)
-
 def set_x_resolution(runres):
     if runres and opts.display_mode == 'g' and not flags.usevnc:
         try:
@@ -1376,7 +1256,7 @@ if __name__ == "__main__":
 
     try:
         main()
-    except ExitError as e:
+    except iutil.ExitError as e:
         # X crashed or something
         print("Anaconda is unable to continue: %s" % e.message)
         iutil.ipmi_report(constants.IPMI_ABORTED)
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
index 2ee0ebb..e09b791 100644
--- a/pyanaconda/iutil.py
+++ b/pyanaconda/iutil.py
@@ -338,6 +338,100 @@ def execConsole():
     except OSError as e:
         raise RuntimeError("Error running /bin/sh: " + e.strerror)
 
+# Dictionary of processes to watch in the form {pid: name, ...}
+_forever_pids = {}
+_watch_process_handler_set = False
+
+class ExitError(RuntimeError):
+    pass
+
+# Raise an error on process exit. The argument is a list of tuples
+# of the form [(name, status), ...] with statuses in the subprocess
+# format (>=0 is return codes, <0 is signal)
+def _raise_exit_error(statuses):
+    exn_message = []
+
+    for proc_name, status in statuses:
+        if status >= 0:
+            status_str = "with status %s" % status
+        else:
+            status_str = "on signal %s" % -status
+
+        exn_message.append("%s exited %s" % (proc_name, status_str))
+
+    raise ExitError(", ".join(exn_message))
+
+# Signal handler used with watchProcess
+def _sigchld_handler(num=None, frame=None):
+    # Check whether anything in the list of processes being watched has
+    # exited. We don't want to call waitpid(-1), since that would break
+    # anything else using wait/waitpid (like the subprocess module).
+    exited_pids = []
+    exit_statuses = []
+
+    for child_pid in _forever_pids:
+        try:
+            pid_result, status = os.waitpid(child_pid, os.WNOHANG)
+        except OSError as e:
+            if e.errno == errno.ECHILD:
+                continue
+
+        if pid_result:
+            proc_name = _forever_pids[child_pid]
+            exited_pids.append(child_pid)
+
+            # Convert the wait-encoded status to the format used by subprocess
+            if os.WIFEXITED(status):
+                sub_status = os.WEXITSTATUS(status)
+            else:
+                sub_status = -os.WTERMSIG(status)
+
+            exit_statuses.append((proc_name, sub_status))
+
+    for child_pid in exited_pids:
+        del _forever_pids[child_pid]
+
+    if exit_statuses:
+        _raise_exit_error(exit_statuses)
+
+def watchProcess(proc, name):
+    """Watch for a process exit, and raise a ExitError when it does.
+
+       This method installs a SIGCHLD signal handler and thus cannot be
+       used with the child_watch_add methods in GLib. Since the SIGCHLD
+       handler calls wait() on the watched process, this call cannot be
+       combined with Popen.wait() or Popen.communicate, and also doing
+       so wouldn't make a whole lot of sense.
+
+       :param proc: The Popen object for the process
+       :param name: The name of the process
+    """
+    global _watch_process_handler_set
+
+    if not _watch_process_handler_set:
+        signal.signal(signal.SIGCHLD, _sigchld_handler)
+        _watch_process_handler_set = True
+
+    # Add the PID to the dictionary
+    _forever_pids[proc.pid] = name
+
+    # Check that the process didn't already exit
+    if proc.poll() is not None:
+        del _forever_pids[proc.pid]
+        _raise_exit_error([(name, proc.returncode)])
+
+def unwatchProcess(proc):
+    """Unwatch a process watched by watchProcess.
+
+       :param proc: The Popen object for the process.
+    """
+    del _forever_pids[proc.pid]
+
+def unwatchAllProcesses():
+    """Clear the watched process list."""
+    global _forever_pids
+    _forever_pids = {}
+
 def getDirSize(directory):
     """ Get the size of a directory and all its subdirectories.
     :param dir: The name of the directory to find the size of.
diff --git a/tests/pyanaconda_tests/iutil_test.py b/tests/pyanaconda_tests/iutil_test.py
index dde3129..a08e167 100644
--- a/tests/pyanaconda_tests/iutil_test.py
+++ b/tests/pyanaconda_tests/iutil_test.py
@@ -384,6 +384,25 @@ done
             # Check that the process is gone
             self.assertIsNotNone(proc.poll())
 
+    def watch_process_test(self):
+        """Test watchProcess"""
+
+        def test_still_running():
+            with timer(5):
+                # Run something forever so we can kill it
+                proc = iutil.startProgram(["/bin/sh", "-c", "while true; do sleep 1; done"])
+                iutil.watchProcess(proc, "test1")
+                proc.kill()
+                # Wait for the SIGCHLD
+                signal.pause()
+        self.assertRaises(iutil.ExitError, test_still_running)
+
+        # Make sure watchProcess checks that the process has not already exited
+        with timer(5):
+            proc = iutil.startProgram(["true"])
+            proc.communicate()
+        self.assertRaises(iutil.ExitError, iutil.watchProcess, proc, "test2")
+
 class MiscTests(unittest.TestCase):
     def get_dir_size_test(self):
         """Test the getDirSize."""
-- 
1.9.3



More information about the anaconda-patches mailing list