Skip to content
This repository has been archived by the owner on Nov 23, 2017. It is now read-only.

Commit

Permalink
Refuse monitoring processes if the child watcher has no loop attached
Browse files Browse the repository at this point in the history
PR #391.
  • Loading branch information
Vincent Michel authored and 1st1 committed Oct 5, 2016
1 parent 3a8fc97 commit 496f60f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 6 deletions.
23 changes: 18 additions & 5 deletions asyncio/unix_events.py
Expand Up @@ -746,6 +746,7 @@ class BaseChildWatcher(AbstractChildWatcher):

def __init__(self):
self._loop = None
self._callbacks = {}

def close(self):
self.attach_loop(None)
Expand All @@ -759,6 +760,12 @@ def _do_waitpid_all(self):
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)

Expand Down Expand Up @@ -807,10 +814,6 @@ class SafeChildWatcher(BaseChildWatcher):
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()
Expand All @@ -822,6 +825,11 @@ def __exit__(self, a, b, c):
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.
Expand Down Expand Up @@ -886,7 +894,6 @@ class FastChildWatcher(BaseChildWatcher):
"""
def __init__(self):
super().__init__()
self._callbacks = {}
self._lock = threading.Lock()
self._zombies = {}
self._forks = 0
Expand Down Expand Up @@ -918,6 +925,12 @@ def __exit__(self, a, b, c):

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)
Expand Down
7 changes: 7 additions & 0 deletions tests/test_subprocess.py
Expand Up @@ -433,6 +433,13 @@ def kill_running():
# 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
Expand Down
14 changes: 13 additions & 1 deletion tests/test_unix_events.py
Expand Up @@ -11,6 +11,7 @@
import tempfile
import threading
import unittest
import warnings
from unittest import mock

if sys.platform == 'win32':
Expand Down Expand Up @@ -1391,7 +1392,9 @@ def test_set_loop_race_condition(self, m):
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)
Expand Down Expand Up @@ -1463,6 +1466,15 @@ def test_close(self, m):
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):
Expand Down

0 comments on commit 496f60f

Please sign in to comment.