Prohibit adding a signal handler for SIGCHLD (#156)

Author: 1st1

README.rst

@@ -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
 ---------------------

tests/test_base.py

@@ -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,
@@ -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()
@@ -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()
@@ -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

tests/test_signals.py

@@ -3,6 +3,7 @@
 import subprocess
 import sys
 import time
+import uvloop
 
 from uvloop import _testbase as tb
 
@@ -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()'

uvloop/includes/stdlib.pxi

@@ -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

uvloop/includes/uv.pxd

@@ -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
 

uvloop/loop.pyx

@@ -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):
@@ -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
@@ -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()
@@ -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)
 
@@ -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