Description

colesbury

opened on Nov 9, 2023

Issue body actions

Feature or enhancement

The current weakref implementation relies on the GIL for thread-safety.

The nogil-3.12 fork substantially modifies the weakref implementation. I think we can implement a simpler change in CPython 3.13 (main) now that all PyObject's have their own mutex (in --disable-gil builds).

Basic idea

Protect access to the weakrefs linked list using the mutex in the weakly referenced object. Use the critical section API.

Prior implementation: colesbury/nogil-3.12@0dddcb6f9d

Linked PRs

• gh-111926: Update _weakref to be threadsafe in --disable-gil build #112189
• gh-111926: Update _PyWeakref_IS_DEAD to be thread-safe #112267
• gh-111926: Set up basic sementics of weakref API for freethreading #113621
• gh-111926: Simplify weakref/proxy creation #116825
• gh-111926: Simplify weakref creation logic #116843
• gh-111926: Simplify proxy creation logic #116844
• gh-111926: Make weakrefs thread-safe in free-threaded builds #117168
• gh-111926: Avoid locking in PyType_IsSubtype #117275

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 9, 2023
2. corona10 commented on Nov 10, 2023

   

   corona10

   on Nov 10, 2023

   Member

   More actions

   Maybe we should check the performance impact the default build first.
   I will try to investigate it.
3. colesbury commented on Nov 10, 2023

   

   colesbury

   on Nov 10, 2023

   ContributorAuthor

   More actions

   The critical section APIs are no-ops on the default build:
   

   cpython/Include/internal/pycore_critical_section.h

   Lines 107 to 113 in 289af86

   	#else /* !Py_NOGIL */
   	// The critical section APIs are no-ops with the GIL.
   	# define Py_BEGIN_CRITICAL_SECTION(op)
   	# define Py_END_CRITICAL_SECTION()
   	# define Py_BEGIN_CRITICAL_SECTION2(a, b)
   	# define Py_END_CRITICAL_SECTION2()
   	#endif /* !Py_NOGIL */
4. corona10 commented on Nov 10, 2023

   

   corona10

   on Nov 10, 2023

   Member

   More actions

   I know that there is no op for the default build but the commit you suggested modify some internal structure, I just want to know ovehead from the structure changed, even I actually do not expect real overhead from some refactoring. (I didn't look at deeply now please let me know if I miss something)
5. colesbury commented on Nov 10, 2023

   

   colesbury

   on Nov 10, 2023

   ContributorAuthor

   More actions

   @corona10 the commit I linked to (in nogil-3.12) is not how I intend to make weakref thread-safe in CPython 3.13. I wouldn't spend too much time investigating it because it's not going to make it's way into 3.13. I linked to it because it's sometimes helpful for me to look back at the nogil-3.12 implementation.

   Reacted by Donghee Na
6. corona10 commented on Nov 16, 2023

   

   corona10

   on Nov 16, 2023

   Member

   More actions

   By the way, I am working on it :)

   Reacted by Sam Gross
7. colesbury

   assigned

   corona10

   on Nov 16, 2023
8. corona10

   added 5 commits that reference this issue on Nov 17, 2023

   pythongh-111926 Update _weakref to be threadsafe in --disable-gil build

   df6d6c1

   pythongh-111926 Update _weakref to be threadsafe in --disable-gil build

   c929bc0

   gh-111926: Update _weakref to be threadsafe in --disable-gil build (g…

   0566ab9

   pythongh-111926 Update _PyWeakref_IS_DEAD to be thread-safe

   866af79

   gh-111926: Update _PyWeakref_IS_DEAD to be thread-safe (gh-112267)

   7c9f267
9. corona10

   added a commit that references this issue on Jan 1, 2024

   pythongh-111926: Set up basic sementics of weakref API for freethreading

   a400a06
10. 34 remaining items
11. NGRsoftlab

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

    Squashed commit of the following:

    1b5fe4c

    Squashed commit of the following:

    a016a54
12. colesbury

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

    Merge branch 'pythongh-111926-thread-safe-weakref' into nogil-integra…

    bd711c8

    Merge branch 'pythongh-111926-thread-safe-weakref' into nogil-integra…

    90a75db

    gh-111926: Make weakrefs thread-safe in free-threaded builds (#117168)

    df73179
13. mpage

    closed this as completedon Apr 8, 2024
14. diegorusso

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

    pythongh-111926: Simplify weakref creation logic (python#116843)

    a4a10d4

    pythongh-111926: Simplify proxy creation logic (python#116844)

    552272b

    pythongh-111926: Avoid locking in PyType_IsSubtype (python#117275)

    3a18cb0

    pythongh-111926: Make weakrefs thread-safe in free-threaded builds (p…

    a1c8a00
15. mpage

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

    Don't raise an error if tuple allocation fails when clearing weakrefs

    6f141bd

    Don't raise an error if tuple allocation fails when clearing weakrefs

    195a7e5
16. Glyphack

    added 3 commits that reference this issue on Sep 2, 2024

    pythongh-111926: Update _weakref to be threadsafe in --disable-gil bu…

    55378dd

    pythongh-111926: Update _PyWeakref_IS_DEAD to be thread-safe (pythong…

    8411ef8

    pythongh-111926: Set up basic sementics of weakref API for freethread…

    7d4b548

Issue actions