[Python-checkins] cpython (3.5): Issue #28368: Refuse monitoring processes if the child watcher has no loop

[yury.selivanov]

https://hg.python.org/cpython/rev/2110dcef5892
changeset:   104299:2110dcef5892
branch:      3.5
parent:      104292:fa09ba71babb
user:        Yury Selivanov <yury at magic.io>
date:        Wed Oct 05 16:57:12 2016 -0400
summary:
  Issue #28368: Refuse monitoring processes if the child watcher has no loop attached.

Patch by Vincent Michel.

files:
  Lib/asyncio/unix_events.py                |  23 ++++++++--
  Lib/test/test_asyncio/test_subprocess.py  |   7 +++
  Lib/test/test_asyncio/test_unix_events.py |  14 ++++++-
  Misc/NEWS                                 |   4 +
  4 files changed, 42 insertions(+), 6 deletions(-)


diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
--- a/Lib/asyncio/unix_events.py
+++ b/Lib/asyncio/unix_events.py
@@ -746,6 +746,7 @@
 
     def __init__(self):
         self._loop = None
+        self._callbacks = {}
 
     def close(self):
         self.attach_loop(None)
@@ -759,6 +760,12 @@
     def attach_loop(self, loop):
         assert loop is None or isinstance(loop, events.AbstractEventLoop)
 
+        if self._loop is not None and loop is None and self._callbacks:
+            warnings.warn(
+                'A loop is being detached '
+                'from a child watcher with pending handlers',
+                RuntimeWarning)
+
         if self._loop is not None:
             self._loop.remove_signal_handler(signal.SIGCHLD)
 
@@ -807,10 +814,6 @@
     big number of children (O(n) each time SIGCHLD is raised)
     """
 
-    def __init__(self):
-        super().__init__()
-        self._callbacks = {}
-
     def close(self):
         self._callbacks.clear()
         super().close()
@@ -822,6 +825,11 @@
         pass
 
     def add_child_handler(self, pid, callback, *args):
+        if self._loop is None:
+            raise RuntimeError(
+                "Cannot add child handler, "
+                "the child watcher does not have a loop attached")
+
         self._callbacks[pid] = (callback, args)
 
         # Prevent a race condition in case the child is already terminated.
@@ -886,7 +894,6 @@
     """
     def __init__(self):
         super().__init__()
-        self._callbacks = {}
         self._lock = threading.Lock()
         self._zombies = {}
         self._forks = 0
@@ -918,6 +925,12 @@
 
     def add_child_handler(self, pid, callback, *args):
         assert self._forks, "Must use the context manager"
+
+        if self._loop is None:
+            raise RuntimeError(
+                "Cannot add child handler, "
+                "the child watcher does not have a loop attached")
+
         with self._lock:
             try:
                 returncode = self._zombies.pop(pid)
diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py
--- a/Lib/test/test_asyncio/test_subprocess.py
+++ b/Lib/test/test_asyncio/test_subprocess.py
@@ -433,6 +433,13 @@
         # the transport was not notified yet
         self.assertFalse(killed)
 
+        # Unlike SafeChildWatcher, FastChildWatcher does not pop the
+        # callbacks if waitpid() is called elsewhere. Let's clear them
+        # manually to avoid a warning when the watcher is detached.
+        if sys.platform != 'win32' and \
+           isinstance(self, SubprocessFastWatcherTests):
+            asyncio.get_child_watcher()._callbacks.clear()
+
     def test_popen_error(self):
         # Issue #24763: check that the subprocess transport is closed
         # when BaseSubprocessTransport fails
diff --git a/Lib/test/test_asyncio/test_unix_events.py b/Lib/test/test_asyncio/test_unix_events.py
--- a/Lib/test/test_asyncio/test_unix_events.py
+++ b/Lib/test/test_asyncio/test_unix_events.py
@@ -11,6 +11,7 @@
 import tempfile
 import threading
 import unittest
+import warnings
 from unittest import mock
 
 if sys.platform == 'win32':
@@ -1391,7 +1392,9 @@
         with mock.patch.object(
                 old_loop, "remove_signal_handler") as m_remove_signal_handler:
 
-            self.watcher.attach_loop(None)
+            with self.assertWarnsRegex(
+                    RuntimeWarning, 'A loop is being detached'):
+                self.watcher.attach_loop(None)
 
             m_remove_signal_handler.assert_called_once_with(
                 signal.SIGCHLD)
@@ -1463,6 +1466,15 @@
                 if isinstance(self.watcher, asyncio.FastChildWatcher):
                     self.assertFalse(self.watcher._zombies)
 
+    @waitpid_mocks
+    def test_add_child_handler_with_no_loop_attached(self, m):
+        callback = mock.Mock()
+        with self.create_watcher() as watcher:
+            with self.assertRaisesRegex(
+                    RuntimeError,
+                    'the child watcher does not have a loop attached'):
+                watcher.add_child_handler(100, callback)
+
 
 class SafeChildWatcherTests (ChildWatcherTestsMixin, test_utils.TestCase):
     def create_watcher(self):
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -346,6 +346,10 @@
 - Issue #27759: Fix selectors incorrectly retain invalid file descriptors.
   Patch by Mark Williams.
 
+- Issue #28368: Refuse monitoring processes if the child watcher has 
+  no loop attached.
+  Patch by Vincent Michel.
+
 IDLE
 ----
 

-- 
Repository URL: https://hg.python.org/cpython