[3.6] bpo-32356: idempotent pause_/resume_reading (GH-4914) (GH-7629)

Backport note: don't add new is_reading() method from master to 3.6.

(cherry picked from commit d757aaf9dd767d13205bf9917e520ebf43e7f6e5)

diff --git a/Doc/library/asyncio-protocol.rst b/Doc/library/asyncio-protocol.rst
index cd84ae76b5d86e0fbd6b7bee880e930cda683c6e..9605261c0a3f46cb32d1ede4f8c48154af08d36d 100644 (file)
--- a/Doc/library/asyncio-protocol.rst
+++ b/Doc/library/asyncio-protocol.rst
@@ -124,11 +124,19 @@ ReadTransport
       the protocol's :meth:`data_received` method until :meth:`resume_reading`
       is called.
 
+      .. versionchanged:: 3.6.7
+         The method is idempotent, i.e. it can be called when the
+         transport is already paused or closed.
+
    .. method:: resume_reading()
 
       Resume the receiving end.  The protocol's :meth:`data_received` method
       will be called once again if some data is available for reading.
 
+      .. versionchanged:: 3.6.7
+         The method is idempotent, i.e. it can be called when the
+         transport is already reading.
+
 
 WriteTransport
 --------------

diff --git a/Lib/asyncio/proactor_events.py b/Lib/asyncio/proactor_events.py
index 967a696961a611df944debb1f161c96bf9ace6c6..6f621ef0cc4f188cbaaed1af3671586a732bdce6 100644 (file)
--- a/Lib/asyncio/proactor_events.py
+++ b/Lib/asyncio/proactor_events.py
@@ -160,20 +160,16 @@ class _ProactorReadPipeTransport(_ProactorBasePipeTransport,
         self._loop.call_soon(self._loop_reading)
 
     def pause_reading(self):
-        if self._closing:
-            raise RuntimeError('Cannot pause_reading() when closing')
-        if self._paused:
-            raise RuntimeError('Already paused')
+        if self._closing or self._paused:
+            return
         self._paused = True
         if self._loop.get_debug():
             logger.debug("%r pauses reading", self)
 
     def resume_reading(self):
-        if not self._paused:
-            raise RuntimeError('Not paused')
-        self._paused = False
-        if self._closing:
+        if self._closing or not self._paused:
             return
+        self._paused = False
         if self._reschedule_on_resume:
             self._loop.call_soon(self._loop_reading, self._read_fut)
             self._reschedule_on_resume = False

diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py
index 81dfd7651c5ed0a8432a7c175a5e3d238bcc9726..bc7c740cc2af3826dc7fb0c895aaeca557a92b15 100644 (file)
--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -703,18 +703,16 @@ class _SelectorSocketTransport(_SelectorTransport):
                                  waiter, None)
 
     def pause_reading(self):
-        if self._closing:
-            raise RuntimeError('Cannot pause_reading() when closing')
-        if self._paused:
-            raise RuntimeError('Already paused')
+        if self._closing or self._paused:
+            return
         self._paused = True
         self._loop._remove_reader(self._sock_fd)
         if self._loop.get_debug():
             logger.debug("%r pauses reading", self)
 
     def resume_reading(self):
-        if not self._paused:
-            raise RuntimeError('Not paused')
+        if self._closing or not self._paused:
+            return
         self._paused = False
         self._add_reader(self._sock_fd, self._read_ready)
         if self._loop.get_debug():

diff --git a/Lib/asyncio/test_utils.py b/Lib/asyncio/test_utils.py
index 8b8c22a74757de5efb0379b855953f7590d4962a..f41720428cecd2ef3d23ddcd56c4a1687c2a75ac 100644 (file)
--- a/Lib/asyncio/test_utils.py
+++ b/Lib/asyncio/test_utils.py
@@ -335,12 +335,19 @@ class TestLoop(base_events.BaseEventLoop):
             return False
 
     def assert_reader(self, fd, callback, *args):
-        assert fd in self.readers, 'fd {} is not registered'.format(fd)
+        if fd not in self.readers:
+            raise AssertionError(f'fd {fd} is not registered')
         handle = self.readers[fd]
-        assert handle._callback == callback, '{!r} != {!r}'.format(
-            handle._callback, callback)
-        assert handle._args == args, '{!r} != {!r}'.format(
-            handle._args, args)
+        if handle._callback != callback:
+            raise AssertionError(
+                f'unexpected callback: {handle._callback} != {callback}')
+        if handle._args != args:
+            raise AssertionError(
+                f'unexpected callback args: {handle._args} != {args}')
+
+    def assert_no_reader(self, fd):
+        if fd in self.readers:
+            raise AssertionError(f'fd {fd} is registered')
 
     def _add_writer(self, fd, callback, *args):
         self.writers[fd] = events.Handle(callback, args, self)

diff --git a/Lib/test/test_asyncio/test_proactor_events.py b/Lib/test/test_asyncio/test_proactor_events.py
index edf0461957f8cdd99c5ca234b8b0a15a9f5624ed..ba187c97fed410a733c079437afe18932f385c46 100644 (file)
--- a/Lib/test/test_asyncio/test_proactor_events.py
+++ b/Lib/test/test_asyncio/test_proactor_events.py
@@ -334,6 +334,7 @@ class ProactorSocketTransportTests(test_utils.TestCase):
             f = asyncio.Future(loop=self.loop)
             f.set_result(msg)
             futures.append(f)
+
         self.loop._proactor.recv.side_effect = futures
         self.loop._run_once()
         self.assertFalse(tr._paused)
@@ -341,11 +342,15 @@ class ProactorSocketTransportTests(test_utils.TestCase):
         self.protocol.data_received.assert_called_with(b'data1')
         self.loop._run_once()
         self.protocol.data_received.assert_called_with(b'data2')
+
+        tr.pause_reading()
         tr.pause_reading()
         self.assertTrue(tr._paused)
         for i in range(10):
             self.loop._run_once()
         self.protocol.data_received.assert_called_with(b'data2')
+
+        tr.resume_reading()
         tr.resume_reading()
         self.assertFalse(tr._paused)
         self.loop._run_once()

diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py
index c359b458029db292fbe7c9046e367715dfb40eb2..533d2898e7fc5824260a7b5d2e012fdf0235cd45 100644 (file)
--- a/Lib/test/test_asyncio/test_selector_events.py
+++ b/Lib/test/test_asyncio/test_selector_events.py
@@ -81,6 +81,7 @@ class BaseSelectorEventLoopTests(test_utils.TestCase):
         with test_utils.disable_logger():
             transport = self.loop._make_ssl_transport(
                 m, asyncio.Protocol(), m, waiter)
+
             # execute the handshake while the logger is disabled
             # to ignore SSL handshake failure
             test_utils.run_briefly(self.loop)
@@ -884,14 +885,19 @@ class SelectorSocketTransportTests(test_utils.TestCase):
         test_utils.run_briefly(self.loop)
         self.assertFalse(tr._paused)
         self.loop.assert_reader(7, tr._read_ready)
+
+        tr.pause_reading()
         tr.pause_reading()
         self.assertTrue(tr._paused)
-        self.assertFalse(7 in self.loop.readers)
+        self.loop.assert_no_reader(7)
+
+        tr.resume_reading()
         tr.resume_reading()
         self.assertFalse(tr._paused)
         self.loop.assert_reader(7, tr._read_ready)
-        with self.assertRaises(RuntimeError):
-            tr.resume_reading()
+
+        tr.close()
+        self.loop.assert_no_reader(7)
 
     def test_read_ready(self):
         transport = self.socket_transport()

diff --git a/Misc/NEWS.d/next/Library/2017-12-17-22-50-51.bpo-32356.roZJpA.rst b/Misc/NEWS.d/next/Library/2017-12-17-22-50-51.bpo-32356.roZJpA.rst
new file mode 100644 (file)
index 0000000..95a71de
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-12-17-22-50-51.bpo-32356.roZJpA.rst
@@ -0,0 +1 @@
+asyncio.transport.resume_reading() and pause_reading() are now idempotent.