Skip to content

gh-117657: Fix race involving GC and heap initialization - #119923

Merged
colesbury merged 1 commit into
python:mainfrom
colesbury:gh-117657-gc-mimalloc-bind
Jun 4, 2024
Merged

gh-117657: Fix race involving GC and heap initialization#119923
colesbury merged 1 commit into
python:mainfrom
colesbury:gh-117657-gc-mimalloc-bind

Conversation

@colesbury

@colesburycolesbury commented Jun 1, 2024

Copy link
Copy Markdown
Contributor

The _PyThreadState_Bind() function is called before PyEval_AcquireThread() so it's not synchronized with the stop the world GC. We had a race where gc_visit_heaps() might visit a thread's heap while it's being initialized.

Use a simple atomic int to avoid visiting heaps for threads that are not yet fully initialized (i.e., before tstate_mimalloc_bind() is called).

The race was reproducible by running:
python Lib/test/test_importlib/partial/pool_in_threads.py.

The `_PyThreadState_Bind()` function is called before the first
`PyEval_AcquireThread()` so it's not synchronized with the stop the
world GC. We had a race where `gc_visit_heaps()` might visit a thread's
heap while it's being initialized.
Use a simple atomic int to avoid visiting heaps for threads that are not
yet fully initialized (i.e., before `tstate_mimalloc_bind()` is called).
The race was reproducible by running:
`python Lib/test/test_importlib/partial/pool_in_threads.py`.
@DinoV

DinoV commented Jun 3, 2024

Copy link
Copy Markdown
Contributor

I guess we could have also used the presence of current_object_heap and made that an atomic sign but I suppose the extra flag is a little clearer.

@DinoVDinoV left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@colesbury
colesbury merged commit e69d068 into python:mainJun 4, 2024
@miss-islington-app

Copy link
Copy Markdown

Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@colesbury
colesbury deleted the gh-117657-gc-mimalloc-bind branch June 4, 2024 13:42
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 4, 2024
…nGH-119923)
The `_PyThreadState_Bind()` function is called before the first
`PyEval_AcquireThread()` so it's not synchronized with the stop the
world GC. We had a race where `gc_visit_heaps()` might visit a thread's
heap while it's being initialized.
Use a simple atomic int to avoid visiting heaps for threads that are not
yet fully initialized (i.e., before `tstate_mimalloc_bind()` is called).
The race was reproducible by running:
`python Lib/test/test_importlib/partial/pool_in_threads.py`.
(cherry picked from commit e69d068)
Co-authored-by: Sam Gross <colesbury@gmail.com>
@bedevere-app

Copy link
Copy Markdown

GH-120038 is a backport of this pull request to the 3.13 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.13 bugs and security fixes label Jun 4, 2024
colesbury added a commit that referenced this pull request Jun 4, 2024
…19923) (#120038)
The `_PyThreadState_Bind()` function is called before the first
`PyEval_AcquireThread()` so it's not synchronized with the stop the
world GC. We had a race where `gc_visit_heaps()` might visit a thread's
heap while it's being initialized.
Use a simple atomic int to avoid visiting heaps for threads that are not
yet fully initialized (i.e., before `tstate_mimalloc_bind()` is called).
The race was reproducible by running:
`python Lib/test/test_importlib/partial/pool_in_threads.py`.
(cherry picked from commit e69d068)
Co-authored-by: Sam Gross <colesbury@gmail.com>
barneygale pushed a commit to barneygale/cpython that referenced this pull request Jun 5, 2024
…n#119923)
The `_PyThreadState_Bind()` function is called before the first
`PyEval_AcquireThread()` so it's not synchronized with the stop the
world GC. We had a race where `gc_visit_heaps()` might visit a thread's
heap while it's being initialized.
Use a simple atomic int to avoid visiting heaps for threads that are not
yet fully initialized (i.e., before `tstate_mimalloc_bind()` is called).
The race was reproducible by running:
`python Lib/test/test_importlib/partial/pool_in_threads.py`.
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
…n#119923)
The `_PyThreadState_Bind()` function is called before the first
`PyEval_AcquireThread()` so it's not synchronized with the stop the
world GC. We had a race where `gc_visit_heaps()` might visit a thread's
heap while it's being initialized.
Use a simple atomic int to avoid visiting heaps for threads that are not
yet fully initialized (i.e., before `tstate_mimalloc_bind()` is called).
The race was reproducible by running:
`python Lib/test/test_importlib/partial/pool_in_threads.py`.
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
…n#119923)
The `_PyThreadState_Bind()` function is called before the first
`PyEval_AcquireThread()` so it's not synchronized with the stop the
world GC. We had a race where `gc_visit_heaps()` might visit a thread's
heap while it's being initialized.
Use a simple atomic int to avoid visiting heaps for threads that are not
yet fully initialized (i.e., before `tstate_mimalloc_bind()` is called).
The race was reproducible by running:
`python Lib/test/test_importlib/partial/pool_in_threads.py`.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@colesbury@DinoV