[master] Restart waitpid on EINTR

David Shea dshea at redhat.com
Mon Aug 11 17:31:28 UTC 2014


Depending on the order or child exits and signals and waitpid starting,
calling waitpid alone sometimes works and sometimes crashes anaconda.
Check if an exception indicates that waitpid should restart.
---
 anaconda                             |  8 ++++++--
 pyanaconda/iutil.py                  | 16 ++++++++++++++++
 pyanaconda/users.py                  |  2 +-
 tests/pyanaconda_tests/iutil_test.py | 36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/anaconda b/anaconda
index b4dddc9..8e58c18 100755
--- a/anaconda
+++ b/anaconda
@@ -56,6 +56,8 @@ class ExitError(RuntimeError):
     pass
 
 def sigchld_handler(num, frame):
+    from pyanaconda.iutil import really_waitpid
+
     # 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).
@@ -64,7 +66,7 @@ def sigchld_handler(num, frame):
 
     for child_pid in forever_pids:
         try:
-            pid_result, status = os.waitpid(child_pid, os.WNOHANG)
+            pid_result, status = really_waitpid(child_pid, os.WNOHANG)
         except OSError as e:
             if e.errno == errno.ECHILD:
                 continue
@@ -281,6 +283,8 @@ def startMetacityWM():
     return childpid
 
 def startAuditDaemon():
+    from pyanaconda.iutil import really_waitpid
+
     childpid = os.fork()
     if not childpid:
         cmd = '/sbin/auditd'
@@ -290,7 +294,7 @@ def startAuditDaemon():
             log.error("Error running the audit daemon: %s", e)
         os._exit(0)
     # auditd will turn into a daemon so catch the immediate child pid now:
-    os.waitpid(childpid, 0)
+    really_waitpid(childpid, 0)
 
 # function to handle X startup special issues for anaconda
 def doStartupX11Actions():
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
index e426511..af87860 100644
--- a/pyanaconda/iutil.py
+++ b/pyanaconda/iutil.py
@@ -913,3 +913,19 @@ def persistent_root_image():
             return False
 
     return True
+
+def really_waitpid(pid, options):
+    """Run waitpid, and restart if waitpid returns with EINTR.
+
+      :param int pid: the PID to wait for
+      :param options: Options to pass to os.waitpid
+      :return: A tuple containing the process ID and exit status
+      :rtype: tuple
+    """
+    while True:
+        try:
+            return os.waitpid(pid, options)
+        except OSError as e:
+            if e.errno == errno.EINTR:
+                continue
+            raise
diff --git a/pyanaconda/users.py b/pyanaconda/users.py
index 82cd2d3..aebe65d 100644
--- a/pyanaconda/users.py
+++ b/pyanaconda/users.py
@@ -218,7 +218,7 @@ class Users:
     def _finishChroot(self, childpid):
         assert childpid > 0
         try:
-            status = os.waitpid(childpid, 0)[1]
+            status = iutil.really_waitpid(childpid, 0)[1]
         except OSError as e:
             log.critical("exception from waitpid: %s %s", e.errno, e.strerror)
             return False
diff --git a/tests/pyanaconda_tests/iutil_test.py b/tests/pyanaconda_tests/iutil_test.py
index ac4fb76..76824a9 100644
--- a/tests/pyanaconda_tests/iutil_test.py
+++ b/tests/pyanaconda_tests/iutil_test.py
@@ -25,6 +25,7 @@ import types
 import os
 import shutil
 from test_constants import ANACONDA_TEST_DIR
+import errno
 
 class UpcaseFirstLetterTests(unittest.TestCase):
 
@@ -434,3 +435,38 @@ class RunProgramTests(unittest.TestCase):
         self.assertFalse(iutil.have_word_match("", None))
         self.assertFalse(iutil.have_word_match(None, ""))
         self.assertFalse(iutil.have_word_match(None, None))
+
+    def waitpid_test(self):
+        """Test restarting waitpid when a signal is received."""
+
+        # Instead of trying to interrupt an actual waitpid call, which is
+        # hard to do without some inherent race, just replace os.waitpid with
+        # a version that raises EINTR
+
+        _oldwaitpid = os.waitpid
+        def _intrwaitpid(pid, options):
+            os.waitpid = _oldwaitpid
+            raise OSError(errno.EINTR, "Interrupted")
+
+        # Test that waitpid doesn't work
+        try:
+            os.waitpid = _intrwaitpid
+            childpid = os.fork()
+            if childpid == 0:
+                os._exit(0)
+            with self.assertRaises(OSError):
+                os.waitpid(childpid, 0)
+        finally:
+            os.waitpid = _oldwaitpid
+
+        # Test that really_waitpid does work
+        try:
+            os.waitpid = _intrwaitpid
+            childpid = os.fork()
+            if childpid == 0:
+                os._exit(0)
+            (_pid, result) = iutil.really_waitpid(childpid, 0)
+            self.assertTrue(os.WIFEXITED(result))
+            self.assertEqual(os.WEXITSTATUS(result), 0)
+        finally:
+            os.waitpid = _oldwaitpid
-- 
2.0.0



More information about the anaconda-patches mailing list