-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
gh-122697: Fix free-threading memory leaks at shutdown #122703
Conversation
We were not properly accounting for interpreter memory leaks at shutdown and had two sources of leaks: * Objects that use deferred reference counting and were reachable via static types outlive the final GC. We now disable deferred reference counting on all objects if we are calling the GC due to interpreter shutdown. * `_PyMem_FreeDelayed` did not properly check for interpreter shutdown so we had some memory blocks that were enqueued to be freed, but never actually freed.
!buildbot nogil refleaks |
🤖 New build scheduled with the buildbot fleet by @colesbury for commit c9d7132 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
!buildbot nogil refleaks |
🤖 New build scheduled with the buildbot fleet by @colesbury for commit dcb709d 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
// Free immediately if the world is stopped, including during | ||
// interpreter shutdown. | ||
if (Py_IsFinalizing() || _PyRuntime.stoptheworld.world_stopped) { | ||
// Free immediately during interpreter shutdown or if the world is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
runtime shutdown would be better instead of interpreter shutdown?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this should be interpreter shutdown. That's why the refleak buildbots had failures.
!buildbot nogil refleaks |
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 7fc200b 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
Misc/NEWS.d/next/Core_and_Builtins/2024-08-05-19-28-12.gh-issue-122697.17MvYl.rst
Outdated
Show resolved
Hide resolved
…e-122697.17MvYl.rst Co-authored-by: Kumar Aditya <[email protected]>
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 70e25f5 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
+1: I don't know my way around this code, but I don't see anything wrong with the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…122703) We were not properly accounting for interpreter memory leaks at shutdown and had two sources of leaks: * Objects that use deferred reference counting and were reachable via static types outlive the final GC. We now disable deferred reference counting on all objects if we are calling the GC due to interpreter shutdown. * `_PyMem_FreeDelayed` did not properly check for interpreter shutdown so we had some memory blocks that were enqueued to be freed, but never actually freed. * `_PyType_FinalizeIdPool` wasn't called at interpreter shutdown.
We were not properly accounting for interpreter memory leaks at shutdown and had two sources of leaks:
Objects that use deferred reference counting and were reachable via static types outlive the final GC. We now disable deferred reference counting on all objects if we are calling the GC due to interpreter shutdown.
_PyMem_FreeDelayed
did not properly check for interpreter shutdown so we had some memory blocks that were enqueued to be freed, but never actually freed.