fix object leak #4964
fix object leak #4964
Conversation
| @@ -1355,8 +1362,8 @@ def call_from_thread( | |||
|
|
|||
| async def run_callback() -> CallThreadReturnType: | |||
| """Run the callback, set the result or error on the future.""" | |||
| self._set_active() | |||
| return await invoke(callback_with_args) | |||
| with self._context(): | |||
There was a problem hiding this comment.
An interesting trick, I might use that in some of my code. Thanks for the tip in the tweet!
| if nodes_remaining: | ||
| print("NODES REMAINING") | ||
|
|
||
| import objgraph | ||
|
|
||
| objgraph.show_backrefs( | ||
| [ | ||
| obj | ||
| for obj in gc.get_objects() | ||
| if any(cls.__name__ == "App" for cls in obj.__class__.__mro__) | ||
| ], | ||
| filename="graph.png", | ||
| max_depth=15, | ||
| ) |
There was a problem hiding this comment.
Does this need to be removed or commented out?
There was a problem hiding this comment.
It's marked as xfail for now. When not run in isolation it fails because pytest keeps references around, presumably to inspect and report errors.
src/textual/reactive.py
Outdated
| getattr(obj, "_watchers", {}).clear() | ||
| getattr(obj, "__computes", []).clear() |
There was a problem hiding this comment.
I feel we should either rename both __watchers and __computes to single underscore prefix or neither.
There was a problem hiding this comment.
Watchers was promoted because I needed to clear it from outside of the class. __computes didn't need that.
On reflection though, I can add a method to Reactive to do that, and I can keep the __
Co-authored-by: Darren Burns <[email protected]>
|
Thank you! |
Fixes #4959
This turned out to be quite a rabbit hole. I fixed a class of problems where something was holding on to references to DOM nodes, and preventing garbage collection.
One culprit was
count_parameters. The@lru_cachewas holding on to a reference chain. I refactored that method to cache in a different way.Caching or cache-like structures were also holding on to references. I've added clearing of these "caches" to make garbage collection more prompt.
I've made some references weak references, to prevent potential reference cycles.
But the largest part of this work was related to context vars. These weren't being managed well, and it was leaving references around to the app or widgets, which would prevent entire branches from being collected.
The context var issue I fixed by writing a context var context manager, which resets the context var. DOMNodes and App have grown a
_context()which sets and restores the active app and message pump.Incidentally, I think this is the cause of some of the weirdness in tests where tests work in a run but fail in isolation. Errors in managing the context vars could result in an app being shared across tests.
Many of the tests had to be updated after the context var changes. For some reason the
pilotfixture broke. I think because of the way that pytest runs async tests.I'm pretty sure these changes improve shutdown time. Although I haven't profiled.