From 176aa3dcb35efe8b4dc4180b267dc37796866dc5 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 24 Jun 2025 11:26:50 -0600 Subject: [PATCH 1/6] gh-133296: Publicly expose critical section API that accepts PyMutex --- Doc/c-api/init.rst | 40 +++++++++++++++++++ Include/cpython/critical_section.h | 20 ++++++++++ Include/internal/pycore_critical_section.h | 14 +------ ...-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst | 3 ++ Modules/_ctypes/ctypes.h | 2 +- Modules/_testcapimodule.c | 7 ++++ Objects/typeobject.c | 4 +- Python/critical_section.c | 18 +++++++++ 8 files changed, 93 insertions(+), 15 deletions(-) create mode 100644 Misc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 3106bf9808f254..f96fb63a7b486e 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -2294,6 +2294,12 @@ is resumed, and its locks reacquired. This means the critical section API provides weaker guarantees than traditional locks -- they are useful because their behavior is similar to the :term:`GIL`. +Variants that accept :c:type:`PyMutex` instances rather than objects are also +available. Use these variants to start a critical section in a situation where +there is no :c:type:`PyObject` -- for example, when working with a C type that +does not extend or wrap :c:type:`PyObject` but still needs to call into the C +API in a manner that might lead to deadlocks. + The functions and structs used by the macros are exposed for cases where C macros are not available. They should only be used as in the given macro expansions. Note that the sizes and contents of the structures may @@ -2339,6 +2345,23 @@ code triggered by the finalizer blocks and calls :c:func:`PyEval_SaveThread`. .. versionadded:: 3.13 +.. c:macro:: Py_BEGIN_CRITICAL_SECTION_MUTEX(m) + + Locks the mutex *m* and begins a critical section. + + In the free-threaded build, this macro expands to:: + + { + PyCriticalSection _py_cs; + PyCriticalSection_BeginMutex(&_py_cs, m) + + Note that unlike :c:macro:`Py_BEGIN_CRITICAL_SECTION`, there is no cast for + the second argument - it must be a :c:type:`PyMutex`. + + On the default build, this macro expands to ``{``. + + .. versionadded:: 3.15 + .. c:macro:: Py_END_CRITICAL_SECTION() Ends the critical section and releases the per-object lock. @@ -2368,6 +2391,23 @@ code triggered by the finalizer blocks and calls :c:func:`PyEval_SaveThread`. .. versionadded:: 3.13 +.. c:macro:: Py_BEGIN_CRITICAL_SECTION2_MUTEX(m1, m2) + + Locks the mutexes *m1* and *m2* and begins a critical section. + + In the free-threaded build, this macro expands to:: + + { + PyCriticalSection2 _py_cs2; + PyCriticalSection2_BeginMutex(&_py_cs2, m1, m2) + + Note that unlike :c:macro:`Py_BEGIN_CRITICAL_SECTION2`, there is no cast for + the second and third arguments - they must be :c:type:`PyMutex` instances. + + On the default build, this macro expands to ``{``. + + .. versionadded:: 3.15 + .. c:macro:: Py_END_CRITICAL_SECTION2() Ends the critical section and releases the per-object locks. diff --git a/Include/cpython/critical_section.h b/Include/cpython/critical_section.h index 35db3fb6a59ce6..30c67f2beea3a6 100644 --- a/Include/cpython/critical_section.h +++ b/Include/cpython/critical_section.h @@ -73,22 +73,32 @@ typedef struct PyCriticalSection2 PyCriticalSection2; PyAPI_FUNC(void) PyCriticalSection_Begin(PyCriticalSection *c, PyObject *op); +PyAPI_FUNC(void) +PyCriticalSection_BeginMutex(PyCriticalSection *c, PyMutex *m); + PyAPI_FUNC(void) PyCriticalSection_End(PyCriticalSection *c); PyAPI_FUNC(void) PyCriticalSection2_Begin(PyCriticalSection2 *c, PyObject *a, PyObject *b); +PyAPI_FUNC(void) +PyCriticalSection2_BeginMutex(PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2); + PyAPI_FUNC(void) PyCriticalSection2_End(PyCriticalSection2 *c); #ifndef Py_GIL_DISABLED # define Py_BEGIN_CRITICAL_SECTION(op) \ { +# define Py_BEGIN_CRITICAL_SECTION_MUTEX(mut) \ + { # define Py_END_CRITICAL_SECTION() \ } # define Py_BEGIN_CRITICAL_SECTION2(a, b) \ { +# define Py_BEGIN_CRITICAL_SECTION2_MUTEX(mut) \ + { # define Py_END_CRITICAL_SECTION2() \ } #else /* !Py_GIL_DISABLED */ @@ -118,6 +128,11 @@ struct PyCriticalSection2 { PyCriticalSection _py_cs; \ PyCriticalSection_Begin(&_py_cs, _PyObject_CAST(op)) +# define Py_BEGIN_CRITICAL_SECTION_MUTEX(mutex) \ + { \ + PyCriticalSection _py_cs; \ + PyCriticalSection_BeginMutex(&_py_cs, mutex) + # define Py_END_CRITICAL_SECTION() \ PyCriticalSection_End(&_py_cs); \ } @@ -127,6 +142,11 @@ struct PyCriticalSection2 { PyCriticalSection2 _py_cs2; \ PyCriticalSection2_Begin(&_py_cs2, _PyObject_CAST(a), _PyObject_CAST(b)) +# define Py_BEGIN_CRITICAL_SECTION2_MUTEX(m1, m2) \ + { \ + PyCriticalSection2 _py_cs2; \ + PyCriticalSection2_BeginMutex(&_py_cs2, m1, m2) + # define Py_END_CRITICAL_SECTION2() \ PyCriticalSection2_End(&_py_cs2); \ } diff --git a/Include/internal/pycore_critical_section.h b/Include/internal/pycore_critical_section.h index 62460c5f8fad30..2601de40737e85 100644 --- a/Include/internal/pycore_critical_section.h +++ b/Include/internal/pycore_critical_section.h @@ -21,16 +21,6 @@ extern "C" { #define _Py_CRITICAL_SECTION_MASK 0x3 #ifdef Py_GIL_DISABLED -# define Py_BEGIN_CRITICAL_SECTION_MUT(mutex) \ - { \ - PyCriticalSection _py_cs; \ - _PyCriticalSection_BeginMutex(&_py_cs, mutex) - -# define Py_BEGIN_CRITICAL_SECTION2_MUT(m1, m2) \ - { \ - PyCriticalSection2 _py_cs2; \ - _PyCriticalSection2_BeginMutex(&_py_cs2, m1, m2) - // Specialized version of critical section locking to safely use // PySequence_Fast APIs without the GIL. For performance, the argument *to* // PySequence_Fast() is provided to the macro, not the *result* of @@ -75,8 +65,6 @@ extern "C" { #else /* !Py_GIL_DISABLED */ // The critical section APIs are no-ops with the GIL. -# define Py_BEGIN_CRITICAL_SECTION_MUT(mut) { -# define Py_BEGIN_CRITICAL_SECTION2_MUT(m1, m2) { # define Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST(original) { # define Py_END_CRITICAL_SECTION_SEQUENCE_FAST() } # define _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(mutex) @@ -119,6 +107,7 @@ _PyCriticalSection_BeginMutex(PyCriticalSection *c, PyMutex *m) _PyCriticalSection_BeginSlow(c, m); } } +#define PyCriticalSection_BeginMutex _PyCriticalSection_BeginMutex static inline void _PyCriticalSection_Begin(PyCriticalSection *c, PyObject *op) @@ -194,6 +183,7 @@ _PyCriticalSection2_BeginMutex(PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2) _PyCriticalSection2_BeginSlow(c, m1, m2, 0); } } +#define PyCriticalSection2_BeginMutex _PyCriticalSection2_BeginMutex static inline void _PyCriticalSection2_Begin(PyCriticalSection2 *c, PyObject *a, PyObject *b) diff --git a/Misc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst b/Misc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst new file mode 100644 index 00000000000000..691cb6036208be --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst @@ -0,0 +1,3 @@ +New variants for the critical section API that accept one or two +:c:type:`PyMutex` instances rather than :c:type:`PyObject` instances are now +public in the non-limited C API. diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index 6a45c11e61af5c..da2b7679e6867d 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -431,7 +431,7 @@ typedef struct { visible to other threads before the `dict_final` bit is set. */ -#define STGINFO_LOCK(stginfo) Py_BEGIN_CRITICAL_SECTION_MUT(&(stginfo)->mutex) +#define STGINFO_LOCK(stginfo) Py_BEGIN_CRITICAL_SECTION_MUTEX(&(stginfo)->mutex) #define STGINFO_UNLOCK() Py_END_CRITICAL_SECTION() static inline uint8_t diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 71fffedee146fa..f7eb29f6e2ee8b 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2419,6 +2419,13 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args)) Py_BEGIN_CRITICAL_SECTION2(module, module); Py_END_CRITICAL_SECTION2(); + PyMutex mut = {0}; + Py_BEGIN_CRITICAL_SECTION_MUTEX(&mut); + Py_END_CRITICAL_SECTION(); + + Py_BEGIN_CRITICAL_SECTION2_MUTEX(&mut, &mut); + Py_END_CRITICAL_SECTION2(); + Py_RETURN_NONE; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b9d549610693c1..48f550025c6b01 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -74,11 +74,11 @@ class object "PyObject *" "&PyBaseObject_Type" // while the stop-the-world mechanism is active. The slots and flags are read // in many places without holding a lock and without atomics. #define TYPE_LOCK &PyInterpreterState_Get()->types.mutex -#define BEGIN_TYPE_LOCK() Py_BEGIN_CRITICAL_SECTION_MUT(TYPE_LOCK) +#define BEGIN_TYPE_LOCK() Py_BEGIN_CRITICAL_SECTION_MUTEX(TYPE_LOCK) #define END_TYPE_LOCK() Py_END_CRITICAL_SECTION() #define BEGIN_TYPE_DICT_LOCK(d) \ - Py_BEGIN_CRITICAL_SECTION2_MUT(TYPE_LOCK, &_PyObject_CAST(d)->ob_mutex) + Py_BEGIN_CRITICAL_SECTION2_MUTEX(TYPE_LOCK, &_PyObject_CAST(d)->ob_mutex) #define END_TYPE_DICT_LOCK() Py_END_CRITICAL_SECTION2() diff --git a/Python/critical_section.c b/Python/critical_section.c index 73857b85496316..e628ba2f6d19bc 100644 --- a/Python/critical_section.c +++ b/Python/critical_section.c @@ -130,6 +130,15 @@ PyCriticalSection_Begin(PyCriticalSection *c, PyObject *op) #endif } +#undef PyCriticalSection_BeginMutex +void +PyCriticalSection_BeginMutex(PyCriticalSection *c, PyMutex *m) +{ +#ifdef Py_GIL_DISABLED + _PyCriticalSection_BeginMutex(c, m); +#endif +} + #undef PyCriticalSection_End void PyCriticalSection_End(PyCriticalSection *c) @@ -148,6 +157,15 @@ PyCriticalSection2_Begin(PyCriticalSection2 *c, PyObject *a, PyObject *b) #endif } +#undef PyCriticalSection2_BeginMutex +void +PyCriticalSection2_BeginMutex(PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2) +{ +#ifdef Py_GIL_DISABLED + _PyCriticalSection2_BeginMutex(c, m1, m2); +#endif +} + #undef PyCriticalSection2_End void PyCriticalSection2_End(PyCriticalSection2 *c) From e0691f5020d308c0a9c8e6d55d27fbe31a372a4f Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 24 Jun 2025 11:37:21 -0600 Subject: [PATCH 2/6] fix copy-paste error --- Include/cpython/critical_section.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/cpython/critical_section.h b/Include/cpython/critical_section.h index 30c67f2beea3a6..4fc46fefb93a24 100644 --- a/Include/cpython/critical_section.h +++ b/Include/cpython/critical_section.h @@ -91,13 +91,13 @@ PyCriticalSection2_End(PyCriticalSection2 *c); #ifndef Py_GIL_DISABLED # define Py_BEGIN_CRITICAL_SECTION(op) \ { -# define Py_BEGIN_CRITICAL_SECTION_MUTEX(mut) \ +# define Py_BEGIN_CRITICAL_SECTION_MUTEX(mutex) \ { # define Py_END_CRITICAL_SECTION() \ } # define Py_BEGIN_CRITICAL_SECTION2(a, b) \ { -# define Py_BEGIN_CRITICAL_SECTION2_MUTEX(mut) \ +# define Py_BEGIN_CRITICAL_SECTION2_MUTEX(m1, m2) \ { # define Py_END_CRITICAL_SECTION2() \ } From 5640b4818284eb6158c5a94287419311ec9772bd Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 24 Jun 2025 11:47:37 -0600 Subject: [PATCH 3/6] avoid compiler warning on GIL-enabled build --- Modules/_testcapimodule.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index f7eb29f6e2ee8b..2572df9719a703 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2419,12 +2419,15 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args)) Py_BEGIN_CRITICAL_SECTION2(module, module); Py_END_CRITICAL_SECTION2(); +#ifdef Py_GIL_DISABLED + // avoid unused variable compiler warning on GIL-enabled build PyMutex mut = {0}; Py_BEGIN_CRITICAL_SECTION_MUTEX(&mut); Py_END_CRITICAL_SECTION(); Py_BEGIN_CRITICAL_SECTION2_MUTEX(&mut, &mut); Py_END_CRITICAL_SECTION2(); +#endif Py_RETURN_NONE; } From 6d987c75f755cc2f06f481a39c8a405dcd5ded6d Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 24 Jun 2025 12:27:14 -0600 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: Sam Gross --- Doc/c-api/init.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index f96fb63a7b486e..75ce0302ecb700 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -2294,7 +2294,7 @@ is resumed, and its locks reacquired. This means the critical section API provides weaker guarantees than traditional locks -- they are useful because their behavior is similar to the :term:`GIL`. -Variants that accept :c:type:`PyMutex` instances rather than objects are also +Variants that accept :c:type:`PyMutex` pointers rather than Python objects are also available. Use these variants to start a critical section in a situation where there is no :c:type:`PyObject` -- for example, when working with a C type that does not extend or wrap :c:type:`PyObject` but still needs to call into the C @@ -2360,7 +2360,7 @@ code triggered by the finalizer blocks and calls :c:func:`PyEval_SaveThread`. On the default build, this macro expands to ``{``. - .. versionadded:: 3.15 + .. versionadded:: next .. c:macro:: Py_END_CRITICAL_SECTION() @@ -2402,11 +2402,11 @@ code triggered by the finalizer blocks and calls :c:func:`PyEval_SaveThread`. PyCriticalSection2_BeginMutex(&_py_cs2, m1, m2) Note that unlike :c:macro:`Py_BEGIN_CRITICAL_SECTION2`, there is no cast for - the second and third arguments - they must be :c:type:`PyMutex` instances. + the second and third arguments - they must be :c:type:`PyMutex` pointers. On the default build, this macro expands to ``{``. - .. versionadded:: 3.15 + .. versionadded:: next .. c:macro:: Py_END_CRITICAL_SECTION2() From 59ac85a6fa8fc3ccdd2612b34dc3838377e70eb5 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 24 Jun 2025 13:19:30 -0600 Subject: [PATCH 5/6] Update changelog --- .../next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst b/Misc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst index 691cb6036208be..41401911a6bc0b 100644 --- a/Misc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst +++ b/Misc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst @@ -1,3 +1,3 @@ New variants for the critical section API that accept one or two -:c:type:`PyMutex` instances rather than :c:type:`PyObject` instances are now +:c:type:`PyMutex` pointers rather than :c:type:`PyObject` instances are now public in the non-limited C API. From 95a0be3efe8ac4895a9cf0acfd7197873aaf54bf Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 3 Jul 2025 11:19:12 -0600 Subject: [PATCH 6/6] fix issue pointed out by Kumar --- Doc/c-api/init.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 75ce0302ecb700..259f43d9f613d7 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -2356,7 +2356,7 @@ code triggered by the finalizer blocks and calls :c:func:`PyEval_SaveThread`. PyCriticalSection_BeginMutex(&_py_cs, m) Note that unlike :c:macro:`Py_BEGIN_CRITICAL_SECTION`, there is no cast for - the second argument - it must be a :c:type:`PyMutex`. + the argument of the macro - it must be a :c:type:`PyMutex` pointer. On the default build, this macro expands to ``{``. @@ -2402,7 +2402,7 @@ code triggered by the finalizer blocks and calls :c:func:`PyEval_SaveThread`. PyCriticalSection2_BeginMutex(&_py_cs2, m1, m2) Note that unlike :c:macro:`Py_BEGIN_CRITICAL_SECTION2`, there is no cast for - the second and third arguments - they must be :c:type:`PyMutex` pointers. + the arguments of the macro - they must be :c:type:`PyMutex` pointers. On the default build, this macro expands to ``{``.