Skip to content
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-117657: Fix QSBR race condition #118843

Merged
merged 14 commits into from
May 10, 2024
Merged

gh-117657: Fix QSBR race condition #118843

merged 14 commits into from
May 10, 2024

Conversation

SonicField
Copy link
Contributor

@SonicField SonicField commented May 9, 2024

Fix TSAN issues in free threaded build:

  • initialize_new_array

Eclips4 and others added 11 commits May 10, 2024 02:14
…exit}__` (python#118812)

These methods are purely wrappers around `Semlock.{acquire,release}`,
which expect a critical section to be held.
…ld (python#118723)

The `list_preallocate_exact` function did not zero initialize array
contents. In the free-threaded build, this could expose uninitialized
memory to concurrent readers between the call to
`list_preallocate_exact` and the filling of the array contents with
items.
…t file (python#118808)

Some embedders and extensions include parts of the internal API. The
pycore_mimalloc.h file is transitively include by a number of other
internal headers. This avoids include errors for code that was
already including those headers.
Avoid immortalizing objects in tests that verify garbage collection of
classes or modules.

This fixes test_ordered_dict and test_struct.
…ython#118722)

Using `race:` filters out warnings if the function appears anywhere in
the stack trace. This can hide a lot of unrelated warnings, especially
for a function like `_PyEval_EvalFrameDefault`, which is somewhere on
the stack more often than not.

Change all free-threaded suppressions to `race_top:`, which only matches
the top frame, and add any new suppressions this exposes.
…NULL object (python#115433)" (python#118861)

This reverts commit ad4f909.

The API ended up not being used.
@colesbury colesbury changed the title gh-117657: initialize_new_array and others gh-117657: Fix QSBR race condition May 10, 2024
@colesbury colesbury added the needs backport to 3.13 bugs and security fixes label May 10, 2024
Copy link
Contributor

@colesbury colesbury left a comment

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 33d2019 into python:main May 10, 2024
37 checks passed
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 10, 2024
`_Py_qsbr_unregister` is called when the PyThreadState is already
detached, so the access to `tstate->qsbr` isn't safe without locking the
shared mutex. Grab the `struct _qsbr_shared` from the interpreter
instead.
(cherry picked from commit 33d2019)

Co-authored-by: Alex Turner <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented May 10, 2024

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

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 10, 2024
colesbury pushed a commit that referenced this pull request May 10, 2024
`_Py_qsbr_unregister` is called when the PyThreadState is already
detached, so the access to `tstate->qsbr` isn't safe without locking the
shared mutex. Grab the `struct _qsbr_shared` from the interpreter
instead.
(cherry picked from commit 33d2019)

Co-authored-by: Alex Turner <[email protected]>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
`_Py_qsbr_unregister` is called when the PyThreadState is already
detached, so the access to `tstate->qsbr` isn't safe without locking the
shared mutex. Grab the `struct _qsbr_shared` from the interpreter
instead.
Sign up for free to 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.

9 participants