Description

colesbury

opened on Nov 14, 2023

Issue body actions

Feature or enhancement

The set object is not currently thread-safe in --disable-gil builds. We should make it thread-safe by using the "critical section" API. to acquire the per-object locks around operations. There should be no effective change in the default build (i.e., with GIL) because critical sections are no-ops in the default build.

Notes:

1. Unlike dict and list, I don't think it's worth the complexity to try to "optimistically avoid locking" around any set operation (except set_len). We could consider doing this in the future if there is a performance justification, but not for 3.13.
2. set_len can avoid locking and instead use relaxed atomics for reading the "used" field. Note that writes to "used" should then also use relaxed atomics.
3. Some operations require locking two containers (like set_merge). Some of these will need refactorings so that the critical sections macros can be added in the correct places.

For context, here is the change from the nogil-3.12 fork: colesbury/nogil-3.12@4ca2924f0d. Note that the critical section API is slightly changed in 3.13 from nogil-3.12; In 3.13 Py_BEGIN_CRITICAL_SECTION takes a PyObject instead of a PyMutex.

TODO:

• Improve set_init (see gh-112069: Make sets thread-safe with the GIL disabled #113800 (comment)). We also want to avoid locking in set_init if possible
• _PySet_NextEntry
• setiter_iternext

Linked PRs

• gh-112069: Make sets thread-safe with the GIL disabled #113800
• gh-112069: Adapt set/frozenset methods to Argument Clinic #115112
• Revert "gh-112069: Make sets thread-safe with the GIL disabled (#113800)" #116525
• gh-112069: Make setiter_iternext to be thread-safe #117935
• gh-112069: Add _PySet_NextEntryRef to be thread-safe. #117990
• gh-112069: Make PySet_GET_SIZE to be atomic safe. #118053
• gh-112069: Do not require lock if the set has never been exposed. #118069

Activity

1. colesbury

   added

   type-featureA feature request or enhancementA feature request or enhancement

   3.13only security fixesonly security fixes

   topic-free-threading

   on Nov 14, 2023
2. tomasr8 commented on Nov 16, 2023

   

   tomasr8

   on Nov 16, 2023

   Member

   More actions

   Hi again! This one looks more challenging than the hashlib one but I'd like to try anyway :)

   Reacted by Sam Gross

   Reacted by Erlend E. Aasland
3. colesbury commented on Nov 16, 2023

   

   colesbury

   on Nov 16, 2023

   ContributorAuthor

   More actions

   Thanks @tomasr8!
4. colesbury

   assigned

   tomasr8

   on Nov 16, 2023
5. DinoV commented on Jan 4, 2024

   

   DinoV

   on Jan 4, 2024

   Contributor

   More actions

   hi @tomasr8, just curious if you're still planning on working on this?
6. tomasr8 commented on Jan 5, 2024

   

   tomasr8

   on Jan 5, 2024

   Member

   More actions

   Hi! Yes, I took a break for Christmas but I have it almost working, just need to fix some tests. I could open a draft PR this weekend

   Reacted by Dino Viehland

   Reacted by Erlend E. Aasland
7. erlend-aasland

   added a commit that references this issue on Feb 8, 2024

   gh-112069: Adapt set/frozenset methods to Argument Clinic (#115112)

   ed1a8da
8. fsc-eriker

   added a commit that references this issue on Feb 14, 2024

   pythongh-112069: Adapt set/frozenset methods to Argument Clinic (pyth…

   d47a773
9. colesbury

   added 2 commits that reference this issue on Mar 8, 2024

   gh-112069: Make sets thread-safe with the GIL disabled (#113800)

   c951e25

   Revert "pythongh-112069: Make sets thread-safe with the GIL disabled (p…

   f86c474
10. adorilson

    added a commit that references this issue on Mar 25, 2024

    pythongh-112069: Make sets thread-safe with the GIL disabled (python#…

    43e4fad
11. corona10

    added a commit that references this issue on Apr 16, 2024

    pythongh-112069: Make setiter_iternext to be thread-safe

    ab18f4e
12. corona10 commented on Apr 16, 2024

    

    corona10

    on Apr 16, 2024

    Member

    More actions

    @colesbury I am going to work on _PySet_NextEntry this week, considering the caller-side usage.
13. corona10

    self-assigned this

    on Apr 16, 2024
14. corona10

    added a commit that references this issue on Apr 16, 2024

    gh-112069: Make setiter_iternext to be thread-safe (gh-117935)

    a734fd5
15. diegorusso

    added 2 commits that reference this issue on Apr 17, 2024

    pythongh-112069: Make sets thread-safe with the GIL disabled (python#…

    c1252ea

    pythongh-112069: Make setiter_iternext to be thread-safe (pythongh-11…

    854fe3e
16. corona10

    added 7 commits that reference this issue on Apr 17, 2024

    pythongh-112069: Make _PySet_NextEntry to be thread-safe.

    f1776aa

    gh-112069: Add _PySet_NextEntryRef to be thread-safe. (gh-117990)

    94444ea

    pythongh-112069: Make PySet_GET_SIZE to be atomic operation

    a4fb0bd

    gh-112069: Make PySet_GET_SIZE to be atomic safe. (gh-118053)

    710c01b

    pythongh-112069: Do not require lock if the set has never been exposed.

    50d62bd

    Merge remote-tracking branch 'upstream/main' into pythongh-112069-init

    abecdf0

    gh-112069: Do not require lock if the set has never been exposed. (gh…

    eb20a7d
17. corona10 commented on Apr 25, 2024

    

    corona10

    on Apr 25, 2024

    Member

    More actions

    @colesbury Now we can close the issue right? Or more thing is left?
18. colesbury commented on Apr 25, 2024

    

    colesbury

    on Apr 25, 2024

    ContributorAuthor

    More actions

    Yeah, I think so

    Reacted by Donghee Na
19. corona10

    closed this as completedon Apr 25, 2024

Issue actions