From dcc2047ba123f5fab6b17f8034500aa520d0b0f2 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Wed, 12 Dec 2018 16:26:22 -0800 Subject: [PATCH 01/11] fix for bpo23057: add loop self socket as wakeup fd for signals --- Lib/asyncio/proactor_events.py | 3 +++ .../next/Library/2018-12-12-16-24-55.bpo-23057.OB4Z1Y.rst | 1 + 2 files changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2018-12-12-16-24-55.bpo-23057.OB4Z1Y.rst diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index 69d96a81035e04..53d57b87e00b23 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -10,6 +10,7 @@ import os import socket import warnings +import signal from . import base_events from . import constants @@ -489,6 +490,7 @@ def __init__(self, proactor): self._accept_futures = {} # socket file descriptor => Future proactor.set_loop(self) self._make_self_pipe() + signal.set_wakeup_fd(self._csock.fileno()) def _make_socket_transport(self, sock, protocol, waiter=None, extra=None, server=None): @@ -529,6 +531,7 @@ def close(self): if self.is_closed(): return + signal.set_wakeup_fd(-1) # Call these methods before closing the event loop (before calling # BaseEventLoop.close), because they can schedule callbacks with # call_soon(), which is forbidden when the event loop is closed. diff --git a/Misc/NEWS.d/next/Library/2018-12-12-16-24-55.bpo-23057.OB4Z1Y.rst b/Misc/NEWS.d/next/Library/2018-12-12-16-24-55.bpo-23057.OB4Z1Y.rst new file mode 100644 index 00000000000000..51b727db3042b8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-12-12-16-24-55.bpo-23057.OB4Z1Y.rst @@ -0,0 +1 @@ +Unblock Proactor event loop when keyboard interrupt is received on Windows From 2e8486cbf16b9631c77de5f1b7b09d20cbfd92b5 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Wed, 12 Dec 2018 16:59:52 -0800 Subject: [PATCH 02/11] ensure fd is int --- Lib/asyncio/proactor_events.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index 53d57b87e00b23..f8c4f8762c4820 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -490,7 +490,9 @@ def __init__(self, proactor): self._accept_futures = {} # socket file descriptor => Future proactor.set_loop(self) self._make_self_pipe() - signal.set_wakeup_fd(self._csock.fileno()) + self_no = self._csock.fileno() + if isinstance(self_no, int): + signal.set_wakeup_fd(self_no) def _make_socket_transport(self, sock, protocol, waiter=None, extra=None, server=None): From 6f3ce857034f332cc070edd4bd948baf5d0fdbaa Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Fri, 14 Dec 2018 13:33:29 -0800 Subject: [PATCH 03/11] test: ProactorEventLoop is interrupted with Ctrl-C --- Lib/test/test_asyncio/test_windows_events.py | 44 +++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_windows_events.py b/Lib/test/test_asyncio/test_windows_events.py index 8f4c50e2c92b45..acc8a0601a1b86 100644 --- a/Lib/test/test_asyncio/test_windows_events.py +++ b/Lib/test/test_asyncio/test_windows_events.py @@ -1,6 +1,9 @@ import os +import signal import socket import sys +import subprocess +import time import unittest from unittest import mock @@ -13,7 +16,7 @@ import asyncio from asyncio import windows_events from test.test_asyncio import utils as test_utils - +from test.support.script_helper import spawn_python def tearDownModule(): asyncio.set_event_loop_policy(None) @@ -33,6 +36,45 @@ def data_received(self, data): self.trans.close() +class ProactorLoopCtrlC(test_utils.TestCase): + def test_ctrl_c(self): + code = """if 1: + import sys + import asyncio + asyncio.set_event_loop_policy( + asyncio.WindowsProactorEventLoopPolicy()) + l = asyncio.get_event_loop() + try: + print("start") + sys.stdout.flush() + l.run_forever() + except KeyboardInterrupt: + # ok case + sys.exit(255) + except: + pass + """ + prev_handler = None + with spawn_python("-c", code) as p: + try: + ready = p.stdout.readline() + self.assertEqual(ready, b"start\r\n") + # set IGN handler in a current process to avoid being interrupted + prev_handler = signal.signal(signal.SIGINT, signal.SIG_IGN) + # wait until child gets to a run_forever state + time.sleep(1) + p.send_signal(signal.CTRL_C_EVENT) + exit_code = p.wait(timeout=5) + self.assertEqual(exit_code, 255) + except: + p.kill() + raise + finally: + # restore old SIGINT handler + if prev_handler is not None: + signal.signal(signal.SIGINT, prev_handler) + + class ProactorTests(test_utils.TestCase): def setUp(self): From b9ddb400fd9cc4a0035ce59f53920063d47afcd7 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Sun, 16 Dec 2018 20:49:32 -0800 Subject: [PATCH 04/11] use dedicated process with new console to avoid interrupting other subprocesses --- .../test_ctrl_c_in_proactor_loop_helper.py | 47 +++++++++++++++++++ Lib/test/test_asyncio/test_windows_events.py | 36 +++----------- 2 files changed, 54 insertions(+), 29 deletions(-) create mode 100644 Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py diff --git a/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py b/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py new file mode 100644 index 00000000000000..ddf8056a6bba5f --- /dev/null +++ b/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py @@ -0,0 +1,47 @@ +import sys + + +def do_in_child_process(): + import asyncio + asyncio.set_event_loop_policy( + asyncio.WindowsProactorEventLoopPolicy()) + l = asyncio.get_event_loop() + try: + print("start") + sys.stdout.flush() + l.run_forever() + except KeyboardInterrupt: + # ok + sys.exit(255) + except: + # error - use default exit code + pass + + +def do_in_main_process(): + import os + import signal + import subprocess + from test.support.script_helper import spawn_python + + ok = False + with spawn_python(__file__, "--child") as p: + try: + ready = p.stdout.readline() + if ready != b"start\r\n": + raise Exception(f"Unexpected line: got {ready}, expected 'start'") + # ignore ctrl-c in current process + signal.signal(signal.SIGINT, signal.SIG_IGN) + os.kill(p.pid, signal.CTRL_C_EVENT) + exit_code = p.wait(timeout=5) + ok = exit_code = 255 + except Exception as e: + sys.stderr.write(repr(e)) + p.kill() + sys.exit(255 if ok else 1) + +if __name__ == "__main__": + if len(sys.argv) == 1: + do_in_main_process() + else: + do_in_child_process() \ No newline at end of file diff --git a/Lib/test/test_asyncio/test_windows_events.py b/Lib/test/test_asyncio/test_windows_events.py index acc8a0601a1b86..98d2129831d6ee 100644 --- a/Lib/test/test_asyncio/test_windows_events.py +++ b/Lib/test/test_asyncio/test_windows_events.py @@ -38,41 +38,19 @@ def data_received(self, data): class ProactorLoopCtrlC(test_utils.TestCase): def test_ctrl_c(self): - code = """if 1: - import sys - import asyncio - asyncio.set_event_loop_policy( - asyncio.WindowsProactorEventLoopPolicy()) - l = asyncio.get_event_loop() - try: - print("start") - sys.stdout.flush() - l.run_forever() - except KeyboardInterrupt: - # ok case - sys.exit(255) - except: - pass - """ - prev_handler = None - with spawn_python("-c", code) as p: + from .test_ctrl_c_in_proactor_loop_helper import __file__ as f + + # ctrl-c will be sent to all processes that share the same console + # in order to isolate the effect of raising ctrl-c we'll create + # a process with a new console + flags = subprocess.CREATE_NEW_CONSOLE + with spawn_python(f, creationflags=flags) as p: try: - ready = p.stdout.readline() - self.assertEqual(ready, b"start\r\n") - # set IGN handler in a current process to avoid being interrupted - prev_handler = signal.signal(signal.SIGINT, signal.SIG_IGN) - # wait until child gets to a run_forever state - time.sleep(1) - p.send_signal(signal.CTRL_C_EVENT) exit_code = p.wait(timeout=5) self.assertEqual(exit_code, 255) except: p.kill() raise - finally: - # restore old SIGINT handler - if prev_handler is not None: - signal.signal(signal.SIGINT, prev_handler) class ProactorTests(test_utils.TestCase): From 846069cbf37c36ce04b4a0bb876bdaae3b7a5b33 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Sun, 16 Dec 2018 21:04:31 -0800 Subject: [PATCH 05/11] formatting --- .../test_asyncio/test_ctrl_c_in_proactor_loop_helper.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py b/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py index ddf8056a6bba5f..91f19e525fde27 100644 --- a/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py +++ b/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py @@ -3,8 +3,8 @@ def do_in_child_process(): import asyncio - asyncio.set_event_loop_policy( - asyncio.WindowsProactorEventLoopPolicy()) + + asyncio.set_event_loop_policy(asyncio.WindowsProactorEventLoopPolicy()) l = asyncio.get_event_loop() try: print("start") @@ -23,7 +23,7 @@ def do_in_main_process(): import signal import subprocess from test.support.script_helper import spawn_python - + ok = False with spawn_python(__file__, "--child") as p: try: @@ -40,8 +40,9 @@ def do_in_main_process(): p.kill() sys.exit(255 if ok else 1) + if __name__ == "__main__": if len(sys.argv) == 1: do_in_main_process() else: - do_in_child_process() \ No newline at end of file + do_in_child_process() From d1c370ba52826c40f76e4593db3ccccc58f446c8 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Sun, 16 Dec 2018 21:13:03 -0800 Subject: [PATCH 06/11] formatting --- Lib/test/test_asyncio/test_windows_events.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_asyncio/test_windows_events.py b/Lib/test/test_asyncio/test_windows_events.py index 98d2129831d6ee..fd2f32e1c67adc 100644 --- a/Lib/test/test_asyncio/test_windows_events.py +++ b/Lib/test/test_asyncio/test_windows_events.py @@ -18,6 +18,7 @@ from test.test_asyncio import utils as test_utils from test.support.script_helper import spawn_python + def tearDownModule(): asyncio.set_event_loop_policy(None) From 949914b1d1cde81d2486cc37b332d6cb9773cf7e Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Sun, 16 Dec 2018 21:21:34 -0800 Subject: [PATCH 07/11] formatting --- Lib/test/test_asyncio/test_windows_events.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_windows_events.py b/Lib/test/test_asyncio/test_windows_events.py index fd2f32e1c67adc..05d875ac3c8523 100644 --- a/Lib/test/test_asyncio/test_windows_events.py +++ b/Lib/test/test_asyncio/test_windows_events.py @@ -40,7 +40,7 @@ def data_received(self, data): class ProactorLoopCtrlC(test_utils.TestCase): def test_ctrl_c(self): from .test_ctrl_c_in_proactor_loop_helper import __file__ as f - + # ctrl-c will be sent to all processes that share the same console # in order to isolate the effect of raising ctrl-c we'll create # a process with a new console From 021ad1f2a182090085ecfafe0f2e16f4c3f370c1 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 18 Dec 2018 00:04:48 -0800 Subject: [PATCH 08/11] ensure that self reading is started when calling run_forever --- Lib/asyncio/windows_events.py | 16 +++++++ .../test_ctrl_c_in_proactor_loop_helper.py | 43 +++++++++++++------ 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/Lib/asyncio/windows_events.py b/Lib/asyncio/windows_events.py index 772ddf4dfebd51..d6f8ee28c6c3da 100644 --- a/Lib/asyncio/windows_events.py +++ b/Lib/asyncio/windows_events.py @@ -308,6 +308,22 @@ def __init__(self, proactor=None): proactor = IocpProactor() super().__init__(proactor) + def run_forever(self): + try: + # if _self_reading_future is cancelled - + # this might indicate that event loop was interrupted before + # and self-reading routine is not hooked up now and needs + # to be restarted + if (self._self_reading_future is not None and + self._self_reading_future.cancelled()): + self._self_reading_future = None + self.call_soon(self._loop_self_reading) + super().run_forever() + except KeyboardInterrupt: + if self._self_reading_future is not None: + self._self_reading_future.cancel() + raise + async def create_pipe_connection(self, protocol_factory, address): f = self._proactor.connect_pipe(address) pipe = await f diff --git a/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py b/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py index 91f19e525fde27..9aeb58aae21280 100644 --- a/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py +++ b/Lib/test/test_asyncio/test_ctrl_c_in_proactor_loop_helper.py @@ -6,33 +6,48 @@ def do_in_child_process(): asyncio.set_event_loop_policy(asyncio.WindowsProactorEventLoopPolicy()) l = asyncio.get_event_loop() - try: - print("start") - sys.stdout.flush() - l.run_forever() - except KeyboardInterrupt: - # ok - sys.exit(255) - except: - # error - use default exit code - pass + + def step(n): + try: + print(n) + sys.stdout.flush() + l.run_forever() + sys.exit(100) + except KeyboardInterrupt: + # ok + pass + except: + # error - use default exit code + sys.exit(200) + + step(1) + step(2) + sys.exit(255) def do_in_main_process(): import os import signal import subprocess + import time from test.support.script_helper import spawn_python ok = False + + def step(p, expected): + s = p.stdout.readline() + if s != expected: + raise Exception(f"Unexpected line: got {s}, expected '{expected}'") + # ensure that child process gets to run_forever + time.sleep(0.5) + os.kill(p.pid, signal.CTRL_C_EVENT) + with spawn_python(__file__, "--child") as p: try: - ready = p.stdout.readline() - if ready != b"start\r\n": - raise Exception(f"Unexpected line: got {ready}, expected 'start'") # ignore ctrl-c in current process signal.signal(signal.SIGINT, signal.SIG_IGN) - os.kill(p.pid, signal.CTRL_C_EVENT) + step(p, b"1\r\n") + step(p, b"2\r\n") exit_code = p.wait(timeout=5) ok = exit_code = 255 except Exception as e: From ecba558d1f811d62640c241d142b2d5914fba159 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 18 Dec 2018 00:18:06 -0800 Subject: [PATCH 09/11] formatting --- Lib/asyncio/windows_events.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/asyncio/windows_events.py b/Lib/asyncio/windows_events.py index d6f8ee28c6c3da..c59b3776b07708 100644 --- a/Lib/asyncio/windows_events.py +++ b/Lib/asyncio/windows_events.py @@ -312,9 +312,9 @@ def run_forever(self): try: # if _self_reading_future is cancelled - # this might indicate that event loop was interrupted before - # and self-reading routine is not hooked up now and needs + # and self-reading routine is not hooked up now and needs # to be restarted - if (self._self_reading_future is not None and + if (self._self_reading_future is not None and self._self_reading_future.cancelled()): self._self_reading_future = None self.call_soon(self._loop_self_reading) From 7473050efea9969c189e6b80853c0da25a1bc137 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 18 Dec 2018 11:51:40 -0800 Subject: [PATCH 10/11] remove isinstance(int) check for fd --- Lib/asyncio/proactor_events.py | 3 +-- Lib/test/test_asyncio/test_proactor_events.py | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index f8c4f8762c4820..2eca93ee4247c8 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -491,8 +491,7 @@ def __init__(self, proactor): proactor.set_loop(self) self._make_self_pipe() self_no = self._csock.fileno() - if isinstance(self_no, int): - signal.set_wakeup_fd(self_no) + signal.set_wakeup_fd(self_no) def _make_socket_transport(self, sock, protocol, waiter=None, extra=None, server=None): diff --git a/Lib/test/test_asyncio/test_proactor_events.py b/Lib/test/test_asyncio/test_proactor_events.py index afc4c19a96b0b1..24ccdfc7d1e8c5 100644 --- a/Lib/test/test_asyncio/test_proactor_events.py +++ b/Lib/test/test_asyncio/test_proactor_events.py @@ -737,7 +737,8 @@ def setUp(self): with mock.patch('asyncio.proactor_events.socket.socketpair', return_value=(self.ssock, self.csock)): - self.loop = BaseProactorEventLoop(self.proactor) + with mock.patch('signal.set_wakeup_fd'): + self.loop = BaseProactorEventLoop(self.proactor) self.set_event_loop(self.loop) @mock.patch.object(BaseProactorEventLoop, 'call_soon') @@ -745,7 +746,8 @@ def setUp(self): def test_ctor(self, socketpair, call_soon): ssock, csock = socketpair.return_value = ( mock.Mock(), mock.Mock()) - loop = BaseProactorEventLoop(self.proactor) + with mock.patch('signal.set_wakeup_fd'): + loop = BaseProactorEventLoop(self.proactor) self.assertIs(loop._ssock, ssock) self.assertIs(loop._csock, csock) self.assertEqual(loop._internal_fds, 1) From 436f56ad29200bd5a81fe95fbeb1a0732bb2deaa Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 18 Dec 2018 13:19:13 -0800 Subject: [PATCH 11/11] start reading from self-socket at the beginning of run_forever, cancel self_reading_future afterwards --- Lib/asyncio/proactor_events.py | 1 - Lib/asyncio/windows_events.py | 14 ++++---------- Lib/test/test_asyncio/test_proactor_events.py | 4 +--- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py index 2eca93ee4247c8..9b9e0aa7c7f1e2 100644 --- a/Lib/asyncio/proactor_events.py +++ b/Lib/asyncio/proactor_events.py @@ -617,7 +617,6 @@ def _make_self_pipe(self): self._ssock.setblocking(False) self._csock.setblocking(False) self._internal_fds += 1 - self.call_soon(self._loop_self_reading) def _loop_self_reading(self, f=None): try: diff --git a/Lib/asyncio/windows_events.py b/Lib/asyncio/windows_events.py index c59b3776b07708..33ffaf97177069 100644 --- a/Lib/asyncio/windows_events.py +++ b/Lib/asyncio/windows_events.py @@ -310,19 +310,13 @@ def __init__(self, proactor=None): def run_forever(self): try: - # if _self_reading_future is cancelled - - # this might indicate that event loop was interrupted before - # and self-reading routine is not hooked up now and needs - # to be restarted - if (self._self_reading_future is not None and - self._self_reading_future.cancelled()): - self._self_reading_future = None - self.call_soon(self._loop_self_reading) + assert self._self_reading_future is None + self.call_soon(self._loop_self_reading) super().run_forever() - except KeyboardInterrupt: + finally: if self._self_reading_future is not None: self._self_reading_future.cancel() - raise + self._self_reading_future = None async def create_pipe_connection(self, protocol_factory, address): f = self._proactor.connect_pipe(address) diff --git a/Lib/test/test_asyncio/test_proactor_events.py b/Lib/test/test_asyncio/test_proactor_events.py index 24ccdfc7d1e8c5..5952ccccce0e5d 100644 --- a/Lib/test/test_asyncio/test_proactor_events.py +++ b/Lib/test/test_asyncio/test_proactor_events.py @@ -741,9 +741,8 @@ def setUp(self): self.loop = BaseProactorEventLoop(self.proactor) self.set_event_loop(self.loop) - @mock.patch.object(BaseProactorEventLoop, 'call_soon') @mock.patch('asyncio.proactor_events.socket.socketpair') - def test_ctor(self, socketpair, call_soon): + def test_ctor(self, socketpair): ssock, csock = socketpair.return_value = ( mock.Mock(), mock.Mock()) with mock.patch('signal.set_wakeup_fd'): @@ -751,7 +750,6 @@ def test_ctor(self, socketpair, call_soon): self.assertIs(loop._ssock, ssock) self.assertIs(loop._csock, csock) self.assertEqual(loop._internal_fds, 1) - call_soon.assert_called_with(loop._loop_self_reading) loop.close() def test_close_self_pipe(self):