Repository navigation
gh-115184: Fix refleak tracking issues in free-threaded build - #115188
Merged
colesbury merged 1 commit intoFeb 9, 2024
Merged
colesbury merged 1 commit into
colesbury merged 1 commit into
Conversation
- We did not account for abandoned segments, which could miss blocks in multithreaded runs. - The mimalloc heap traversal needed to call "_mi_page_free_collect" earlier in order to get an accurate count of blocks in use. - `_Py_DecRefSharedDebug` was missing a `_Py_IncRefTotal`, but this was mostly offset by a missing accounting in `_Py_ExplicitMergeRefcount`. - get_num_global_allocated_blocks should pause other threads to ensure that traversing the mimalloc heaps is safe.
Copy link
Copy Markdown
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 1b17640 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
Sorry, something went wrong.
DinoV
reviewed
Feb 9, 2024
Comment thread
Objects/object.c
| _Py_AddRefTotal(_PyInterpreterState_GET(), extra); | ||
| #endif | ||
|
|
||
| _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, 0); |
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
Do I understand this correctly that we're not racing here because we're either own the owning thread or the world is stopped for GC?
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Author
There was a problem hiding this comment.
The possible cases are:
- We are the owning thread and gave up ownership (typical)
- We are the queueing thread and the owning thread has exited (gh-110481: Implement inter-thread queue for biased reference counting #114824)
- We are the queueing thread and have stopped the world (due to OOM) (gh-110481: Implement inter-thread queue for biased reference counting #114824)
The GC could have used this code path, but it uses a different function that avoids atomics (because it's stopped the world).
The reason case (2) is thread-safe is a bit subtle:
- Queueing is done once, so only one thread will attempt it
- Although thread ids may be reused, the merge for (2) happens while holding a lock that would need to be acquired by any new thread that reuses the thread id.
Sorry, something went wrong.
DinoV
approved these changes
Feb 9, 2024
DinoV
left a comment
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment.
LGTM!
Sorry, something went wrong.
fsc-eriker
pushed a commit
to fsc-eriker/cpython
that referenced
this pull request
Feb 14, 2024
…ython#115188) Fixes a few issues related to refleak tracking in the free-threaded build: - Count blocks in abandoned segments - Call `_mi_page_free_collect` earlier during heap traversal in order to get an accurate count of blocks in use. - Add missing refcount tracking in `_Py_DecRefSharedDebug` and `_Py_ExplicitMergeRefcount`. - Pause threads in `get_num_global_allocated_blocks` to ensure that traversing the mimalloc heaps is safe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.