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-121464: Make concurrent iteration over enumerate safe under free-threading #125734

Merged
merged 22 commits into from
Mar 13, 2025

Conversation

eendebakpt
Copy link
Contributor

@eendebakpt eendebakpt commented Oct 19, 2024

We make concurrent iteration with enumerate thread-safe (e.g. not crash, correct results are not guaranteed, see #124397) by

  • Using _PyObject_IsUniquelyReferenced for the re-use of the result tuple

  • In enum_next using FT_ATOMIC_LOAD_SSIZE_RELAXED/FT_ATOMIC_STORE_SSIZE_RELAXED for loading and saving of the mutable en->en_index

  • In enum_next_long (and enum_reduce) we have to deal with en->en_longindex which is mutable. Since the objects in en_longindex need 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 enumerate with numbers that do not fit in a long are rare
    ii) The en->en_longindex can be atomically swapped with stepped_up using _Py_atomic_exchange_ptr. With this approach en->en_longindex is either zero, or has a refcount of 1. The thread performing the swap of en->en_longindex can decrement the old en->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.

@eendebakpt eendebakpt changed the title Draft: gh-120608: Make concurrent iteration over enumerate safe under free-threading Draft: gh-121464: Make concurrent iteration over enumerate safe under free-threading Oct 19, 2024
@eendebakpt eendebakpt changed the title Draft: gh-121464: Make concurrent iteration over enumerate safe under free-threading gh-121464: Make concurrent iteration over enumerate safe under free-threading Oct 19, 2024
@ethanfurman
Copy link
Member

My expertise is with the Python-based Enum, not the C-based enumerate. Removing myself as a reviewer.

@ethanfurman ethanfurman removed their request for review October 20, 2024 19:05


class EnumerateThreading(unittest.TestCase):
@staticmethod
Copy link
Contributor

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.

fiona02

This comment was marked as off-topic.

@rruuaanng
Copy link
Contributor

cc @colesbury

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.

Looks good overall. Two comments below.

@eendebakpt eendebakpt requested a review from colesbury January 27, 2025 16:35
@kumaraditya303 kumaraditya303 merged commit ec46a55 into python:main Mar 13, 2025
48 checks passed
plashchynski pushed a commit to plashchynski/cpython that referenced this pull request Mar 17, 2025
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.

6 participants