[Python-checkins] bpo-39010: Fix errors logged on proactor loop restart (GH-22017) (#22035)

Thu Sep 3 15:38:39 EDT 2020

https://github.com/python/cpython/commit/a986b061a3cd4661eb9f857e2936291f1847bd15
commit: a986b061a3cd4661eb9f857e2936291f1847bd15
branch: 3.8
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2020年09月03日T12:38:34-07:00
summary:
bpo-39010: Fix errors logged on proactor loop restart (GH-22017) (#22035)
Stopping and restarting a proactor event loop on windows can lead to
spurious errors logged (ConnectionResetError while reading from the
self pipe). This fixes the issue by ensuring that we don't attempt
to start multiple copies of the self-pipe reading loop.
(cherry picked from commit ea5a6363c3f8cc90b7c0cc573922b10f296073b6)
Co-authored-by: Ben Darnell <ben at bendarnell.com>
Co-authored-by: Ben Darnell <ben at bendarnell.com>
files:
A Misc/NEWS.d/next/Library/2020-08-30-10-24-26.bpo-39010._mzXJW.rst
M Lib/asyncio/proactor_events.py
M Lib/asyncio/windows_events.py
M Lib/test/test_asyncio/test_proactor_events.py
M Lib/test/test_asyncio/test_windows_events.py

diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py
index 830d8edc32f90..54e4deb2c349c 100644
--- a/Lib/asyncio/proactor_events.py
+++ b/Lib/asyncio/proactor_events.py
@@ -766,6 +766,14 @@ def _loop_self_reading(self, f=None):
 try:
 if f is not None:
 f.result() # may raise
+ if self._self_reading_future is not f:
+ # When we scheduled this Future, we assigned it to
+ # _self_reading_future. If it's not there now, something has
+ # tried to cancel the loop while this callback was still in the
+ # queue (see windows_events.ProactorEventLoop.run_forever). In
+ # that case stop here instead of continuing to schedule a new
+ # iteration.
+ return
 f = self._proactor.recv(self._ssock, 4096)
 except exceptions.CancelledError:
 # _close_self_pipe() has been called, stop waiting for data
diff --git a/Lib/asyncio/windows_events.py b/Lib/asyncio/windows_events.py
index 12e87abfbf2ea..a5ad6fc423fbe 100644
--- a/Lib/asyncio/windows_events.py
+++ b/Lib/asyncio/windows_events.py
@@ -318,8 +318,12 @@ def run_forever(self):
 if self._self_reading_future is not None:
 ov = self._self_reading_future._ov
 self._self_reading_future.cancel()
- # self_reading_future was just cancelled so it will never be signalled
- # Unregister it otherwise IocpProactor.close will wait for it forever
+ # self_reading_future was just cancelled so if it hasn't been
+ # finished yet, it never will be (it's possible that it has
+ # already finished and its callback is waiting in the queue,
+ # where it could still happen if the event loop is restarted).
+ # Unregister it otherwise IocpProactor.close will wait for it
+ # forever
 if ov is not None:
 self._proactor._unregister(ov)
 self._self_reading_future = None
diff --git a/Lib/test/test_asyncio/test_proactor_events.py b/Lib/test/test_asyncio/test_proactor_events.py
index 007039a7cdf5d..e4809c303a71d 100644
--- a/Lib/test/test_asyncio/test_proactor_events.py
+++ b/Lib/test/test_asyncio/test_proactor_events.py
@@ -736,6 +736,7 @@ def test_loop_self_reading(self):
 
 def test_loop_self_reading_fut(self):
 fut = mock.Mock()
+ self.loop._self_reading_future = fut
 self.loop._loop_self_reading(fut)
 self.assertTrue(fut.result.called)
 self.proactor.recv.assert_called_with(self.ssock, 4096)
diff --git a/Lib/test/test_asyncio/test_windows_events.py b/Lib/test/test_asyncio/test_windows_events.py
index 6b005702c9be7..33388a87d48f3 100644
--- a/Lib/test/test_asyncio/test_windows_events.py
+++ b/Lib/test/test_asyncio/test_windows_events.py
@@ -211,6 +211,26 @@ def test_wait_for_handle_cancel(self):
 fut.cancel()
 fut.cancel()
 
+ def test_read_self_pipe_restart(self):
+ # Regression test for https://bugs.python.org/issue39010
+ # Previously, restarting a proactor event loop in certain states
+ # would lead to spurious ConnectionResetErrors being logged.
+ self.loop.call_exception_handler = mock.Mock()
+ # Start an operation in another thread so that the self-pipe is used.
+ # This is theoretically timing-dependent (the task in the executor
+ # must complete before our start/stop cycles), but in practice it
+ # seems to work every time.
+ f = self.loop.run_in_executor(None, lambda: None)
+ self.loop.stop()
+ self.loop.run_forever()
+ self.loop.stop()
+ self.loop.run_forever()
+ # If we don't wait for f to complete here, we may get another
+ # warning logged about a thread that didn't shut down cleanly.
+ self.loop.run_until_complete(f)
+ self.loop.close()
+ self.assertFalse(self.loop.call_exception_handler.called)
+
 
 class WinPolicyTests(test_utils.TestCase):
 
diff --git a/Misc/NEWS.d/next/Library/2020-08-30-10-24-26.bpo-39010._mzXJW.rst b/Misc/NEWS.d/next/Library/2020-08-30-10-24-26.bpo-39010._mzXJW.rst
new file mode 100644
index 0000000000000..0d9015b490e4c
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-08-30-10-24-26.bpo-39010._mzXJW.rst
@@ -0,0 +1,2 @@
+Restarting a ``ProactorEventLoop`` on Windows no longer logs spurious
+``ConnectionResetErrors``.
