-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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 data races reported by TSAN in some set methods #120914
Conversation
Thanks! This pattern occurs in a bunch of places outside of I think it's also probably worth refactoring the fast hash check into its own inline function (maybe in |
Thanks for the review! I think this new function is not related to unicode object only, so I added it in |
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 looks good. Just minor comments about _PyObject_HashFast
Co-authored-by: Sam Gross <colesbury@gmail.com>
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! LGTM
Thanks @aisk for the PR, and @colesbury for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
…ythonGH-120914) Refactor the fast Unicode hash check into `_PyObject_HashFast` and use relaxed atomic loads in the free-threaded build. After this change, the TSAN doesn't report data races for this method. (cherry picked from commit 294e724) Co-authored-by: AN Long <aisk@users.noreply.github.com>
GH-121240 is a backport of this pull request to the 3.13 branch. |
Nice change, I like _PyObject_HashFast() which factorizes the code. |
…GH-120914) (#121240) Refactor the fast Unicode hash check into `_PyObject_HashFast` and use relaxed atomic loads in the free-threaded build. After this change, the TSAN doesn't report data races for this method. (cherry picked from commit 294e724) Co-authored-by: AN Long <aisk@users.noreply.github.com>
|
Buildbot failure is unrelated to this PR. See #119909 |
…ython#120914) Refactor the fast Unicode hash check into `_PyObject_HashFast` and use relaxed atomic loads in the free-threaded build. After this change, the TSAN doesn't report data races for this method.
…ython#120914) Refactor the fast Unicode hash check into `_PyObject_HashFast` and use relaxed atomic loads in the free-threaded build. After this change, the TSAN doesn't report data races for this method.
…ython#120914) Refactor the fast Unicode hash check into `_PyObject_HashFast` and use relaxed atomic loads in the free-threaded build. After this change, the TSAN doesn't report data races for this method.
set_add_key
andset_discard_key
have the same issue and should be fixed, although they have not been detected by TSAN.After this change, the TSAN doesn't report data races for this method on my machine.