Skip to content

Prohibit adding a signal handler for SIGCHLD #156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,6 @@ loop policy:
import uvloop
asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())

Alternatively, you can create an instance of the loop
manually, using:

.. code:: python

loop = uvloop.new_event_loop()
asyncio.set_event_loop(loop)


Development of uvloop
---------------------
Expand Down
106 changes: 54 additions & 52 deletions tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -461,56 +461,6 @@ def handler(loop, context):
self.mock_pattern('Unhandled error in exception handler'),
exc_info=mock.ANY)

def test_default_exc_handler_broken(self):
logger = logging.getLogger('asyncio')
_context = None

class Loop(uvloop.Loop):

_selector = mock.Mock()
_process_events = mock.Mock()

def default_exception_handler(self, context):
nonlocal _context
_context = context
# Simulates custom buggy "default_exception_handler"
raise ValueError('spam')

loop = Loop()
self.addCleanup(loop.close)
asyncio.set_event_loop(loop)

def run_loop():
def zero_error():
loop.stop()
1 / 0
loop.call_soon(zero_error)
loop.run_forever()

with mock.patch.object(logger, 'error') as log:
run_loop()
log.assert_called_with(
'Exception in default exception handler',
exc_info=True)

def custom_handler(loop, context):
raise ValueError('ham')

_context = None
loop.set_exception_handler(custom_handler)
with mock.patch.object(logger, 'error') as log:
run_loop()
log.assert_called_with(
self.mock_pattern('Exception in default exception.*'
'while handling.*in custom'),
exc_info=True)

# Check that original context was passed to default
# exception handler.
self.assertIn('context', _context)
self.assertIs(type(_context['context']['exception']),
ZeroDivisionError)

def test_set_task_factory_invalid(self):
with self.assertRaisesRegex(
TypeError,
Expand Down Expand Up @@ -663,7 +613,7 @@ def test_loop_create_future(self):
fut.cancel()

def test_loop_call_soon_handle_cancelled(self):
cb = lambda: False
cb = lambda: False # NoQA
handle = self.loop.call_soon(cb)
self.assertFalse(handle.cancelled())
handle.cancel()
Expand All @@ -675,7 +625,7 @@ def test_loop_call_soon_handle_cancelled(self):
self.assertFalse(handle.cancelled())

def test_loop_call_later_handle_cancelled(self):
cb = lambda: False
cb = lambda: False # NoQA
handle = self.loop.call_later(0.01, cb)
self.assertFalse(handle.cancelled())
handle.cancel()
Expand All @@ -692,6 +642,58 @@ def test_loop_std_files_cloexec(self):
flags = fcntl.fcntl(fd, fcntl.F_GETFD)
self.assertFalse(flags & fcntl.FD_CLOEXEC)

def test_default_exc_handler_broken(self):
logger = logging.getLogger('asyncio')
_context = None

class Loop(uvloop.Loop):

_selector = mock.Mock()
_process_events = mock.Mock()

def default_exception_handler(self, context):
nonlocal _context
_context = context
# Simulates custom buggy "default_exception_handler"
raise ValueError('spam')

loop = Loop()
self.addCleanup(loop.close)
self.addCleanup(lambda: asyncio.set_event_loop(None))

asyncio.set_event_loop(loop)

def run_loop():
def zero_error():
loop.stop()
1 / 0
loop.call_soon(zero_error)
loop.run_forever()

with mock.patch.object(logger, 'error') as log:
run_loop()
log.assert_called_with(
'Exception in default exception handler',
exc_info=True)

def custom_handler(loop, context):
raise ValueError('ham')

_context = None
loop.set_exception_handler(custom_handler)
with mock.patch.object(logger, 'error') as log:
run_loop()
log.assert_called_with(
self.mock_pattern('Exception in default exception.*'
'while handling.*in custom'),
exc_info=True)

# Check that original context was passed to default
# exception handler.
self.assertIn('context', _context)
self.assertIs(type(_context['context']['exception']),
ZeroDivisionError)


class TestBaseAIO(_TestBase, AIOTestCase):
pass
Expand Down
33 changes: 33 additions & 0 deletions tests/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import subprocess
import sys
import time
import uvloop

from uvloop import _testbase as tb

Expand Down Expand Up @@ -277,6 +278,38 @@ async def coro(): pass
class Test_UV_Signals(_TestSignal, tb.UVTestCase):
NEW_LOOP = 'uvloop.new_event_loop()'

def test_signals_no_SIGCHLD(self):
with self.assertRaisesRegex(RuntimeError,
r"cannot add.*handler.*SIGCHLD"):

self.loop.add_signal_handler(signal.SIGCHLD, lambda *a: None)

def test_asyncio_add_watcher_SIGCHLD_nop(self):
async def proc(loop):
proc = await asyncio.create_subprocess_exec(
'echo',
stdout=subprocess.DEVNULL,
loop=loop)
await proc.wait()

aio_loop = asyncio.new_event_loop()
asyncio.set_event_loop(aio_loop)
try:
aio_loop.run_until_complete(proc(aio_loop))
finally:
aio_loop.close()
asyncio.set_event_loop(None)

try:
loop = uvloop.new_event_loop()
with self.assertWarnsRegex(
RuntimeWarning,
"asyncio is trying to install its ChildWatcher"):
asyncio.set_event_loop(loop)
finally:
asyncio.set_event_loop(None)
loop.close()


class Test_AIO_Signals(_TestSignal, tb.AIOTestCase):
NEW_LOOP = 'asyncio.new_event_loop()'
1 change: 1 addition & 0 deletions uvloop/includes/stdlib.pxi
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ cdef aio_isfuture = getattr(asyncio, 'isfuture', None)
cdef aio_get_running_loop = getattr(asyncio, '_get_running_loop', None)
cdef aio_set_running_loop = getattr(asyncio, '_set_running_loop', None)
cdef aio_debug_wrapper = getattr(asyncio.coroutines, 'debug_wrapper', None)
cdef aio_AbstractChildWatcher = asyncio.AbstractChildWatcher

cdef col_deque = collections.deque
cdef col_Iterable = collections.abc.Iterable
Expand Down
1 change: 1 addition & 0 deletions uvloop/includes/uv.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ cdef extern from "uv.h" nogil:

cdef int SIGINT
cdef int SIGHUP
cdef int SIGCHLD
cdef int SIGKILL
cdef int SIGTERM

Expand Down
51 changes: 41 additions & 10 deletions uvloop/loop.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ include "includes/stdlib.pxi"

include "errors.pyx"

cdef int PY37
PY37 = PY_VERSION_HEX >= 0x03070000
cdef:
int PY37 = PY_VERSION_HEX >= 0x03070000
int PY36 = PY_VERSION_HEX >= 0x03060000


cdef _is_sock_stream(sock_type):
Expand Down Expand Up @@ -1034,19 +1035,19 @@ cdef class Loop:

if enabled:
if current_wrapper not in (None, wrapper):
warnings.warn(
_warn_with_source(
"loop.set_debug(True): cannot set debug coroutine "
"wrapper; another wrapper is already set %r" %
current_wrapper, RuntimeWarning)
current_wrapper, RuntimeWarning, self)
else:
sys_set_coroutine_wrapper(wrapper)
self._coroutine_debug_set = True
else:
if current_wrapper not in (None, wrapper):
warnings.warn(
_warn_with_source(
"loop.set_debug(False): cannot unset debug coroutine "
"wrapper; another wrapper was set %r" %
current_wrapper, RuntimeWarning)
current_wrapper, RuntimeWarning, self)
else:
sys_set_coroutine_wrapper(None)
self._coroutine_debug_set = False
Expand Down Expand Up @@ -2547,8 +2548,31 @@ cdef class Loop:

if (aio_iscoroutine(callback)
or aio_iscoroutinefunction(callback)):
raise TypeError("coroutines cannot be used "
"with add_signal_handler()")
raise TypeError(
"coroutines cannot be used with add_signal_handler()")

if sig == uv.SIGCHLD:
if (hasattr(callback, '__self__') and
isinstance(callback.__self__, aio_AbstractChildWatcher)):

_warn_with_source(
"!!! asyncio is trying to install its ChildWatcher for "
"SIGCHLD signal !!!\n\nThis is probably because a uvloop "
"instance is used with asyncio.set_event_loop(). "
"The correct way to use uvloop is to install its policy: "
"`asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())`"
"\n\n", RuntimeWarning, self)

# TODO: ideally we should always raise an error here,
# but that would be a backwards incompatible change,
# because we recommended using "asyncio.set_event_loop()"
# in our README. Need to start a deprecation period
# at some point to turn this warning into an error.
return

raise RuntimeError(
'cannot add a signal handler for SIGCHLD: it is used '
'by the event loop to track subprocesses')

self._check_signal(sig)
self._check_closed()
Expand Down Expand Up @@ -2771,10 +2795,10 @@ cdef class Loop:

def _asyncgen_firstiter_hook(self, agen):
if self._asyncgens_shutdown_called:
warnings_warn(
_warn_with_source(
"asynchronous generator {!r} was scheduled after "
"loop.shutdown_asyncgens() call".format(agen),
ResourceWarning, source=self)
ResourceWarning, self)

self._asyncgens.add(agen)

Expand Down Expand Up @@ -2909,6 +2933,13 @@ cdef _set_signal_wakeup_fd(fd):
signal_set_wakeup_fd(fd)


cdef _warn_with_source(msg, cls, source):
if PY36:
warnings_warn(msg, cls, source=source)
else:
warnings_warn(msg, cls)


########### Stuff for tests:

@cython.iterable_coroutine
Expand Down