gh-91887: Store strong references to pending tasks #121264
Conversation
alexhartl
requested review from
1st1,
asvetlov,
gvanrossum,
kumaraditya303 and
willingc
as code owners
|
All commit authors signed the Contributor License Agreement. |
Misc/NEWS.d/next/Library/2024-07-02-14-07-32.gh-issue-91887.eWzc5E.rst
Outdated
Show resolved
Hide resolved
Misc/NEWS.d/next/Library/2024-07-02-14-07-32.gh-issue-91887.eWzc5E.rst
Outdated
Show resolved
Hide resolved
| return True | ||
|
|
||
| def __schedule_callbacks(self): | ||
| """Internal: Ask the event loop to call all callbacks. | ||
| def _finish_execution(self): |
There was a problem hiding this comment.
For clarification, what's the reason for this name change? This is still, effectively, scheduling all callbacks -- "finish execution" is a bit more ambiguous to me.
There was a problem hiding this comment.
Yes, in Future, "_schedule_callbacks" is perfectly fine to describe what the function does. When overriding this function in Task, I think _finish_execution is more meaningful to indicate that it will be called when the task completes.
Lib/test/test_asyncio/test_tasks.py
Outdated
There was a problem hiding this comment.
It might be a good idea to add a test to make sure that this actually fixes #91887, to make sure that someone doesn't accidentally break this in the future.
Likely would be something like:
def test_strong_task_references(self):
called = False
async def coro():
nonlocal called
called = True
async def main():
asyncio.create_task(coro())
loop = asyncio.new_event_loop()
try:
loop.run_until_complete(main())
finally:
loop.close()
self.assertTrue(called)
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
|
I can ponder on this during the core sprint (and think how this will play with uvloop). |
|
@alexhartl I'm at the CPython core team sprint, so I've taken the liberty of merging with main so I can discuss this with @1st1 and others. There were some conflicts introduced as a result of #120974. |
freakboy3742
force-pushed
the
strong-refs-for-bg-tasks
branch
from
9c5073b
to
eb89d4c
Compare
|
Thank you for picking this up @freakboy3742 ! Yes, the last time I checked, the asyncio C code appeared to be in the middle of some restructuring. Let me know if I can help with anything. |
Store strong references into a global set as well. Hopefully can get removed one day, as python/cpython#121264 was merged just this week :)
This adds a
_pending_tasksset toBaseEventLoop. OnTaskcreation, a (strong) reference to the task is added to this set in_register_task. When a task completes, the respective reference is removed from_pending_tasksin_unregister_task. See the discussion at #91887.