Fix potential memory leak with persistent task groups#140
Fix potential memory leak with persistent task groups#140achimnol wants to merge 1 commit intogeldata:masterfrom
Conversation
* Check out the added test cases in my derivation: achimnol/aiotools#21
I'm not sure if it's an actual memory leak. Tasks don't store references to the TaskGroup that spawned them so there's no cyclic dependency here. And once a TaskGroup is GCed, all of its attributes will be cleared, including the set of tasks. So do you have any evidence that the current design causes problems?
Can you make this PR against the edgedb/edgedb repo? There are tests there (in tests/common/test_taskgroups.py) |
|
As I said, when TaskGroup objects are short-lived, it's okay. |
|
BTW, how is the progress about incorporating TaskGroup APIs into the standard asyncio library? |
Right, makes sense. I'm torn between making it a weakset vs keeping it a regular set but just having tasks remove themselves from it when they're done. Thoughts?
There's a small WG with Guido, Nathaniel, a few other people and I working on ExceptionGroups. That's the fundamental blocker for having task groups in asyncio. No eta yet, but at least we started working on them. :/ |
If WeakSet does not introduce side-effects, I think, why not using an automated clean-up mechanism? |
|
I've merged this PR to the edgedb repo where TaskGroups are used at runtime. In case of edgedb-python they're only used in tests, so we don't really care about this specific issue. |
I just discovered a potential memory leak issue because the task group internally stores references of all spawned tasks and never clears them in
_tasksset.This should not be a problem if the users of
TaskGrouponly uses it for short-lived asyncio tasks, but if they use it for a whole lifecycle of long-lived objects or the entire application, it would be a source of memory leak.Changing
_taskstoweakref.WeakSetautomatically clears (weak) references to completed tasks, and this does not affect the behavior of the currentTaskGroupimplementation.Please have a look in my personal fork's self-PR at achimnol/aiotools#21.
I've skipped adding test cases because there seems no place for
TaskGrouptest cases.