From 614a20462aebfe85a54ce35a7daaf1a7dbde44a7 Mon Sep 17 00:00:00 2001 From: Jakub Stasiak Date: Fri, 20 May 2016 10:23:36 +0200 Subject: [PATCH] Remove select.poll and improve subprocess green select: Delete unpatched poll once again https://github.com/eventlet/eventlet/pull/317 Previously attempted in f63165c, had to be reverted in 8ea9df6 because subprocess was failing after monkey patching. Turns out we haven't been monkey patching the subprocess module at all, this patch adds that in order for the tests to pass. This part is changed because otherwise Popen class instantiation would cause an infinite loop when monkey patching is applied: -subprocess_orig = __import__("subprocess") +subprocess_orig = patcher.original("subprocess") This patch is contributed by Smarkets Limited. * green subprocess: Provide green check_output This patch is contributed by Smarkets Limited. --- eventlet/green/select.py | 4 +--- eventlet/green/subprocess.py | 17 ++++++++++++++--- eventlet/patcher.py | 8 +++++++- tests/isolated/mysqldb_monkey_patch.py | 10 +++++++++- ...tcher_blocking_select_methods_are_deleted.py | 4 +--- tests/patcher_test.py | 10 +++++----- 6 files changed, 37 insertions(+), 16 deletions(-) diff --git a/eventlet/green/select.py b/eventlet/green/select.py index d1cba12..60301d3 100644 --- a/eventlet/green/select.py +++ b/eventlet/green/select.py @@ -6,9 +6,7 @@ from eventlet.support import six __patched__ = ['select'] -# FIXME: must also delete `poll`, but it breaks subprocess `communicate()` -# https://github.com/eventlet/eventlet/issues/290 -__deleted__ = ['devpoll', 'epoll', 'kqueue', 'kevent'] +__deleted__ = ['devpoll', 'poll', 'epoll', 'kqueue', 'kevent'] def get_fileno(obj): diff --git a/eventlet/green/subprocess.py b/eventlet/green/subprocess.py index 1347404..597dfdf 100644 --- a/eventlet/green/subprocess.py +++ b/eventlet/green/subprocess.py @@ -9,6 +9,7 @@ from eventlet.green import select, threading, time from eventlet.support import six +__patched__ = ['call', 'check_call', 'Popen'] to_patch = [('select', select), ('threading', threading), ('time', time)] if sys.version_info > (3, 4): @@ -16,7 +17,7 @@ if sys.version_info > (3, 4): to_patch.append(('selectors', selectors)) patcher.inject('subprocess', globals(), *to_patch) -subprocess_orig = __import__("subprocess") +subprocess_orig = patcher.original("subprocess") mswindows = sys.platform == "win32" @@ -114,7 +115,17 @@ class Popen(subprocess_orig.Popen): except AttributeError: pass + # Borrow subprocess.call() and check_call(), but patch them so they reference # OUR Popen class rather than subprocess.Popen. -call = FunctionType(six.get_function_code(subprocess_orig.call), globals()) -check_call = FunctionType(six.get_function_code(subprocess_orig.check_call), globals()) +def patched_function(function): + return FunctionType(six.get_function_code(function), globals()) + + +call = patched_function(subprocess_orig.call) +check_call = patched_function(subprocess_orig.check_call) +# check_output is Python 2.7+ +if hasattr(subprocess_orig, 'check_output'): + __patched__.append('check_output') + check_output = patched_function(subprocess_orig.check_output) +del patched_function diff --git a/eventlet/patcher.py b/eventlet/patcher.py index 115fd9b..2b75272 100644 --- a/eventlet/patcher.py +++ b/eventlet/patcher.py @@ -224,7 +224,7 @@ def monkey_patch(**on): """ accepted_args = set(('os', 'select', 'socket', 'thread', 'time', 'psycopg', 'MySQLdb', - 'builtins')) + 'builtins', 'subprocess')) # To make sure only one of them is passed here assert not ('__builtin__' in on and 'builtins' in on) try: @@ -262,6 +262,7 @@ def monkey_patch(**on): ('time', _green_time_modules), ('MySQLdb', _green_MySQLdb), ('builtins', _green_builtins), + ('subprocess', _green_subprocess_modules), ]: if on[name] and not already_patched.get(name): modules_to_patch += modules_function() @@ -401,6 +402,11 @@ def _green_socket_modules(): return [('socket', socket)] +def _green_subprocess_modules(): + from eventlet.green import subprocess + return [('subprocess', subprocess)] + + def _green_thread_modules(): from eventlet.green import Queue from eventlet.green import thread diff --git a/tests/isolated/mysqldb_monkey_patch.py b/tests/isolated/mysqldb_monkey_patch.py index da1e074..01041fe 100644 --- a/tests/isolated/mysqldb_monkey_patch.py +++ b/tests/isolated/mysqldb_monkey_patch.py @@ -6,6 +6,14 @@ if __name__ == '__main__': from eventlet.green import MySQLdb as gm patcher.monkey_patch(all=True, MySQLdb=True) patched_set = set(patcher.already_patched) - set(['psycopg']) - assert patched_set == frozenset(['MySQLdb', 'os', 'select', 'socket', 'thread', 'time']) + assert patched_set == frozenset([ + 'MySQLdb', + 'os', + 'select', + 'socket', + 'subprocess', + 'thread', + 'time', + ]) assert m.connect == gm.connect print('pass') diff --git a/tests/isolated/patcher_blocking_select_methods_are_deleted.py b/tests/isolated/patcher_blocking_select_methods_are_deleted.py index 67da6a5..a761e1e 100644 --- a/tests/isolated/patcher_blocking_select_methods_are_deleted.py +++ b/tests/isolated/patcher_blocking_select_methods_are_deleted.py @@ -11,9 +11,7 @@ if __name__ == '__main__': # * https://bitbucket.org/eventlet/eventlet/issues/167 # * https://github.com/eventlet/eventlet/issues/169 import select - # FIXME: must also delete `poll`, but it breaks subprocess `communicate()` - # https://github.com/eventlet/eventlet/issues/290 - for name in ['devpoll', 'epoll', 'kqueue', 'kevent']: + for name in ['devpoll', 'poll', 'epoll', 'kqueue', 'kevent']: assert not hasattr(select, name), name import sys diff --git a/tests/patcher_test.py b/tests/patcher_test.py index 0e3c0cf..5aae0ac 100644 --- a/tests/patcher_test.py +++ b/tests/patcher_test.py @@ -185,15 +185,15 @@ print("already_patched {0}".format(",".join(sorted(patcher.already_patched.keys( def test_boolean(self): self.assert_boolean_logic("patcher.monkey_patch()", - 'os,select,socket,thread,time') + 'os,select,socket,subprocess,thread,time') def test_boolean_all(self): self.assert_boolean_logic("patcher.monkey_patch(all=True)", - 'os,select,socket,thread,time') + 'os,select,socket,subprocess,thread,time') def test_boolean_all_single(self): self.assert_boolean_logic("patcher.monkey_patch(all=True, socket=True)", - 'os,select,socket,thread,time') + 'os,select,socket,subprocess,thread,time') def test_boolean_all_negative(self): self.assert_boolean_logic( @@ -210,11 +210,11 @@ print("already_patched {0}".format(",".join(sorted(patcher.already_patched.keys( def test_boolean_negative(self): self.assert_boolean_logic("patcher.monkey_patch(socket=False)", - 'os,select,thread,time') + 'os,select,subprocess,thread,time') def test_boolean_negative2(self): self.assert_boolean_logic("patcher.monkey_patch(socket=False, time=False)", - 'os,select,thread') + 'os,select,subprocess,thread') def test_conflicting_specifications(self): self.assert_boolean_logic("patcher.monkey_patch(socket=False, select=True)",