Use subprocess to start child processes; closes #185.

mitogen/parent.py

@@ -34,6 +34,7 @@ import os
 import select
 import signal
 import socket
+import subprocess
 import sys
 import termios
 import textwrap
@@ -243,56 +244,58 @@ def create_socketpair():
 
 def create_child(*args):
     parentfp, childfp = create_socketpair()
-    pid = os.fork()
-    if not pid:
     # When running under a monkey patches-enabled gevent, the socket module
     # yields file descriptors who already have O_NONBLOCK, which is
     # persisted across fork, totally breaking Python. Therefore, drop
     # O_NONBLOCK from Python's future stdin fd.
     mitogen.core.set_block(childfp.fileno())
-        os.dup2(childfp.fileno(), 0)
-        os.dup2(childfp.fileno(), 1)
-        childfp.close()
-        parentfp.close()
-        os.execvp(args[0], args)
 
+    proc = subprocess.Popen(
+        args=args,
+        stdin=childfp,
+        stdout=childfp,
+        close_fds=True,
+    )
     childfp.close()
     # Decouple the socket from the lifetime of the Python socket object.
     fd = os.dup(parentfp.fileno())
     parentfp.close()
 
     LOG.debug('create_child() child %d fd %d, parent %d, cmd: %s',
-              pid, fd, os.getpid(), Argv(args))
-    return pid, fd
+              proc.pid, fd, os.getpid(), Argv(args))
+    return proc.pid, fd
 
 
-def tty_create_child(*args):
-    master_fd, slave_fd = os.openpty()
-    disable_echo(master_fd)
-    disable_echo(slave_fd)
-
-    pid = os.fork()
-    if not pid:
-        mitogen.core.set_block(slave_fd)
-        os.dup2(slave_fd, 0)
-        os.dup2(slave_fd, 1)
-        os.dup2(slave_fd, 2)
-        close_nonstandard_fds()
+def _acquire_controlling_tty():
     os.setsid()
     if sys.platform == 'linux2':
         # On Linux, the controlling tty becomes the first tty opened by a
         # process lacking any prior tty.
-            os.close(os.open(os.ttyname(1), os.O_RDWR))
+        os.close(os.open(os.ttyname(0), os.O_RDWR))
     if sys.platform.startswith('freebsd') or sys.platform == 'darwin':
         # On BSD an explicit ioctl is required.
         fcntl.ioctl(0, termios.TIOCSCTTY)
-        os.execvp(args[0], args)
-        os._exit(1)
+
+
+def tty_create_child(*args):
+    master_fd, slave_fd = os.openpty()
+    mitogen.core.set_block(slave_fd)
+    disable_echo(master_fd)
+    disable_echo(slave_fd)
+
+    proc = subprocess.Popen(
+        args=args,
+        stdin=slave_fd,
+        stdout=slave_fd,
+        stderr=slave_fd,
+        preexec_fn=_acquire_controlling_tty,
+        close_fds=True,
+    )
 
     os.close(slave_fd)
     LOG.debug('tty_create_child() child %d fd %d, parent %d, cmd: %s',
-              pid, master_fd, os.getpid(), Argv(args))
-    return pid, master_fd
+              proc.pid, master_fd, os.getpid(), Argv(args))
+    return proc.pid, master_fd
 
 
 def write_all(fd, s, deadline=None):
@@ -584,7 +587,13 @@ class Stream(mitogen.core.Stream):
     name_prefix = 'local'
 
     def start_child(self):
-        return self.create_child(*self.get_boot_command())
+        args = self.get_boot_command()
+        try:
+            return self.create_child(*args)
+        except OSError:
+            e = sys.exc_info()[1]
+            msg = 'Child start failed: %s. Command was: %s' % (e, Argv(args))
+            raise mitogen.core.StreamError(msg)
 
     def connect(self):
         LOG.debug('%r.connect()', self)

tests/parent_test.py

@@ -9,6 +9,51 @@ import testlib
 import mitogen.parent
 
 
+class StreamErrorTest(testlib.RouterMixin, testlib.TestCase):
+    def test_direct_eof(self):
+        e = self.assertRaises(mitogen.core.StreamError,
+            lambda: self.router.local(
+                python_path='/bin/true',
+                connect_timeout=3,
+            )
+        )
+        self.assertEquals(e.args[0], "EOF on stream; last 300 bytes received: ''")
+
+    def test_via_eof(self):
+        # Verify FD leakage does not keep failed process open.
+        local = self.router.fork()
+        e = self.assertRaises(mitogen.core.StreamError,
+            lambda: self.router.local(
+                via=local,
+                python_path='/bin/true',
+                connect_timeout=3,
+            )
+        )
+        self.assertEquals(e.args[0], "EOF on stream; last 300 bytes received: ''")
+
+    def test_direct_enoent(self):
+        e = self.assertRaises(mitogen.core.StreamError,
+            lambda: self.router.local(
+                python_path='derp',
+                connect_timeout=3,
+            )
+        )
+        prefix = 'Child start failed: [Errno 2] No such file or directory.'
+        self.assertTrue(e.args[0].startswith(prefix))
+
+    def test_via_enoent(self):
+        local = self.router.fork()
+        e = self.assertRaises(mitogen.core.StreamError,
+            lambda: self.router.local(
+                via=local,
+                python_path='derp',
+                connect_timeout=3,
+            )
+        )
+        prefix = 'Child start failed: [Errno 2] No such file or directory.'
+        self.assertTrue(e.args[0].startswith(prefix))
+
+
 class ContextTest(testlib.RouterMixin, unittest2.TestCase):
     def test_context_shutdown(self):
         local = self.router.local()