Commit 77e3f63e authored by Yury Selivanov's avatar Yury Selivanov

Issue #28368: Refuse monitoring processes if the child watcher has no loop attached.

Patch by Vincent Michel.
parent a273afd9
...@@ -746,6 +746,7 @@ class BaseChildWatcher(AbstractChildWatcher): ...@@ -746,6 +746,7 @@ class BaseChildWatcher(AbstractChildWatcher):
def __init__(self): def __init__(self):
self._loop = None self._loop = None
self._callbacks = {}
def close(self): def close(self):
self.attach_loop(None) self.attach_loop(None)
...@@ -759,6 +760,12 @@ class BaseChildWatcher(AbstractChildWatcher): ...@@ -759,6 +760,12 @@ class BaseChildWatcher(AbstractChildWatcher):
def attach_loop(self, loop): def attach_loop(self, loop):
assert loop is None or isinstance(loop, events.AbstractEventLoop) 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: if self._loop is not None:
self._loop.remove_signal_handler(signal.SIGCHLD) self._loop.remove_signal_handler(signal.SIGCHLD)
...@@ -807,10 +814,6 @@ class SafeChildWatcher(BaseChildWatcher): ...@@ -807,10 +814,6 @@ class SafeChildWatcher(BaseChildWatcher):
big number of children (O(n) each time SIGCHLD is raised) big number of children (O(n) each time SIGCHLD is raised)
""" """
def __init__(self):
super().__init__()
self._callbacks = {}
def close(self): def close(self):
self._callbacks.clear() self._callbacks.clear()
super().close() super().close()
...@@ -822,6 +825,11 @@ class SafeChildWatcher(BaseChildWatcher): ...@@ -822,6 +825,11 @@ class SafeChildWatcher(BaseChildWatcher):
pass pass
def add_child_handler(self, pid, callback, *args): 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) self._callbacks[pid] = (callback, args)
# Prevent a race condition in case the child is already terminated. # Prevent a race condition in case the child is already terminated.
...@@ -886,7 +894,6 @@ class FastChildWatcher(BaseChildWatcher): ...@@ -886,7 +894,6 @@ class FastChildWatcher(BaseChildWatcher):
""" """
def __init__(self): def __init__(self):
super().__init__() super().__init__()
self._callbacks = {}
self._lock = threading.Lock() self._lock = threading.Lock()
self._zombies = {} self._zombies = {}
self._forks = 0 self._forks = 0
...@@ -918,6 +925,12 @@ class FastChildWatcher(BaseChildWatcher): ...@@ -918,6 +925,12 @@ class FastChildWatcher(BaseChildWatcher):
def add_child_handler(self, pid, callback, *args): def add_child_handler(self, pid, callback, *args):
assert self._forks, "Must use the context manager" 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: with self._lock:
try: try:
returncode = self._zombies.pop(pid) returncode = self._zombies.pop(pid)
......
...@@ -433,6 +433,13 @@ class SubprocessMixin: ...@@ -433,6 +433,13 @@ class SubprocessMixin:
# the transport was not notified yet # the transport was not notified yet
self.assertFalse(killed) 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): def test_popen_error(self):
# Issue #24763: check that the subprocess transport is closed # Issue #24763: check that the subprocess transport is closed
# when BaseSubprocessTransport fails # when BaseSubprocessTransport fails
......
...@@ -11,6 +11,7 @@ import sys ...@@ -11,6 +11,7 @@ import sys
import tempfile import tempfile
import threading import threading
import unittest import unittest
import warnings
from unittest import mock from unittest import mock
if sys.platform == 'win32': if sys.platform == 'win32':
...@@ -1391,6 +1392,8 @@ class ChildWatcherTestsMixin: ...@@ -1391,6 +1392,8 @@ class ChildWatcherTestsMixin:
with mock.patch.object( with mock.patch.object(
old_loop, "remove_signal_handler") as m_remove_signal_handler: old_loop, "remove_signal_handler") as m_remove_signal_handler:
with self.assertWarnsRegex(
RuntimeWarning, 'A loop is being detached'):
self.watcher.attach_loop(None) self.watcher.attach_loop(None)
m_remove_signal_handler.assert_called_once_with( m_remove_signal_handler.assert_called_once_with(
...@@ -1463,6 +1466,15 @@ class ChildWatcherTestsMixin: ...@@ -1463,6 +1466,15 @@ class ChildWatcherTestsMixin:
if isinstance(self.watcher, asyncio.FastChildWatcher): if isinstance(self.watcher, asyncio.FastChildWatcher):
self.assertFalse(self.watcher._zombies) 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): class SafeChildWatcherTests (ChildWatcherTestsMixin, test_utils.TestCase):
def create_watcher(self): def create_watcher(self):
......
...@@ -346,6 +346,10 @@ Library ...@@ -346,6 +346,10 @@ Library
- Issue #27759: Fix selectors incorrectly retain invalid file descriptors. - Issue #27759: Fix selectors incorrectly retain invalid file descriptors.
Patch by Mark Williams. Patch by Mark Williams.
- Issue #28368: Refuse monitoring processes if the child watcher has
no loop attached.
Patch by Vincent Michel.
IDLE IDLE
---- ----
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment