Skip to content

Commit a237a99

Browse files
committed
testing: Cancel all pending coroutines in tearDown
It's difficult to synchronize test shutdown with the exits of all coroutines, so explicitly cancel all native coroutines (which are spammy when allowed to be GC'd). Suppress logging of CancelledErrors in IOLoop.
1 parent ae9a2da commit a237a99

File tree

6 files changed

+73
-7
lines changed

6 files changed

+73
-7
lines changed

tornado/http1connection.py

+5-1
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,11 @@ async def _server_request_loop(
812812
request_delegate = delegate.start_request(self, conn)
813813
try:
814814
ret = await conn.read_response(request_delegate)
815-
except (iostream.StreamClosedError, iostream.UnsatisfiableReadError):
815+
except (
816+
iostream.StreamClosedError,
817+
iostream.UnsatisfiableReadError,
818+
asyncio.CancelledError,
819+
):
816820
return
817821
except _QuietException:
818822
# This exception was already logged.

tornado/ioloop.py

+5-1
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,9 @@ def set_default_executor(self, executor: concurrent.futures.Executor) -> None:
724724
def _run_callback(self, callback: Callable[[], Any]) -> None:
725725
"""Runs a callback with error handling.
726726
727-
For use in subclasses.
727+
.. versionchanged:: 6.0
728+
729+
CancelledErrors are no longer logged.
728730
"""
729731
try:
730732
ret = callback()
@@ -744,6 +746,8 @@ def _run_callback(self, callback: Callable[[], Any]) -> None:
744746
pass
745747
else:
746748
self.add_future(ret, self._discard_future_result)
749+
except asyncio.CancelledError:
750+
pass
747751
except Exception:
748752
app_log.error("Exception in callback %r", callback, exc_info=True)
749753

tornado/iostream.py

+5-5
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import sys
3434
import re
3535

36-
from tornado.concurrent import Future
36+
from tornado.concurrent import Future, future_set_result_unless_cancelled
3737
from tornado import ioloop
3838
from tornado.log import gen_log
3939
from tornado.netutil import ssl_wrap_socket, _client_ssl_defaults, _server_ssl_defaults
@@ -803,7 +803,7 @@ def _finish_read(self, size: int, streaming: bool) -> None:
803803
if self._read_future is not None:
804804
future = self._read_future
805805
self._read_future = None
806-
future.set_result(result)
806+
future_set_result_unless_cancelled(future, result)
807807
self._maybe_add_error_listener()
808808

809809
def _try_inline_read(self) -> None:
@@ -972,7 +972,7 @@ def _handle_write(self) -> None:
972972
if index > self._total_write_done_index:
973973
break
974974
self._write_futures.popleft()
975-
future.set_result(None)
975+
future_set_result_unless_cancelled(future, None)
976976

977977
def _consume(self, loc: int) -> bytes:
978978
# Consume loc bytes from the read buffer and return them
@@ -1311,7 +1311,7 @@ def _handle_connect(self) -> None:
13111311
if self._connect_future is not None:
13121312
future = self._connect_future
13131313
self._connect_future = None
1314-
future.set_result(self)
1314+
future_set_result_unless_cancelled(future, self)
13151315
self._connecting = False
13161316

13171317
def set_nodelay(self, value: bool) -> None:
@@ -1429,7 +1429,7 @@ def _finish_ssl_connect(self) -> None:
14291429
if self._ssl_connect_future is not None:
14301430
future = self._ssl_connect_future
14311431
self._ssl_connect_future = None
1432-
future.set_result(self)
1432+
future_set_result_unless_cancelled(future, self)
14331433

14341434
def _verify_cert(self, peercert: Any) -> bool:
14351435
"""Returns True if peercert is valid according to the configured

tornado/test/httpclient_test.py

+1
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,7 @@ def stop_server():
664664

665665
@gen.coroutine
666666
def slow_stop():
667+
yield self.server.close_all_connections()
667668
# The number of iterations is difficult to predict. Typically,
668669
# one is sufficient, although sometimes it needs more.
669670
for i in range(5):

tornado/test/testing_test.py

+26
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
from tornado import gen, ioloop
22
from tornado.httpserver import HTTPServer
3+
from tornado.locks import Event
34
from tornado.testing import AsyncHTTPTestCase, AsyncTestCase, bind_unused_port, gen_test
45
from tornado.web import Application
56
import asyncio
67
import contextlib
8+
import gc
79
import os
810
import platform
911
import traceback
@@ -55,6 +57,30 @@ def test_subsequent_wait_calls(self):
5557
self.wait(timeout=0.15)
5658

5759

60+
class LeakTest(AsyncTestCase):
61+
def tearDown(self):
62+
super().tearDown()
63+
# Trigger a gc to make warnings more deterministic.
64+
gc.collect()
65+
66+
def test_leaked_coroutine(self):
67+
# This test verifies that "leaked" coroutines are shut down
68+
# without triggering warnings like "task was destroyed but it
69+
# is pending". If this test were to fail, it would fail
70+
# because runtests.py detected unexpected output to stderr.
71+
event = Event()
72+
73+
async def callback():
74+
try:
75+
await event.wait()
76+
except asyncio.CancelledError:
77+
pass
78+
79+
self.io_loop.add_callback(callback)
80+
self.io_loop.add_callback(self.stop)
81+
self.wait()
82+
83+
5884
class AsyncHTTPTestCaseTest(AsyncHTTPTestCase):
5985
def setUp(self):
6086
super(AsyncHTTPTestCaseTest, self).setUp()

tornado/testing.py

+31
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
for the tornado.autoreload module to rerun the tests when code changes.
1010
"""
1111

12+
import asyncio
1213
from collections.abc import Generator
1314
import functools
1415
import inspect
@@ -178,6 +179,36 @@ def setUp(self) -> None:
178179
self.io_loop.make_current()
179180

180181
def tearDown(self) -> None:
182+
# Native coroutines tend to produce warnings if they're not
183+
# allowed to run to completion. It's difficult to ensure that
184+
# this always happens in tests, so cancel any tasks that are
185+
# still pending by the time we get here.
186+
asyncio_loop = self.io_loop.asyncio_loop # type: ignore
187+
if hasattr(asyncio, "all_tasks"): # py37
188+
tasks = asyncio.all_tasks(asyncio_loop) # type: ignore
189+
else:
190+
tasks = asyncio.Task.all_tasks(asyncio_loop)
191+
# Tasks that are done may still appear here and may contain
192+
# non-cancellation exceptions, so filter them out.
193+
tasks = [t for t in tasks if not t.done()]
194+
for t in tasks:
195+
t.cancel()
196+
# Allow the tasks to run and finalize themselves (which means
197+
# raising a CancelledError inside the coroutine). This may
198+
# just transform the "task was destroyed but it is pending"
199+
# warning into a "uncaught CancelledError" warning, but
200+
# catching CancelledErrors in coroutines that may leak is
201+
# simpler than ensuring that no coroutines leak.
202+
if tasks:
203+
done, pending = self.io_loop.run_sync(lambda: asyncio.wait(tasks))
204+
assert not pending
205+
# If any task failed with anything but a CancelledError, raise it.
206+
for f in done:
207+
try:
208+
f.result()
209+
except asyncio.CancelledError:
210+
pass
211+
181212
# Clean up Subprocess, so it can be used again with a new ioloop.
182213
Subprocess.uninitialize()
183214
self.io_loop.clear_current()

0 commit comments

Comments
 (0)