From 3c743f8c4bb76af8a832a7e8762a952c102dceb8 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 10 Apr 2024 20:23:16 +0000 Subject: [PATCH 01/13] gh-117511: Make PyMutex public in the non-limited API --- Doc/c-api/init.rst | 33 +++++++++++++++ Doc/whatsnew/3.13.rst | 5 +++ Include/Python.h | 1 + Include/cpython/lock.h | 59 +++++++++++++++++++++++++++ Include/internal/pycore_lock.h | 64 +----------------------------- Include/lock.h | 16 ++++++++ Include/object.h | 4 -- Makefile.pre.in | 2 + PCbuild/pythoncore.vcxproj | 2 + PCbuild/pythoncore.vcxproj.filters | 6 +++ 10 files changed, 125 insertions(+), 67 deletions(-) create mode 100644 Include/cpython/lock.h create mode 100644 Include/lock.h diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 05f2fd13cf2069..1e658e367fa65a 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -53,6 +53,11 @@ The following functions can be safely called before Python is initialized: * :c:func:`PyMem_RawCalloc` * :c:func:`PyMem_RawFree` +* Synchronization + + * :c:func:`PyMutex_Lock` + * :c:func:`PyMutex_Unlock` + .. note:: The following functions **should not be called** before @@ -1942,3 +1947,31 @@ be used in new code. .. c:function:: void PyThread_delete_key_value(int key) .. c:function:: void PyThread_ReInitTLS() +Synchronization Primitives +========================== + +The C-API provides a basic mutual exclusion lock. + +.. c:type:: PyMutex + + A mutual exclusion lock. The ``PyMutex`` should be initialized to zero to + represent the unlocked state. For example:: + + PyMutex mutex = {0}; + + .. versionadded:: 3.13 + +.. c:function:: void PyMutex_Lock(PyMutex *m) + + Lock the mutex. If another thread has already locked it, the calling + thread will block until the mutex is unlocked. While blocked, the thread + will temporarily release the :term:`GIL` if it is held. + + .. versionadded:: 3.13 + +.. c:function:: void PyMutex_Unlock(PyMutex *m) + + Unlock the mutex. The mutex must be locked --- otherwise, the function will + issue a fatal error. + + .. versionadded:: 3.13 \ No newline at end of file diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index d394fbe3b0c357..ba08dbd324f4f4 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -1813,6 +1813,11 @@ New Features * Add :c:func:`PyType_GetModuleByDef` to the limited C API (Contributed by Victor Stinner in :gh:`116936`.) +* Add :c:type:`PyMutex` API, a lightweight mutex that occupies a single byte. + The :c:func:`PyMutex_Lock` function will release the GIL (if currently held) + if the operation needs to block. + (Contributed by Sam Gross in :gh:`108724`.) + Porting to Python 3.13 ---------------------- diff --git a/Include/Python.h b/Include/Python.h index ca38a98d8c4eca..0ad604a073a57e 100644 --- a/Include/Python.h +++ b/Include/Python.h @@ -55,6 +55,7 @@ #include "pybuffer.h" #include "pystats.h" #include "pyatomic.h" +#include "lock.h" #include "object.h" #include "objimpl.h" #include "typeslots.h" diff --git a/Include/cpython/lock.h b/Include/cpython/lock.h new file mode 100644 index 00000000000000..6d4846eaf827bc --- /dev/null +++ b/Include/cpython/lock.h @@ -0,0 +1,59 @@ +#ifndef Py_CPYTHON_LOCK_H +# error "this header file must not be included directly" +#endif + +#define _Py_UNLOCKED 0 +#define _Py_LOCKED 1 + +// A mutex that occupies one byte. The lock can be zero initialized. +// +// Only the two least significant bits are used. The remaining bits should be +// zero: +// 0b00: unlocked +// 0b01: locked +// 0b10: unlocked and has parked threads +// 0b11: locked and has parked threads +// +// Typical initialization: +// PyMutex m = (PyMutex){0}; +// +// Or initialize as global variables: +// static PyMutex m; +// +// Typical usage: +// PyMutex_Lock(&m); +// ... +// PyMutex_Unlock(&m); +typedef struct _PyMutex { + uint8_t v; +} PyMutex; + +// (private) slow path for locking the mutex +PyAPI_FUNC(void) _PyMutex_LockSlow(PyMutex *m); + +// (private) slow path for unlocking the mutex +PyAPI_FUNC(void) _PyMutex_UnlockSlow(PyMutex *m); + +// Locks the mutex. +// +// If the mutex is currently locked, the calling thread will be parked until +// the mutex is unlocked. If the current thread holds the GIL, then the GIL +// will be released while the thread is parked. +static inline void +PyMutex_Lock(PyMutex *m) +{ + uint8_t expected = _Py_UNLOCKED; + if (!_Py_atomic_compare_exchange_uint8(&m->v, &expected, _Py_LOCKED)) { + _PyMutex_LockSlow(m); + } +} + +// Unlocks the mutex. +static inline void +PyMutex_Unlock(PyMutex *m) +{ + uint8_t expected = _Py_LOCKED; + if (!_Py_atomic_compare_exchange_uint8(&m->v, &expected, _Py_UNLOCKED)) { + _PyMutex_UnlockSlow(m); + } +} \ No newline at end of file diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index f993c95ecbf75a..36f49aede1c4ba 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -13,48 +13,10 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif - -// A mutex that occupies one byte. The lock can be zero initialized. -// -// Only the two least significant bits are used. The remaining bits should be -// zero: -// 0b00: unlocked -// 0b01: locked -// 0b10: unlocked and has parked threads -// 0b11: locked and has parked threads -// -// Typical initialization: -// PyMutex m = (PyMutex){0}; -// -// Or initialize as global variables: -// static PyMutex m; -// -// Typical usage: -// PyMutex_Lock(&m); -// ... -// PyMutex_Unlock(&m); - -// NOTE: In Py_GIL_DISABLED builds, `struct _PyMutex` is defined in Include/object.h. -// The Py_GIL_DISABLED builds need the definition in Include/object.h for the -// `ob_mutex` field in PyObject. For the default (non-free-threaded) build, -// we define the struct here to avoid exposing it in the public API. -#ifndef Py_GIL_DISABLED -struct _PyMutex { uint8_t v; }; -#endif - -typedef struct _PyMutex PyMutex; - -#define _Py_UNLOCKED 0 -#define _Py_LOCKED 1 +//_Py_UNLOCKED is defined as 0 and _Py_LOCKED as 1 in Include/cpython/lock.h #define _Py_HAS_PARKED 2 #define _Py_ONCE_INITIALIZED 4 -// (private) slow path for locking the mutex -PyAPI_FUNC(void) _PyMutex_LockSlow(PyMutex *m); - -// (private) slow path for unlocking the mutex -PyAPI_FUNC(void) _PyMutex_UnlockSlow(PyMutex *m); - static inline int PyMutex_LockFast(uint8_t *lock_bits) { @@ -62,30 +24,6 @@ PyMutex_LockFast(uint8_t *lock_bits) return _Py_atomic_compare_exchange_uint8(lock_bits, &expected, _Py_LOCKED); } -// Locks the mutex. -// -// If the mutex is currently locked, the calling thread will be parked until -// the mutex is unlocked. If the current thread holds the GIL, then the GIL -// will be released while the thread is parked. -static inline void -PyMutex_Lock(PyMutex *m) -{ - uint8_t expected = _Py_UNLOCKED; - if (!_Py_atomic_compare_exchange_uint8(&m->v, &expected, _Py_LOCKED)) { - _PyMutex_LockSlow(m); - } -} - -// Unlocks the mutex. -static inline void -PyMutex_Unlock(PyMutex *m) -{ - uint8_t expected = _Py_LOCKED; - if (!_Py_atomic_compare_exchange_uint8(&m->v, &expected, _Py_UNLOCKED)) { - _PyMutex_UnlockSlow(m); - } -} - // Checks if the mutex is currently locked. static inline int PyMutex_IsLocked(PyMutex *m) diff --git a/Include/lock.h b/Include/lock.h new file mode 100644 index 00000000000000..782b9dbc70d056 --- /dev/null +++ b/Include/lock.h @@ -0,0 +1,16 @@ +#ifndef Py_LOCK_H +#define Py_LOCK_H +#ifdef __cplusplus +extern "C" { +#endif + +#ifndef Py_LIMITED_API +# define Py_CPYTHON_LOCK_H +# include "cpython/lock.h" +# undef Py_CPYTHON_LOCK_H +#endif + +#ifdef __cplusplus +} +#endif +#endif /* !Py_LOCK_H */ diff --git a/Include/object.h b/Include/object.h index 13443329dfb5a2..72127a33e8ac1f 100644 --- a/Include/object.h +++ b/Include/object.h @@ -207,10 +207,6 @@ struct _object { // Create a shared field from a refcnt and desired flags #define _Py_REF_SHARED(refcnt, flags) (((refcnt) << _Py_REF_SHARED_SHIFT) + (flags)) -// NOTE: In non-free-threaded builds, `struct _PyMutex` is defined in -// pycore_lock.h. See pycore_lock.h for more details. -struct _PyMutex { uint8_t v; }; - struct _object { // ob_tid stores the thread id (or zero). It is also used by the GC and the // trashcan mechanism as a linked list pointer and by the GC to store the diff --git a/Makefile.pre.in b/Makefile.pre.in index fd8678cdaf8207..7aa60feb39a10e 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -1013,6 +1013,7 @@ PYTHON_HEADERS= \ $(srcdir)/Include/intrcheck.h \ $(srcdir)/Include/iterobject.h \ $(srcdir)/Include/listobject.h \ + $(srcdir)/Include/lock.h \ $(srcdir)/Include/longobject.h \ $(srcdir)/Include/marshal.h \ $(srcdir)/Include/memoryobject.h \ @@ -1084,6 +1085,7 @@ PYTHON_HEADERS= \ $(srcdir)/Include/cpython/import.h \ $(srcdir)/Include/cpython/initconfig.h \ $(srcdir)/Include/cpython/listobject.h \ + $(srcdir)/Include/cpython/lock.h \ $(srcdir)/Include/cpython/longintrepr.h \ $(srcdir)/Include/cpython/longobject.h \ $(srcdir)/Include/cpython/memoryobject.h \ diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 3a019a5fe550db..984a83d31dd632 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -155,6 +155,7 @@ + @@ -308,6 +309,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index e43970410bd378..efcec68b26017b 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -114,6 +114,9 @@ Include + + Include + Include @@ -393,6 +396,9 @@ Include\cpython + + Include + Include From d340d28f48dd20cd76054b06bf67a49753c0551e Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 10 Apr 2024 16:48:10 -0400 Subject: [PATCH 02/13] Add blurb --- .../next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst diff --git a/Misc/NEWS.d/next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst b/Misc/NEWS.d/next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst new file mode 100644 index 00000000000000..a6082354c6e3dc --- /dev/null +++ b/Misc/NEWS.d/next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst @@ -0,0 +1 @@ +Make ``PyMutex`` public in the non-limited C API. From e59e0f794800994f73e77e0184a997c338a713ad Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 10 Apr 2024 16:52:35 -0400 Subject: [PATCH 03/13] Add missing newline --- Doc/c-api/init.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 1e658e367fa65a..7d21287627dcba 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -1974,4 +1974,4 @@ The C-API provides a basic mutual exclusion lock. Unlock the mutex. The mutex must be locked --- otherwise, the function will issue a fatal error. - .. versionadded:: 3.13 \ No newline at end of file + .. versionadded:: 3.13 From 1cef880f6229672ec7302b194eeb6e3822f0bea0 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 10 Apr 2024 16:58:32 -0400 Subject: [PATCH 04/13] Replace `struct _PyMutex` with `PyMutex` --- Include/cpython/lock.h | 2 +- Include/cpython/weakrefobject.h | 2 +- Include/internal/pycore_warnings.h | 2 +- Include/object.h | 2 +- Objects/object.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Include/cpython/lock.h b/Include/cpython/lock.h index 6d4846eaf827bc..9923e1530964ba 100644 --- a/Include/cpython/lock.h +++ b/Include/cpython/lock.h @@ -24,7 +24,7 @@ // PyMutex_Lock(&m); // ... // PyMutex_Unlock(&m); -typedef struct _PyMutex { +typedef struct PyMutex { uint8_t v; } PyMutex; diff --git a/Include/cpython/weakrefobject.h b/Include/cpython/weakrefobject.h index 9a796098c6b48f..fe695b283547d9 100644 --- a/Include/cpython/weakrefobject.h +++ b/Include/cpython/weakrefobject.h @@ -36,7 +36,7 @@ struct _PyWeakReference { * Normally this can be derived from wr_object, but in some cases we need * to lock after wr_object has been set to Py_None. */ - struct _PyMutex *weakrefs_lock; + PyMutex *weakrefs_lock; #endif }; diff --git a/Include/internal/pycore_warnings.h b/Include/internal/pycore_warnings.h index 114796df42b2b6..f9f6559312f4ef 100644 --- a/Include/internal/pycore_warnings.h +++ b/Include/internal/pycore_warnings.h @@ -14,7 +14,7 @@ struct _warnings_runtime_state { PyObject *filters; /* List */ PyObject *once_registry; /* Dict */ PyObject *default_action; /* String */ - struct _PyMutex mutex; + PyMutex mutex; long filters_version; }; diff --git a/Include/object.h b/Include/object.h index 72127a33e8ac1f..9f5ef3027fae62 100644 --- a/Include/object.h +++ b/Include/object.h @@ -213,7 +213,7 @@ struct _object { // computed "gc_refs" refcount. uintptr_t ob_tid; uint16_t _padding; - struct _PyMutex ob_mutex; // per-object lock + PyMutex ob_mutex; // per-object lock uint8_t ob_gc_bits; // gc-related state uint32_t ob_ref_local; // local reference count Py_ssize_t ob_ref_shared; // shared (atomic) reference count diff --git a/Objects/object.c b/Objects/object.c index c8e6f8fc1a2b40..5ceb9720c7573e 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2368,7 +2368,7 @@ new_reference(PyObject *op) #else op->ob_tid = _Py_ThreadId(); op->_padding = 0; - op->ob_mutex = (struct _PyMutex){ 0 }; + op->ob_mutex = (PyMutex){ 0 }; op->ob_gc_bits = 0; op->ob_ref_local = 1; op->ob_ref_shared = 0; From 28c90bce2d57df7dd31740a9adb060b2231d17d2 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 11 Apr 2024 11:53:45 -0400 Subject: [PATCH 05/13] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Doc/c-api/init.rst | 4 ++-- .../next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 7d21287627dcba..1a1c7708a692b0 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -1963,7 +1963,7 @@ The C-API provides a basic mutual exclusion lock. .. c:function:: void PyMutex_Lock(PyMutex *m) - Lock the mutex. If another thread has already locked it, the calling + Lock mutex *m*. If another thread has already locked it, the calling thread will block until the mutex is unlocked. While blocked, the thread will temporarily release the :term:`GIL` if it is held. @@ -1971,7 +1971,7 @@ The C-API provides a basic mutual exclusion lock. .. c:function:: void PyMutex_Unlock(PyMutex *m) - Unlock the mutex. The mutex must be locked --- otherwise, the function will + Unlock mutex *m*. The mutex must be locked --- otherwise, the function will issue a fatal error. .. versionadded:: 3.13 diff --git a/Misc/NEWS.d/next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst b/Misc/NEWS.d/next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst index a6082354c6e3dc..586685a3407a3d 100644 --- a/Misc/NEWS.d/next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst +++ b/Misc/NEWS.d/next/C API/2024-04-10-16-48-04.gh-issue-117511.RZtBRK.rst @@ -1 +1 @@ -Make ``PyMutex`` public in the non-limited C API. +Make the :c:type:`PyMutex` public in the non-limited C API. From e321817b21be96e2bdcebed4ea42a3e4b57492d6 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 11 Apr 2024 11:53:58 -0400 Subject: [PATCH 06/13] Update Doc/c-api/init.rst Co-authored-by: Erlend E. Aasland --- Doc/c-api/init.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 1a1c7708a692b0..66d1dcf9236b54 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -1954,7 +1954,7 @@ The C-API provides a basic mutual exclusion lock. .. c:type:: PyMutex - A mutual exclusion lock. The ``PyMutex`` should be initialized to zero to + A mutual exclusion lock. The :c:type:`!PyMutex` should be initialized to zero to represent the unlocked state. For example:: PyMutex mutex = {0}; From 6533089b2539b6b90b9ce91e084fd80a7567a90f Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 11 Apr 2024 18:11:41 +0000 Subject: [PATCH 07/13] Use underscore prefix for PyMutex field. Update code comments to further clarify that the `_bits` field is not part of the public C API. --- Include/cpython/lock.h | 24 ++++++++++++---------- Include/internal/pycore_critical_section.h | 6 +++--- Include/internal/pycore_lock.h | 4 ++-- Modules/_testinternalcapi/test_lock.c | 16 +++++++-------- Python/lock.c | 22 ++++++++++---------- 5 files changed, 37 insertions(+), 35 deletions(-) diff --git a/Include/cpython/lock.h b/Include/cpython/lock.h index 9923e1530964ba..e7759f66873868 100644 --- a/Include/cpython/lock.h +++ b/Include/cpython/lock.h @@ -5,14 +5,8 @@ #define _Py_UNLOCKED 0 #define _Py_LOCKED 1 -// A mutex that occupies one byte. The lock can be zero initialized. -// -// Only the two least significant bits are used. The remaining bits should be -// zero: -// 0b00: unlocked -// 0b01: locked -// 0b10: unlocked and has parked threads -// 0b11: locked and has parked threads +// A mutex that occupies one byte. The lock can be zero initialized to +// represent the unlocked state. // // Typical initialization: // PyMutex m = (PyMutex){0}; @@ -24,8 +18,16 @@ // PyMutex_Lock(&m); // ... // PyMutex_Unlock(&m); +// +// The contents of the PyMutex are not part of the public API, but are +// described to aid in understanding the implementation and debugging. Only +// the least significant bits are used. The remaining bits are always zero: +// 0b00: unlocked +// 0b01: locked +// 0b10: unlocked and has parked threads +// 0b11: locked and has parked threads typedef struct PyMutex { - uint8_t v; + uint8_t _bits; // (private) } PyMutex; // (private) slow path for locking the mutex @@ -43,7 +45,7 @@ static inline void PyMutex_Lock(PyMutex *m) { uint8_t expected = _Py_UNLOCKED; - if (!_Py_atomic_compare_exchange_uint8(&m->v, &expected, _Py_LOCKED)) { + if (!_Py_atomic_compare_exchange_uint8(&m->_bits, &expected, _Py_LOCKED)) { _PyMutex_LockSlow(m); } } @@ -53,7 +55,7 @@ static inline void PyMutex_Unlock(PyMutex *m) { uint8_t expected = _Py_LOCKED; - if (!_Py_atomic_compare_exchange_uint8(&m->v, &expected, _Py_UNLOCKED)) { + if (!_Py_atomic_compare_exchange_uint8(&m->_bits, &expected, _Py_UNLOCKED)) { _PyMutex_UnlockSlow(m); } } \ No newline at end of file diff --git a/Include/internal/pycore_critical_section.h b/Include/internal/pycore_critical_section.h index 23b85c2f9e9bb2..a462ec3f24b732 100644 --- a/Include/internal/pycore_critical_section.h +++ b/Include/internal/pycore_critical_section.h @@ -191,7 +191,7 @@ _PyCriticalSection2_BeginSlow(_PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2, static inline void _PyCriticalSection_Begin(_PyCriticalSection *c, PyMutex *m) { - if (PyMutex_LockFast(&m->v)) { + if (PyMutex_LockFast(&m->_bits)) { PyThreadState *tstate = _PyThreadState_GET(); c->mutex = m; c->prev = tstate->critical_section; @@ -262,8 +262,8 @@ _PyCriticalSection2_Begin(_PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2) m2 = tmp; } - if (PyMutex_LockFast(&m1->v)) { - if (PyMutex_LockFast(&m2->v)) { + if (PyMutex_LockFast(&m1->_bits)) { + if (PyMutex_LockFast(&m2->_bits)) { PyThreadState *tstate = _PyThreadState_GET(); c->base.mutex = m1; c->mutex2 = m2; diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index 36f49aede1c4ba..025d0f6098cff0 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -28,7 +28,7 @@ PyMutex_LockFast(uint8_t *lock_bits) static inline int PyMutex_IsLocked(PyMutex *m) { - return (_Py_atomic_load_uint8(&m->v) & _Py_LOCKED) != 0; + return (_Py_atomic_load_uint8(&m->_bits) & _Py_LOCKED) != 0; } // Re-initializes the mutex after a fork to the unlocked state. @@ -59,7 +59,7 @@ static inline void PyMutex_LockFlags(PyMutex *m, _PyLockFlags flags) { uint8_t expected = _Py_UNLOCKED; - if (!_Py_atomic_compare_exchange_uint8(&m->v, &expected, _Py_LOCKED)) { + if (!_Py_atomic_compare_exchange_uint8(&m->_bits, &expected, _Py_LOCKED)) { _PyMutex_LockTimed(m, -1, flags); } } diff --git a/Modules/_testinternalcapi/test_lock.c b/Modules/_testinternalcapi/test_lock.c index 1c5048170e9f2e..a4bb4246b27934 100644 --- a/Modules/_testinternalcapi/test_lock.c +++ b/Modules/_testinternalcapi/test_lock.c @@ -36,9 +36,9 @@ test_lock_basic(PyObject *self, PyObject *obj) // uncontended lock and unlock PyMutex_Lock(&m); - assert(m.v == 1); + assert(m._bits == 1); PyMutex_Unlock(&m); - assert(m.v == 0); + assert(m._bits == 0); Py_RETURN_NONE; } @@ -57,10 +57,10 @@ lock_thread(void *arg) _Py_atomic_store_int(&test_data->started, 1); PyMutex_Lock(m); - assert(m->v == 1); + assert(m->_bits == 1); PyMutex_Unlock(m); - assert(m->v == 0); + assert(m->_bits == 0); _PyEvent_Notify(&test_data->done); } @@ -73,7 +73,7 @@ test_lock_two_threads(PyObject *self, PyObject *obj) memset(&test_data, 0, sizeof(test_data)); PyMutex_Lock(&test_data.m); - assert(test_data.m.v == 1); + assert(test_data.m._bits == 1); PyThread_start_new_thread(lock_thread, &test_data); @@ -82,17 +82,17 @@ test_lock_two_threads(PyObject *self, PyObject *obj) uint8_t v; do { pysleep(10); // allow some time for the other thread to try to lock - v = _Py_atomic_load_uint8_relaxed(&test_data.m.v); + v = _Py_atomic_load_uint8_relaxed(&test_data.m._bits); assert(v == 1 || v == 3); iters++; } while (v != 3 && iters < 200); // both the "locked" and the "has parked" bits should be set - assert(test_data.m.v == 3); + assert(test_data.m._bits == 3); PyMutex_Unlock(&test_data.m); PyEvent_Wait(&test_data.done); - assert(test_data.m.v == 0); + assert(test_data.m._bits == 0); Py_RETURN_NONE; } diff --git a/Python/lock.c b/Python/lock.c index 7d1ead585dee6c..e5233a91b238f8 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -56,9 +56,9 @@ _PyMutex_LockSlow(PyMutex *m) PyLockStatus _PyMutex_LockTimed(PyMutex *m, PyTime_t timeout, _PyLockFlags flags) { - uint8_t v = _Py_atomic_load_uint8_relaxed(&m->v); + uint8_t v = _Py_atomic_load_uint8_relaxed(&m->_bits); if ((v & _Py_LOCKED) == 0) { - if (_Py_atomic_compare_exchange_uint8(&m->v, &v, v|_Py_LOCKED)) { + if (_Py_atomic_compare_exchange_uint8(&m->_bits, &v, v|_Py_LOCKED)) { return PY_LOCK_ACQUIRED; } } @@ -81,7 +81,7 @@ _PyMutex_LockTimed(PyMutex *m, PyTime_t timeout, _PyLockFlags flags) for (;;) { if ((v & _Py_LOCKED) == 0) { // The lock is unlocked. Try to grab it. - if (_Py_atomic_compare_exchange_uint8(&m->v, &v, v|_Py_LOCKED)) { + if (_Py_atomic_compare_exchange_uint8(&m->_bits, &v, v|_Py_LOCKED)) { return PY_LOCK_ACQUIRED; } continue; @@ -102,17 +102,17 @@ _PyMutex_LockTimed(PyMutex *m, PyTime_t timeout, _PyLockFlags flags) if (!(v & _Py_HAS_PARKED)) { // We are the first waiter. Set the _Py_HAS_PARKED flag. newv = v | _Py_HAS_PARKED; - if (!_Py_atomic_compare_exchange_uint8(&m->v, &v, newv)) { + if (!_Py_atomic_compare_exchange_uint8(&m->_bits, &v, newv)) { continue; } } - int ret = _PyParkingLot_Park(&m->v, &newv, sizeof(newv), timeout, + int ret = _PyParkingLot_Park(&m->_bits, &newv, sizeof(newv), timeout, &entry, (flags & _PY_LOCK_DETACH) != 0); if (ret == Py_PARK_OK) { if (entry.handed_off) { // We own the lock now. - assert(_Py_atomic_load_uint8_relaxed(&m->v) & _Py_LOCKED); + assert(_Py_atomic_load_uint8_relaxed(&m->_bits) & _Py_LOCKED); return PY_LOCK_ACQUIRED; } } @@ -134,7 +134,7 @@ _PyMutex_LockTimed(PyMutex *m, PyTime_t timeout, _PyLockFlags flags) } } - v = _Py_atomic_load_uint8_relaxed(&m->v); + v = _Py_atomic_load_uint8_relaxed(&m->_bits); } } @@ -154,13 +154,13 @@ mutex_unpark(PyMutex *m, struct mutex_entry *entry, int has_more_waiters) v |= _Py_HAS_PARKED; } } - _Py_atomic_store_uint8(&m->v, v); + _Py_atomic_store_uint8(&m->_bits, v); } int _PyMutex_TryUnlock(PyMutex *m) { - uint8_t v = _Py_atomic_load_uint8(&m->v); + uint8_t v = _Py_atomic_load_uint8(&m->_bits); for (;;) { if ((v & _Py_LOCKED) == 0) { // error: the mutex is not locked @@ -168,10 +168,10 @@ _PyMutex_TryUnlock(PyMutex *m) } else if ((v & _Py_HAS_PARKED)) { // wake up a single thread - _PyParkingLot_Unpark(&m->v, (_Py_unpark_fn_t *)mutex_unpark, m); + _PyParkingLot_Unpark(&m->_bits, (_Py_unpark_fn_t *)mutex_unpark, m); return 0; } - else if (_Py_atomic_compare_exchange_uint8(&m->v, &v, _Py_UNLOCKED)) { + else if (_Py_atomic_compare_exchange_uint8(&m->_bits, &v, _Py_UNLOCKED)) { // fast-path: no waiters return 0; } From 215020a895869239bd9a58e34a55e96e5065b367 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 11 Apr 2024 18:58:30 +0000 Subject: [PATCH 08/13] Fix comment --- Include/cpython/lock.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/cpython/lock.h b/Include/cpython/lock.h index e7759f66873868..17b65bf6402795 100644 --- a/Include/cpython/lock.h +++ b/Include/cpython/lock.h @@ -21,7 +21,7 @@ // // The contents of the PyMutex are not part of the public API, but are // described to aid in understanding the implementation and debugging. Only -// the least significant bits are used. The remaining bits are always zero: +// the two least significant bits are used. The remaining bits are always zero: // 0b00: unlocked // 0b01: locked // 0b10: unlocked and has parked threads @@ -58,4 +58,4 @@ PyMutex_Unlock(PyMutex *m) if (!_Py_atomic_compare_exchange_uint8(&m->_bits, &expected, _Py_UNLOCKED)) { _PyMutex_UnlockSlow(m); } -} \ No newline at end of file +} From c0f75f97c47b4a632384bba205b44d9ebd67a29d Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 29 Apr 2024 21:16:49 +0000 Subject: [PATCH 09/13] PyMutex should not be copied or moved --- Doc/c-api/init.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 66d1dcf9236b54..7a8897d9cf5f2e 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -1959,6 +1959,9 @@ The C-API provides a basic mutual exclusion lock. PyMutex mutex = {0}; + Instances of :c:type:`!PyMutex` should not be copied or moved. Both the + contents and address of a :c:type:`!PyMutex` are meaningful. + .. versionadded:: 3.13 .. c:function:: void PyMutex_Lock(PyMutex *m) From 19a34922baf686ce4e3da3200c9ae4fde5f336a8 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 17 Jun 2024 19:23:05 +0000 Subject: [PATCH 10/13] Update documentation --- Doc/c-api/init.rst | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index fb8d306627a1d2..5b638c1f4b49b6 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -2164,13 +2164,20 @@ The C-API provides a basic mutual exclusion lock. .. c:type:: PyMutex - A mutual exclusion lock. The :c:type:`!PyMutex` should be initialized to zero to - represent the unlocked state. For example:: + A mutual exclusion lock. The :c:type:`!PyMutex` should be initialized to + zero to represent the unlocked state. For example:: PyMutex mutex = {0}; Instances of :c:type:`!PyMutex` should not be copied or moved. Both the - contents and address of a :c:type:`!PyMutex` are meaningful. + contents and address of a :c:type:`!PyMutex` are meaningful, and it must + remain at a fixed, writable location in memory. + + .. note:: + + A :c:type:`!PyMutex` currently occupies one byte, but the size should be + considered unstable. The size may change in future Python releases + without a deprecation period. .. versionadded:: 3.13 From 72301341ee181fe55c0847d4f01ec10f0adb56f9 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 17 Jun 2024 19:56:33 +0000 Subject: [PATCH 11/13] Export PyMutex_Lock and PyMutex_Unlock as regular functions --- Include/cpython/lock.h | 18 ++++++++++-------- Python/critical_section.c | 2 +- Python/lock.c | 6 ++++-- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Include/cpython/lock.h b/Include/cpython/lock.h index 17b65bf6402795..8ee03e82f74dfd 100644 --- a/Include/cpython/lock.h +++ b/Include/cpython/lock.h @@ -30,11 +30,11 @@ typedef struct PyMutex { uint8_t _bits; // (private) } PyMutex; -// (private) slow path for locking the mutex -PyAPI_FUNC(void) _PyMutex_LockSlow(PyMutex *m); +// exported function for locking the mutex +PyAPI_FUNC(void) PyMutex_Lock(PyMutex *m); -// (private) slow path for unlocking the mutex -PyAPI_FUNC(void) _PyMutex_UnlockSlow(PyMutex *m); +// exported function for unlocking the mutex +PyAPI_FUNC(void) PyMutex_Unlock(PyMutex *m); // Locks the mutex. // @@ -42,20 +42,22 @@ PyAPI_FUNC(void) _PyMutex_UnlockSlow(PyMutex *m); // the mutex is unlocked. If the current thread holds the GIL, then the GIL // will be released while the thread is parked. static inline void -PyMutex_Lock(PyMutex *m) +_PyMutex_Lock(PyMutex *m) { uint8_t expected = _Py_UNLOCKED; if (!_Py_atomic_compare_exchange_uint8(&m->_bits, &expected, _Py_LOCKED)) { - _PyMutex_LockSlow(m); + PyMutex_Lock(m); } } +#define PyMutex_Lock _PyMutex_Lock // Unlocks the mutex. static inline void -PyMutex_Unlock(PyMutex *m) +_PyMutex_Unlock(PyMutex *m) { uint8_t expected = _Py_LOCKED; if (!_Py_atomic_compare_exchange_uint8(&m->_bits, &expected, _Py_UNLOCKED)) { - _PyMutex_UnlockSlow(m); + PyMutex_Unlock(m); } } +#define PyMutex_Unlock _PyMutex_Unlock diff --git a/Python/critical_section.c b/Python/critical_section.c index 2214d80eeb297b..ac679ac3f8a019 100644 --- a/Python/critical_section.c +++ b/Python/critical_section.c @@ -14,7 +14,7 @@ _PyCriticalSection_BeginSlow(_PyCriticalSection *c, PyMutex *m) c->prev = (uintptr_t)tstate->critical_section; tstate->critical_section = (uintptr_t)c; - _PyMutex_LockSlow(m); + PyMutex_Lock(m); c->mutex = m; } diff --git a/Python/lock.c b/Python/lock.c index f57d64d6b89850..f242c295b452b7 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -47,8 +47,9 @@ _Py_yield(void) #endif } +#undef PyMutex_Lock void -_PyMutex_LockSlow(PyMutex *m) +PyMutex_Lock(PyMutex *m) { _PyMutex_LockTimed(m, -1, _PY_LOCK_DETACH); } @@ -182,8 +183,9 @@ _PyMutex_TryUnlock(PyMutex *m) } } +#undef PyMutex_Unlock void -_PyMutex_UnlockSlow(PyMutex *m) +PyMutex_Unlock(PyMutex *m) { if (_PyMutex_TryUnlock(m) < 0) { Py_FatalError("unlocking mutex that is not locked"); From 6b2d3774b23aa8795a64415ace2930a343e013da Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 18 Jun 2024 14:24:07 +0000 Subject: [PATCH 12/13] Move PyMutex_Lock/Unlock definitions to end of file --- Python/lock.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/Python/lock.c b/Python/lock.c index f242c295b452b7..7c6a5175e88ff1 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -47,13 +47,6 @@ _Py_yield(void) #endif } -#undef PyMutex_Lock -void -PyMutex_Lock(PyMutex *m) -{ - _PyMutex_LockTimed(m, -1, _PY_LOCK_DETACH); -} - PyLockStatus _PyMutex_LockTimed(PyMutex *m, PyTime_t timeout, _PyLockFlags flags) { @@ -183,15 +176,6 @@ _PyMutex_TryUnlock(PyMutex *m) } } -#undef PyMutex_Unlock -void -PyMutex_Unlock(PyMutex *m) -{ - if (_PyMutex_TryUnlock(m) < 0) { - Py_FatalError("unlocking mutex that is not locked"); - } -} - // _PyRawMutex stores a linked list of `struct raw_mutex_entry`, one for each // thread waiting on the mutex, directly in the mutex itself. struct raw_mutex_entry { @@ -586,3 +570,19 @@ uint32_t _PySeqLock_AfterFork(_PySeqLock *seqlock) return 0; } + +#undef PyMutex_Lock +void +PyMutex_Lock(PyMutex *m) +{ + _PyMutex_LockTimed(m, -1, _PY_LOCK_DETACH); +} + +#undef PyMutex_Unlock +void +PyMutex_Unlock(PyMutex *m) +{ + if (_PyMutex_TryUnlock(m) < 0) { + Py_FatalError("unlocking mutex that is not locked"); + } +} From 476e493470592247807e886eab3fd4f338bf60aa Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 19 Jun 2024 09:35:39 +0200 Subject: [PATCH 13/13] Update Doc/c-api/init.rst --- Doc/c-api/init.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index 5b638c1f4b49b6..58c79031de5320 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -55,7 +55,7 @@ The following functions can be safely called before Python is initialized: * :c:func:`PyMem_RawCalloc` * :c:func:`PyMem_RawFree` -* Synchronization +* Synchronization: * :c:func:`PyMutex_Lock` * :c:func:`PyMutex_Unlock`