[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