-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
gh-121464: Make concurrent iteration over enumerate safe under free-threading #125734
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
|
My expertise is with the Python-based |
|
|
||
|
|
||
| class EnumerateThreading(unittest.TestCase): | ||
| @staticmethod |
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 @staticmethod seems to be removable.
|
cc @colesbury |
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.
Looks good overall. Two comments below.
Co-authored-by: Sam Gross <colesbury@gmail.com>
We make concurrent iteration with
enumeratethread-safe (e.g. not crash, correct results are not guaranteed, see #124397) byUsing
_PyObject_IsUniquelyReferencedfor the re-use of the result tupleIn
enum_nextusingFT_ATOMIC_LOAD_SSIZE_RELAXED/FT_ATOMIC_STORE_SSIZE_RELAXEDfor loading and saving of the mutableen->en_indexIn
enum_next_long(andenum_reduce) we have to deal withen->en_longindexwhich is mutable. Since the objects inen_longindexneed to be properly refcounted this is a bit hard to handle. Two options:i) Use a lock. This is a simple approach and the performance cost might be acceptable since use cases of
enumeratewith numbers that do not fit in a long are rareii) The
en->en_longindexcan be atomically swapped withstepped_upusing_Py_atomic_exchange_ptr. With this approachen->en_longindexis either zero, or has a refcount of 1. The thread performing the swap ofen->en_longindexcan decrement the olden->en_longindex. A branch with this approach is enumerate_ft_v3.In this PR we pick option i) because it is simpler and more robust.