Skip to content
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

gen: Hold strong references to all asyncio.Tasks #3269

Merged
merged 1 commit into from
May 15, 2023

Conversation

bdarnell
Copy link
Member

Per the warning in the asyncio documentation, we need to hold a strong reference to all asyncio Tasks to prevent premature GC. Following discussions in cpython (python/cpython#91887), we hold these references on the IOLoop instance to ensure that they are strongly held but do not cause leaks if the event loop itself is discarded.

This is expected to fix all of the various "task was destroyed but it is pending" warnings that have been reported. The IOLoop._pending_tasks set is expected to become obsolete if corresponding changes are made to asyncio in Python 3.13.

Fixes #3209
Fixes #3047
Fixes #2763

Some issues involve this warning as their most visible symptom, but have an underlying cause that should still be addressed. Updates #2914
Updates #2356

Per the warning in the asyncio documentation, we need to hold a strong
reference to all asyncio Tasks to prevent premature GC. Following
discussions in cpython (python/cpython#91887),
we hold these references on the IOLoop instance to ensure that they are
strongly held but do not cause leaks if the event loop itself is
discarded.

This is expected to fix all of the various "task was destroyed but
it is pending" warnings that have been reported. The
IOLoop._pending_tasks set is expected to become obsolete if
corresponding changes are made to asyncio in Python 3.13.

Fixes tornadoweb#3209
Fixes tornadoweb#3047
Fixes tornadoweb#2763

Some issues involve this warning as their most visible symptom,
but have an underlying cause that should still be addressed.
Updates tornadoweb#2914
Updates tornadoweb#2356
@bdarnell bdarnell merged commit b5dad63 into tornadoweb:master May 15, 2023
10 checks passed
@bdarnell bdarnell deleted the refer-to-all-tasks branch May 15, 2023 01:34
@thetorpedodog
Copy link

Are there plans to either backport this to a 6.3.x point release or to release a v6.4?

I believe this is related to hanging/dropped requests we are experiencing in our JupyterLab installation. It appears the culprit may be in Tornado’s web module:

tornado/tornado/web.py

Lines 2431 to 2438 in e4d6984

fut = gen.convert_yielded(
self.handler._execute(transforms, *self.path_args, **self.path_kwargs)
)
fut.add_done_callback(lambda f: f.result())
# If we are streaming the request body, then execute() is finished
# when the handler has prepared to receive the body. If not,
# it doesn't matter when execute() finishes (so we return None)
return self.handler._prepared_future

The above seems to create a free-flying task, if I am reading correctly. I can also confirm that running pip install git+https://github.com/tornadoweb/tornado.git fixes the issue.

thetorpedodog added a commit to TileDB-Inc/tornado-dev that referenced this pull request Aug 23, 2023
This ensures that, when an async task for an HTTP handler delegate
is running, a strong reference is maintained at all times by
the delegate itself.

This is a complement to tornadoweb#3269.
While that change creates a global store of running tasks, this one
independently stores it locally.
@bdarnell
Copy link
Member Author

Tornado 6.4 is coming soon (in the next month, before Python 3.12 is released). That will include this change.

@thefunny42
Copy link

I have a similar issue in my application, which I suspect triggers some bugs randomly. Do you know when 6.4 will be released?

@bdarnell
Copy link
Member Author

bdarnell commented Nov 6, 2023

I was hoping to get this release done in October but I found some last-minute issues that revealed some important gaps in our testing. It's still coming soon but I haven't had much time to work on it recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants