diff options
| -rw-r--r-- | oslo_concurrency/processutils.py | 25 | ||||
| -rw-r--r-- | oslo_concurrency/tests/unit/test_processutils.py | 39 |
2 files changed, 61 insertions, 3 deletions
diff --git a/oslo_concurrency/processutils.py b/oslo_concurrency/processutils.py index 9e619b9..2900918 100644 --- a/oslo_concurrency/processutils.py +++ b/oslo_concurrency/processutils.py @@ -42,8 +42,20 @@ from oslo_concurrency._i18n import _ # time module as the check because that's a monkey patched module we use # in combination with subprocess below, so they need to match. eventlet = importutils.try_import('eventlet') -if eventlet and eventlet.patcher.is_monkey_patched(time): - from eventlet.green import subprocess +eventlet_patched = eventlet and eventlet.patcher.is_monkey_patched(time) +if eventlet_patched: + if os.name == 'nt': + # subprocess.Popen.communicate will spawn two threads consuming + # stdout/stderr when passing data through stdin. We need to make + # sure that *native* threads will be used as pipes are blocking + # on Windows. + # Recent eventlet versions actually do patch subprocess. + subprocess = eventlet.patcher.original('subprocess') + subprocess.threading = eventlet.patcher.original('threading') + else: + from eventlet.green import subprocess + + from eventlet import tpool else: import subprocess @@ -377,7 +389,14 @@ def execute(*cmd, **kwargs): on_execute(obj) try: - result = obj.communicate(process_input) + # eventlet.green.subprocess is not really greenthread friendly + # on Windows. In order to avoid blocking other greenthreads, + # we have to wrap this call using tpool. + if eventlet_patched and os.name == 'nt': + result = tpool.execute(obj.communicate, + process_input) + else: + result = obj.communicate(process_input) obj.stdin.close() # pylint: disable=E1101 _returncode = obj.returncode # pylint: disable=E1101 diff --git a/oslo_concurrency/tests/unit/test_processutils.py b/oslo_concurrency/tests/unit/test_processutils.py index ae64af9..29b4c65 100644 --- a/oslo_concurrency/tests/unit/test_processutils.py +++ b/oslo_concurrency/tests/unit/test_processutils.py @@ -124,6 +124,45 @@ class UtilsTest(test_base.BaseTestCase): if type(e).__name__ != 'SubprocessError': raise + @mock.patch.object(os, 'name', 'nt') + @mock.patch.object(processutils.subprocess, "Popen") + @mock.patch.object(processutils, 'tpool', create=True) + def _test_windows_execute(self, mock_tpool, mock_popen, + use_eventlet=False): + # We want to ensure that if eventlet is used on Windows, + # 'communicate' calls are wrapped with eventlet.tpool.execute. + mock_comm = mock_popen.return_value.communicate + mock_comm.return_value = None + mock_tpool.execute.return_value = mock_comm.return_value + + fake_pinput = 'fake pinput'.encode('utf-8') + + with mock.patch.object(processutils, 'eventlet_patched', + use_eventlet): + processutils.execute( + TRUE_UTILITY, + process_input=fake_pinput, + check_exit_code=False) + + mock_popen.assert_called_once_with( + [TRUE_UTILITY], + stdin=mock.ANY, stdout=mock.ANY, + stderr=mock.ANY, close_fds=mock.ANY, + preexec_fn=mock.ANY, shell=mock.ANY, + cwd=mock.ANY, env=mock.ANY) + + if use_eventlet: + mock_tpool.execute.assert_called_once_with( + mock_comm, fake_pinput) + else: + mock_comm.assert_called_once_with(fake_pinput) + + def test_windows_execute_without_eventlet(self): + self._test_windows_execute() + + def test_windows_execute_using_eventlet(self): + self._test_windows_execute(use_eventlet=True) + class ProcessExecutionErrorTest(test_base.BaseTestCase): |