From c6284e00e99b36e88b888e70aa1dcd70af49feaf Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 18 Apr 2018 16:32:06 +0000 Subject: [PATCH] Use subprocess to start child processes; closes #185. --- mitogen/parent.py | 79 ++++++++++++++++++++++++-------------------- tests/parent_test.py | 45 +++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 35 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 95cbbc14..20ab7a39 100644 --- a/mitogen/parent.py +++ b/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) - + # 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()) + + 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 _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(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) 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) - 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() - 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)) - 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) + 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) diff --git a/tests/parent_test.py b/tests/parent_test.py index da9f5e15..74fc0b6f 100644 --- a/tests/parent_test.py +++ b/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()