ioloop: Deprecate add_callback_from_signal

I don't believe this method is currently working as intended, and I'm
not sure it ever has since the move to asyncio. I think this is
responsible for occasional test failures in CI.

Fixes #3225

diff --git a/maint/benchmark/benchmark.py b/maint/benchmark/benchmark.py
index d1f32d33ef32afa19c8fd4edfa330cb17442917b..845c3ff2e8a77902f739b52c0f58361414c8711e 100755 (executable)
--- a/maint/benchmark/benchmark.py
+++ b/maint/benchmark/benchmark.py
@@ -14,18 +14,11 @@
 # % sort time
 # % stats 20
 
-from tornado.ioloop import IOLoop
 from tornado.options import define, options, parse_command_line
 from tornado.web import RequestHandler, Application
 
+import asyncio
 import random
-import signal
-import subprocess
-
-try:
-    xrange
-except NameError:
-    xrange = range
 
 # choose a random port to avoid colliding with TIME_WAIT sockets left over
 # from previous runs.
@@ -44,8 +37,6 @@ define("quiet", type=bool, default=False)
 # --n=15000 for its JIT to reach full effectiveness
 define("num_runs", type=int, default=1)
 
-define("ioloop", type=str, default=None)
-
 
 class RootHandler(RequestHandler):
     def get(self):
@@ -55,24 +46,16 @@ class RootHandler(RequestHandler):
         pass
 
 
-def handle_sigchld(sig, frame):
-    IOLoop.current().add_callback_from_signal(IOLoop.current().stop)
-
-
 def main():
     parse_command_line()
-    if options.ioloop:
-        IOLoop.configure(options.ioloop)
-    for i in xrange(options.num_runs):
-        run()
+    for i in range(options.num_runs):
+        asyncio.run(run())
 
 
-def run():
-    io_loop = IOLoop(make_current=True)
+async def run():
     app = Application([("/", RootHandler)])
     port = random.randrange(options.min_port, options.max_port)
-    app.listen(port, address='127.0.0.1')
-    signal.signal(signal.SIGCHLD, handle_sigchld)
+    app.listen(port, address="127.0.0.1")
     args = ["ab"]
     args.extend(["-n", str(options.n)])
     args.extend(["-c", str(options.c)])
@@ -82,11 +65,9 @@ def run():
         # just stops the progress messages printed to stderr
         args.append("-q")
     args.append("http://127.0.0.1:%d/" % port)
-    subprocess.Popen(args)
-    io_loop.start()
-    io_loop.close()
-    io_loop.clear_current()
+    proc = await asyncio.create_subprocess_exec(*args)
+    await proc.wait()
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     main()

diff --git a/tornado/ioloop.py b/tornado/ioloop.py
index bcdcca097be90560952e1dde6dd2bfe809bebba0..6fbe9ee192c32616d369a09bceeed79207e361c7 100644 (file)
--- a/tornado/ioloop.py
+++ b/tornado/ioloop.py
@@ -632,9 +632,6 @@ class IOLoop(Configurable):
         other interaction with the `IOLoop` must be done from that
         `IOLoop`'s thread.  `add_callback()` may be used to transfer
         control from other threads to the `IOLoop`'s thread.
-
-        To add a callback from a signal handler, see
-        `add_callback_from_signal`.
         """
         raise NotImplementedError()
 
@@ -643,8 +640,13 @@ class IOLoop(Configurable):
     ) -> None:
         """Calls the given callback on the next I/O loop iteration.
 
-        Safe for use from a Python signal handler; should not be used
-        otherwise.
+        Intended to be afe for use from a Python signal handler; should not be
+        used otherwise.
+
+        .. deprecated:: 6.4
+           Use ``asyncio.AbstractEventLoop.add_signal_handler`` instead.
+           This method is suspected to have been broken since Tornado 5.0 and
+           will be removed in version 7.0.
         """
         raise NotImplementedError()
 

diff --git a/tornado/platform/asyncio.py b/tornado/platform/asyncio.py
index a15a74df4d3bc47f66c38ae07d68e1785fec7935..5248c1f9513c0c89da409acb814a13d90c55d327 100644 (file)
--- a/tornado/platform/asyncio.py
+++ b/tornado/platform/asyncio.py
@@ -239,6 +239,7 @@ class BaseAsyncIOLoop(IOLoop):
     def add_callback_from_signal(
         self, callback: Callable, *args: Any, **kwargs: Any
     ) -> None:
+        warnings.warn("add_callback_from_signal is deprecated", DeprecationWarning)
         try:
             self.asyncio_loop.call_soon_threadsafe(
                 self._run_callback, functools.partial(callback, *args, **kwargs)

diff --git a/tornado/process.py b/tornado/process.py
index 26428feb7780d1da595d856e80934ec7f5262224..12e3eb648d1d036198c5958303d8e78c22abedea 100644 (file)
--- a/tornado/process.py
+++ b/tornado/process.py
@@ -17,6 +17,7 @@
 the server into multiple processes and managing subprocesses.
 """
 
+import asyncio
 import os
 import multiprocessing
 import signal
@@ -210,7 +211,6 @@ class Subprocess(object):
 
     _initialized = False
     _waiting = {}  # type: ignore
-    _old_sigchld = None
 
     def __init__(self, *args: Any, **kwargs: Any) -> None:
         self.io_loop = ioloop.IOLoop.current()
@@ -322,11 +322,8 @@ class Subprocess(object):
         """
         if cls._initialized:
             return
-        io_loop = ioloop.IOLoop.current()
-        cls._old_sigchld = signal.signal(
-            signal.SIGCHLD,
-            lambda sig, frame: io_loop.add_callback_from_signal(cls._cleanup),
-        )
+        loop = asyncio.get_event_loop()
+        loop.add_signal_handler(signal.SIGCHLD, cls._cleanup)
         cls._initialized = True
 
     @classmethod
@@ -334,7 +331,8 @@ class Subprocess(object):
         """Removes the ``SIGCHLD`` handler."""
         if not cls._initialized:
             return
-        signal.signal(signal.SIGCHLD, cls._old_sigchld)
+        loop = asyncio.get_event_loop()
+        loop.remove_signal_handler(signal.SIGCHLD)
         cls._initialized = False
 
     @classmethod
@@ -352,7 +350,7 @@ class Subprocess(object):
             return
         assert ret_pid == pid
         subproc = cls._waiting.pop(pid)
-        subproc.io_loop.add_callback_from_signal(subproc._set_returncode, status)
+        subproc.io_loop.add_callback(subproc._set_returncode, status)
 
     def _set_returncode(self, status: int) -> None:
         if sys.platform == "win32":

diff --git a/tornado/test/ioloop_test.py b/tornado/test/ioloop_test.py
index 7de392f83e6c70bfdc8875d340c75e90b21668ce..9485afeaeb005e90a31cb9d2578249cf44db58df 100644 (file)
--- a/tornado/test/ioloop_test.py
+++ b/tornado/test/ioloop_test.py
@@ -127,7 +127,8 @@ class TestIOLoop(AsyncTestCase):
     def test_add_callback_from_signal(self):
         # cheat a little bit and just run this normally, since we can't
         # easily simulate the races that happen with real signal handlers
-        self.io_loop.add_callback_from_signal(self.stop)
+        with ignore_deprecation():
+            self.io_loop.add_callback_from_signal(self.stop)
         self.wait()
 
     def test_add_callback_from_signal_other_thread(self):
@@ -137,7 +138,8 @@ class TestIOLoop(AsyncTestCase):
         other_ioloop = IOLoop()
         thread = threading.Thread(target=other_ioloop.start)
         thread.start()
-        other_ioloop.add_callback_from_signal(other_ioloop.stop)
+        with ignore_deprecation():
+            other_ioloop.add_callback_from_signal(other_ioloop.stop)
         thread.join()
         other_ioloop.close()