-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-129967: Fix race condition in repr(set)
#129978
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
Conversation
The call to `PySequence_List()` could temporarily unlock and relock the set, allowing the items to be cleared and return the incorrect notation `{}` for a empty set (it should be `set()`).
for set_repr in set_reprs: | ||
self.assertIn(set_repr, ("set()", "{1, 2, 3, 4, 5, 6, 7, 8}")) |
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.
... this would report all failing set reprs, not just the first one, and avoid hardcoding the repr we're expecting. (4/4)
for set_repr in set_reprs: | |
self.assertIn(set_repr, ("set()", "{1, 2, 3, 4, 5, 6, 7, 8}")) | |
self.assertEqual(set_reprs, expected) |
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.
This assertion ended up not working:
expected is { "set()", "{1, 2, 3, 4, 5, 6, 7, 8}" }
set_reprs may be any of the three cases:
{ "set()", "{1, 2, 3, 4, 5, 6, 7, 8}" }
{ "{1, 2, 3, 4, 5, 6, 7, 8}" }
{ "set()" }
So we could do self.assertTrue(set_reprs.issubset(expected))
or something similar, but at that point I'd prefer the original assertion failure message.
Co-authored-by: T. Wouters <thomas@python.org>
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, @colesbury, I could not cleanly backport this to
|
GH-130020 is a backport of this pull request to the 3.13 branch. |
…29978) The call to `PySequence_List()` could temporarily unlock and relock the set, allowing the items to be cleared and return the incorrect notation `{}` for a empty set (it should be `set()`). (cherry picked from commit a7427f2) Co-authored-by: Sam Gross <colesbury@gmail.com> Co-authored-by: T. Wouters <thomas@python.org>
In the free threaded build, the call to
PySequence_List()
could temporarily unlock and relock the set, allowing the items to be cleared and return the incorrect notation{}
for a empty set (it should beset()
).clear
causes concurrent__str__
to print as empty dict #129967