From a8cbd7c752848da938a8e1ff83a8bb1305b80960 Mon Sep 17 00:00:00 2001 From: Timofey Ivankov Date: Sat, 8 Aug 2026 17:21:45 +0300 Subject: [PATCH] gh-154928: Fix data race on the recursion count of threading.RLock --- Include/cpython/pyatomic.h | 6 ++++++ Include/cpython/pyatomic_gcc.h | 8 ++++++++ Include/cpython/pyatomic_msc.h | 12 ++++++++++++ Include/cpython/pyatomic_std.h | 16 ++++++++++++++++ Include/internal/pycore_pyatomic_ft_wrappers.h | 6 ++++++ Lib/test/test_free_threading/test_threading.py | 18 ++++++++++++++++++ ...6-08-08-14-58-34.gh-issue-154928.wJtXhe.rst | 2 ++ Modules/_threadmodule.c | 8 +++++--- Python/lock.c | 7 ++++--- 9 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-08-08-14-58-34.gh-issue-154928.wJtXhe.rst diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h index e85b360c986668c..ea9e15daa21008a 100644 --- a/Include/cpython/pyatomic.h +++ b/Include/cpython/pyatomic.h @@ -378,6 +378,9 @@ _Py_atomic_load_uint_relaxed(const unsigned int *obj); static inline Py_ssize_t _Py_atomic_load_ssize_relaxed(const Py_ssize_t *obj); +static inline size_t +_Py_atomic_load_size_relaxed(const size_t *obj); + static inline void * _Py_atomic_load_ptr_relaxed(const void *obj); @@ -475,6 +478,9 @@ _Py_atomic_store_ptr_relaxed(void *obj, void *value); static inline void _Py_atomic_store_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value); +static inline void +_Py_atomic_store_size_relaxed(size_t *obj, size_t value); + static inline void _Py_atomic_store_ullong_relaxed(unsigned long long *obj, unsigned long long value); diff --git a/Include/cpython/pyatomic_gcc.h b/Include/cpython/pyatomic_gcc.h index 253b35082aafcd2..edf0a4fa39ef21e 100644 --- a/Include/cpython/pyatomic_gcc.h +++ b/Include/cpython/pyatomic_gcc.h @@ -382,6 +382,10 @@ static inline Py_ssize_t _Py_atomic_load_ssize_relaxed(const Py_ssize_t *obj) { return __atomic_load_n(obj, __ATOMIC_RELAXED); } +static inline size_t +_Py_atomic_load_size_relaxed(const size_t *obj) +{ return __atomic_load_n(obj, __ATOMIC_RELAXED); } + static inline void * _Py_atomic_load_ptr_relaxed(const void *obj) { return (void *)__atomic_load_n((void * const *)obj, __ATOMIC_RELAXED); } @@ -512,6 +516,10 @@ static inline void _Py_atomic_store_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value) { __atomic_store_n(obj, value, __ATOMIC_RELAXED); } +static inline void +_Py_atomic_store_size_relaxed(size_t *obj, size_t value) +{ __atomic_store_n(obj, value, __ATOMIC_RELAXED); } + static inline void _Py_atomic_store_ullong_relaxed(unsigned long long *obj, unsigned long long value) diff --git a/Include/cpython/pyatomic_msc.h b/Include/cpython/pyatomic_msc.h index 3b3c5f7017e9575..00377a8f76b45c5 100644 --- a/Include/cpython/pyatomic_msc.h +++ b/Include/cpython/pyatomic_msc.h @@ -748,6 +748,12 @@ _Py_atomic_load_ssize_relaxed(const Py_ssize_t *obj) return *(volatile Py_ssize_t *)obj; } +static inline size_t +_Py_atomic_load_size_relaxed(const size_t *obj) +{ + return *(volatile size_t *)obj; +} + static inline void* _Py_atomic_load_ptr_relaxed(const void *obj) { @@ -940,6 +946,12 @@ _Py_atomic_store_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value) *(volatile Py_ssize_t *)obj = value; } +static inline void +_Py_atomic_store_size_relaxed(size_t *obj, size_t value) +{ + *(volatile size_t *)obj = value; +} + static inline void _Py_atomic_store_ullong_relaxed(unsigned long long *obj, unsigned long long value) diff --git a/Include/cpython/pyatomic_std.h b/Include/cpython/pyatomic_std.h index faef303da70314c..be3264c1db093c3 100644 --- a/Include/cpython/pyatomic_std.h +++ b/Include/cpython/pyatomic_std.h @@ -667,6 +667,14 @@ _Py_atomic_load_ssize_relaxed(const Py_ssize_t *obj) memory_order_relaxed); } +static inline size_t +_Py_atomic_load_size_relaxed(const size_t *obj) +{ + _Py_USING_STD; + return atomic_load_explicit((const _Atomic(size_t)*)obj, + memory_order_relaxed); +} + static inline void* _Py_atomic_load_ptr_relaxed(const void *obj) { @@ -907,6 +915,14 @@ _Py_atomic_store_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value) memory_order_relaxed); } +static inline void +_Py_atomic_store_size_relaxed(size_t *obj, size_t value) +{ + _Py_USING_STD; + atomic_store_explicit((_Atomic(size_t)*)obj, value, + memory_order_relaxed); +} + static inline void _Py_atomic_store_ullong_relaxed(unsigned long long *obj, unsigned long long value) diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index d8ec306a0dae3fc..a65258050730a8b 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -27,6 +27,8 @@ extern "C" { _Py_atomic_load_ssize_acquire(&value) #define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) \ _Py_atomic_load_ssize_relaxed(&value) +#define FT_ATOMIC_LOAD_SIZE_RELAXED(value) \ + _Py_atomic_load_size_relaxed(&value) #define FT_ATOMIC_STORE_PTR(value, new_value) \ _Py_atomic_store_ptr(&value, new_value) #define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) \ @@ -67,6 +69,8 @@ extern "C" { _Py_atomic_store_int8_release(&value, new_value) #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \ _Py_atomic_store_ssize_relaxed(&value, new_value) +#define FT_ATOMIC_STORE_SIZE_RELAXED(value, new_value) \ + _Py_atomic_store_size_relaxed(&value, new_value) #define FT_ATOMIC_STORE_SSIZE_RELEASE(value, new_value) \ _Py_atomic_store_ssize_release(&value, new_value) #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \ @@ -147,6 +151,7 @@ extern "C" { #define FT_ATOMIC_LOAD_SSIZE(value) value #define FT_ATOMIC_LOAD_SSIZE_ACQUIRE(value) value #define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value +#define FT_ATOMIC_LOAD_SIZE_RELAXED(value) value #define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) value #define FT_ATOMIC_LOAD_PTR_CONSUME(value) value #define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) value @@ -166,6 +171,7 @@ extern "C" { #define FT_ATOMIC_STORE_INT8_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_INT8_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_STORE_SIZE_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_SSIZE_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value diff --git a/Lib/test/test_free_threading/test_threading.py b/Lib/test/test_free_threading/test_threading.py index b5a5ca272b9405a..421a65a206efd36 100644 --- a/Lib/test/test_free_threading/test_threading.py +++ b/Lib/test/test_free_threading/test_threading.py @@ -21,6 +21,24 @@ def mutate_thread(): threading_helper.run_concurrently([repr_thread, mutate_thread]) + def test_recursion_count_race(self): + # gh-154928: repr() reads the count while another thread updates it + import _thread + r = _thread.RLock() + + def repr_thread(): + for _ in range(2000): + repr(r) + + def recurse_thread(): + for _ in range(2000): + r.acquire() + r.acquire() + r.release() + r.release() + + threading_helper.run_concurrently([repr_thread, recurse_thread]) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Library/2026-08-08-14-58-34.gh-issue-154928.wJtXhe.rst b/Misc/NEWS.d/next/Library/2026-08-08-14-58-34.gh-issue-154928.wJtXhe.rst new file mode 100644 index 000000000000000..8ca2d843acaefd0 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-08-08-14-58-34.gh-issue-154928.wJtXhe.rst @@ -0,0 +1,2 @@ +Fix data race on the recursion count of :class:`threading.RLock` in the +:term:`free-threaded build`. diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 199e4ac3db723bf..836af5bf3b29416 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -8,6 +8,7 @@ #include "pycore_modsupport.h" // _PyArg_NoKeywords() #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_object_deferred.h" // _PyObject_SetDeferredRefcount() +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_SIZE_RELAXED() #include "pycore_pylifecycle.h" #include "pycore_pystate.h" // _PyThreadState_SetCurrent() #include "pycore_time.h" // _PyTime_FromSeconds() @@ -1207,7 +1208,7 @@ _thread_RLock__acquire_restore_impl(rlockobject *self, PyObject *state) _PyRecursiveMutex_Lock(&self->lock); _Py_atomic_store_ullong_relaxed(&self->lock.thread, owner); - self->lock.level = (size_t)count - 1; + FT_ATOMIC_STORE_SIZE_RELAXED(self->lock.level, (size_t)count - 1); Py_RETURN_NONE; } @@ -1230,7 +1231,8 @@ _thread_RLock__release_save_impl(rlockobject *self) PyThread_ident_t owner = self->lock.thread; Py_ssize_t count = self->lock.level + 1; - self->lock.level = 0; // ensure the unlock releases the lock + // ensure the unlock releases the lock + FT_ATOMIC_STORE_SIZE_RELAXED(self->lock.level, 0); _PyRecursiveMutex_Unlock(&self->lock); return Py_BuildValue("n" Py_PARSE_THREAD_IDENT_T, count, owner); } @@ -1292,7 +1294,7 @@ rlock_repr(PyObject *op) int locked = rlock_locked_impl(self); size_t count; if (locked) { - count = self->lock.level + 1; + count = FT_ATOMIC_LOAD_SIZE_RELAXED(self->lock.level) + 1; } else { count = 0; diff --git a/Python/lock.c b/Python/lock.c index b636b91e79678c6..64898755b471524 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -4,6 +4,7 @@ #include "pycore_lock.h" #include "pycore_parking_lot.h" +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_STORE_SIZE_RELAXED() #include "pycore_semaphore.h" #include "pycore_time.h" // _PyTime_Add() #include "pycore_stats.h" // FT_STAT_MUTEX_SLEEP_INC() @@ -418,7 +419,7 @@ _PyRecursiveMutex_Lock(_PyRecursiveMutex *m) { PyThread_ident_t thread = PyThread_get_thread_ident_ex(); if (recursive_mutex_is_owned_by(m, thread)) { - m->level++; + FT_ATOMIC_STORE_SIZE_RELAXED(m->level, m->level + 1); return; } PyMutex_Lock(&m->mutex); @@ -431,7 +432,7 @@ _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags { PyThread_ident_t thread = PyThread_get_thread_ident_ex(); if (recursive_mutex_is_owned_by(m, thread)) { - m->level++; + FT_ATOMIC_STORE_SIZE_RELAXED(m->level, m->level + 1); return PY_LOCK_ACQUIRED; } PyLockStatus s = _PyMutex_LockTimed(&m->mutex, timeout, flags); @@ -459,7 +460,7 @@ _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m) return -1; } if (m->level > 0) { - m->level--; + FT_ATOMIC_STORE_SIZE_RELAXED(m->level, m->level - 1); return 0; } assert(m->level == 0);