Merge pull request #592 from gevent/patch-subprocess-by-default

Patch subprocess by default. Fixes #466.

changelog.rst

@@ -41,6 +41,7 @@ Unreleased
   return True, supporting its use-case as an "infinite" or unbounded
   semaphore providing no exclusing, and allowing the idiom ``if
   sem.acquire(): ...``. PR #544 by Mouad Benchchaoui.
+- Patch ``subprocess`` by default in ``gevent.monkey.patch_all``. See #446.
 
 Release 1.0.2
 -------------

gevent/monkey.py

@@ -235,7 +235,7 @@ def patch_subprocess():
 
 
 def patch_all(socket=True, dns=True, time=True, select=True, thread=True, os=True, ssl=True, httplib=False,
-              subprocess=False, sys=False, aggressive=True, Event=False):
+              subprocess=True, sys=False, aggressive=True, Event=False):
     """Do all of the default monkey patching (calls every other function in this module."""
     # order is important
     if os:

gevent/subprocess.py

@@ -307,8 +307,8 @@ class Popen(object):
             self.stderr.close()
 
         self.wait()
-        return (None if stdout is None else stdout.value or '',
-                None if stderr is None else stderr.value or '')
+        return (None if stdout is None else stdout.value or b'',
+                None if stderr is None else stderr.value or b'')
 
     def poll(self):
         return self._internal_poll()

greentest/patched_tests_setup.py

@@ -167,6 +167,31 @@ if sys.platform == 'darwin':
         # causes Mac OS X to show "Python crashes" dialog box which is annoying
     ]
 
+if hasattr(sys, 'pypy_version_info'):
+    disabled_tests += [
+        'test_subprocess.POSIXProcessTestCase.test_terminate_dead',
+        'test_subprocess.POSIXProcessTestCase.test_send_signal_dead',
+        'test_subprocess.POSIXProcessTestCase.test_kill_dead',
+        # Don't exist in the CPython test suite; with our monkey patch in place,
+        # they fail because the process they're looking for has been allowed to exit.
+        # Our monkey patch waits for the process with a watcher and so detects
+        # the exit before the normal polling mechanism would
+
+        'test_subprocess.POSIXProcessTestCase.test_preexec_errpipe_does_not_double_close_pipes',
+        # Does not exist in the CPython test suite. Subclasses Popen, and overrides
+        # _execute_child. But our version has a different parameter list than the
+        # version that comes with PyPy, so fails with a TypeError.
+
+        'test_subprocess.ProcessTestCase.test_failed_child_execute_fd_leak',
+        # Does not exist in the CPython test suite, tests for a specific bug
+        # in PyPy's forking. Only runs on linux and is specific to the PyPy
+        # implementation of subprocess (possibly explains the extra parameter to
+        # _execut_child)
+
+        'test_signal.InterProcessSignalTests.test_main',
+        # Fails to get the signal to the correct handler due to
+        # https://bitbucket.org/cffi/cffi/issue/152/handling-errors-from-signal-handlers-in
+    ]
 
 # if 'signalfd' in os.environ.get('GEVENT_BACKEND', ''):
 #     # tests that don't interact well with signalfd

known_failures.py

@@ -76,6 +76,7 @@ if PYPY:
 
         # check_sendall_interrupted and testInterruptedTimeout fail due to
         # https://bitbucket.org/cffi/cffi/issue/152/handling-errors-from-signal-handlers-in
+        # See also patched_tests_setup and 'test_signal.InterProcessSignalTests.test_main'
         'test_socket.py',
     ]