[master] Restart waitpid on EINTR

Vratislav Podzimek vpodzime at redhat.com
Tue Aug 12 07:02:30 UTC 2014


On Mon, 2014-08-11 at 13:31 -0400, David Shea wrote:
> 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
This looks good to me.

-- 
Vratislav Podzimek

Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic




More information about the anaconda-patches mailing list