Skip to content

Commit a8cbd7c

Browse files
deadlovellldeadlovelll
committed
gh-154928: Fix data race on the recursion count of threading.RLock
1 parent 8ed1479 commit a8cbd7c

9 files changed

Lines changed: 77 additions & 6 deletions

File tree

‎Include/cpython/pyatomic.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,9 @@ _Py_atomic_load_uint_relaxed(const unsigned int *obj);
378378
static inline Py_ssize_t
379379
_Py_atomic_load_ssize_relaxed(const Py_ssize_t *obj);
380380

381+
static inline size_t
382+
_Py_atomic_load_size_relaxed(const size_t *obj);
383+
381384
static inline void *
382385
_Py_atomic_load_ptr_relaxed(const void *obj);
383386

@@ -475,6 +478,9 @@ _Py_atomic_store_ptr_relaxed(void *obj, void *value);
475478
static inline void
476479
_Py_atomic_store_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value);
477480

481+
static inline void
482+
_Py_atomic_store_size_relaxed(size_t *obj, size_t value);
483+
478484
static inline void
479485
_Py_atomic_store_ullong_relaxed(unsigned long long *obj,
480486
unsigned long long value);

‎Include/cpython/pyatomic_gcc.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,10 @@ static inline Py_ssize_t
382382
_Py_atomic_load_ssize_relaxed(const Py_ssize_t *obj)
383383
{ return __atomic_load_n(obj, __ATOMIC_RELAXED); }
384384

385+
static inline size_t
386+
_Py_atomic_load_size_relaxed(const size_t *obj)
387+
{ return __atomic_load_n(obj, __ATOMIC_RELAXED); }
388+
385389
static inline void *
386390
_Py_atomic_load_ptr_relaxed(const void *obj)
387391
{ return (void *)__atomic_load_n((void * const *)obj, __ATOMIC_RELAXED); }
@@ -512,6 +516,10 @@ static inline void
512516
_Py_atomic_store_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value)
513517
{ __atomic_store_n(obj, value, __ATOMIC_RELAXED); }
514518

519+
static inline void
520+
_Py_atomic_store_size_relaxed(size_t *obj, size_t value)
521+
{ __atomic_store_n(obj, value, __ATOMIC_RELAXED); }
522+
515523
static inline void
516524
_Py_atomic_store_ullong_relaxed(unsigned long long *obj,
517525
unsigned long long value)

‎Include/cpython/pyatomic_msc.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,12 @@ _Py_atomic_load_ssize_relaxed(const Py_ssize_t *obj)
748748
return *(volatile Py_ssize_t *)obj;
749749
}
750750

751+
static inline size_t
752+
_Py_atomic_load_size_relaxed(const size_t *obj)
753+
{
754+
return *(volatile size_t *)obj;
755+
}
756+
751757
static inline void*
752758
_Py_atomic_load_ptr_relaxed(const void *obj)
753759
{
@@ -940,6 +946,12 @@ _Py_atomic_store_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value)
940946
*(volatile Py_ssize_t *)obj = value;
941947
}
942948

949+
static inline void
950+
_Py_atomic_store_size_relaxed(size_t *obj, size_t value)
951+
{
952+
*(volatile size_t *)obj = value;
953+
}
954+
943955
static inline void
944956
_Py_atomic_store_ullong_relaxed(unsigned long long *obj,
945957
unsigned long long value)

‎Include/cpython/pyatomic_std.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,14 @@ _Py_atomic_load_ssize_relaxed(const Py_ssize_t *obj)
667667
memory_order_relaxed);
668668
}
669669

670+
static inline size_t
671+
_Py_atomic_load_size_relaxed(const size_t *obj)
672+
{
673+
_Py_USING_STD;
674+
return atomic_load_explicit((const _Atomic(size_t)*)obj,
675+
memory_order_relaxed);
676+
}
677+
670678
static inline void*
671679
_Py_atomic_load_ptr_relaxed(const void *obj)
672680
{
@@ -907,6 +915,14 @@ _Py_atomic_store_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value)
907915
memory_order_relaxed);
908916
}
909917

918+
static inline void
919+
_Py_atomic_store_size_relaxed(size_t *obj, size_t value)
920+
{
921+
_Py_USING_STD;
922+
atomic_store_explicit((_Atomic(size_t)*)obj, value,
923+
memory_order_relaxed);
924+
}
925+
910926
static inline void
911927
_Py_atomic_store_ullong_relaxed(unsigned long long *obj,
912928
unsigned long long value)

‎Include/internal/pycore_pyatomic_ft_wrappers.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ extern "C" {
2727
_Py_atomic_load_ssize_acquire(&value)
2828
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) \
2929
_Py_atomic_load_ssize_relaxed(&value)
30+
#define FT_ATOMIC_LOAD_SIZE_RELAXED(value) \
31+
_Py_atomic_load_size_relaxed(&value)
3032
#define FT_ATOMIC_STORE_PTR(value, new_value) \
3133
_Py_atomic_store_ptr(&value, new_value)
3234
#define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) \
@@ -67,6 +69,8 @@ extern "C" {
6769
_Py_atomic_store_int8_release(&value, new_value)
6870
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \
6971
_Py_atomic_store_ssize_relaxed(&value, new_value)
72+
#define FT_ATOMIC_STORE_SIZE_RELAXED(value, new_value) \
73+
_Py_atomic_store_size_relaxed(&value, new_value)
7074
#define FT_ATOMIC_STORE_SSIZE_RELEASE(value, new_value) \
7175
_Py_atomic_store_ssize_release(&value, new_value)
7276
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \
@@ -147,6 +151,7 @@ extern "C" {
147151
#define FT_ATOMIC_LOAD_SSIZE(value) value
148152
#define FT_ATOMIC_LOAD_SSIZE_ACQUIRE(value) value
149153
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value
154+
#define FT_ATOMIC_LOAD_SIZE_RELAXED(value) value
150155
#define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) value
151156
#define FT_ATOMIC_LOAD_PTR_CONSUME(value) value
152157
#define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) value
@@ -166,6 +171,7 @@ extern "C" {
166171
#define FT_ATOMIC_STORE_INT8_RELAXED(value, new_value) value = new_value
167172
#define FT_ATOMIC_STORE_INT8_RELEASE(value, new_value) value = new_value
168173
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value
174+
#define FT_ATOMIC_STORE_SIZE_RELAXED(value, new_value) value = new_value
169175
#define FT_ATOMIC_STORE_SSIZE_RELEASE(value, new_value) value = new_value
170176
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value
171177
#define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value

‎Lib/test/test_free_threading/test_threading.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,24 @@ def mutate_thread():
2121

2222
threading_helper.run_concurrently([repr_thread, mutate_thread])
2323

24+
def test_recursion_count_race(self):
25+
# gh-154928: repr() reads the count while another thread updates it
26+
import _thread
27+
r = _thread.RLock()
28+
29+
def repr_thread():
30+
for _ in range(2000):
31+
repr(r)
32+
33+
def recurse_thread():
34+
for _ in range(2000):
35+
r.acquire()
36+
r.acquire()
37+
r.release()
38+
r.release()
39+
40+
threading_helper.run_concurrently([repr_thread, recurse_thread])
41+
2442

2543
if __name__ == "__main__":
2644
unittest.main()

‎Misc/NEWS.d/next/Library/2026-08-08-14-58-34.gh-issue-154928.wJtXhe.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix data race on the recursion count of :class:`threading.RLock` in the
2+
:term:`free-threaded build`.

‎Modules/_threadmodule.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "pycore_modsupport.h" // _PyArg_NoKeywords()
99
#include "pycore_moduleobject.h" // _PyModule_GetState()
1010
#include "pycore_object_deferred.h" // _PyObject_SetDeferredRefcount()
11+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_SIZE_RELAXED()
1112
#include "pycore_pylifecycle.h"
1213
#include "pycore_pystate.h" // _PyThreadState_SetCurrent()
1314
#include "pycore_time.h" // _PyTime_FromSeconds()
@@ -1207,7 +1208,7 @@ _thread_RLock__acquire_restore_impl(rlockobject *self, PyObject *state)
12071208

12081209
_PyRecursiveMutex_Lock(&self->lock);
12091210
_Py_atomic_store_ullong_relaxed(&self->lock.thread, owner);
1210-
self->lock.level = (size_t)count - 1;
1211+
FT_ATOMIC_STORE_SIZE_RELAXED(self->lock.level, (size_t)count - 1);
12111212
Py_RETURN_NONE;
12121213
}
12131214

@@ -1230,7 +1231,8 @@ _thread_RLock__release_save_impl(rlockobject *self)
12301231

12311232
PyThread_ident_t owner = self->lock.thread;
12321233
Py_ssize_t count = self->lock.level + 1;
1233-
self->lock.level = 0; // ensure the unlock releases the lock
1234+
// ensure the unlock releases the lock
1235+
FT_ATOMIC_STORE_SIZE_RELAXED(self->lock.level, 0);
12341236
_PyRecursiveMutex_Unlock(&self->lock);
12351237
return Py_BuildValue("n" Py_PARSE_THREAD_IDENT_T, count, owner);
12361238
}
@@ -1292,7 +1294,7 @@ rlock_repr(PyObject *op)
12921294
int locked = rlock_locked_impl(self);
12931295
size_t count;
12941296
if (locked) {
1295-
count = self->lock.level + 1;
1297+
count = FT_ATOMIC_LOAD_SIZE_RELAXED(self->lock.level) + 1;
12961298
}
12971299
else {
12981300
count = 0;

‎Python/lock.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include "pycore_lock.h"
66
#include "pycore_parking_lot.h"
7+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_STORE_SIZE_RELAXED()
78
#include "pycore_semaphore.h"
89
#include "pycore_time.h" // _PyTime_Add()
910
#include "pycore_stats.h" // FT_STAT_MUTEX_SLEEP_INC()
@@ -418,7 +419,7 @@ _PyRecursiveMutex_Lock(_PyRecursiveMutex *m)
418419
{
419420
PyThread_ident_t thread = PyThread_get_thread_ident_ex();
420421
if (recursive_mutex_is_owned_by(m, thread)) {
421-
m->level++;
422+
FT_ATOMIC_STORE_SIZE_RELAXED(m->level, m->level + 1);
422423
return;
423424
}
424425
PyMutex_Lock(&m->mutex);
@@ -431,7 +432,7 @@ _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags
431432
{
432433
PyThread_ident_t thread = PyThread_get_thread_ident_ex();
433434
if (recursive_mutex_is_owned_by(m, thread)) {
434-
m->level++;
435+
FT_ATOMIC_STORE_SIZE_RELAXED(m->level, m->level + 1);
435436
return PY_LOCK_ACQUIRED;
436437
}
437438
PyLockStatus s = _PyMutex_LockTimed(&m->mutex, timeout, flags);
@@ -459,7 +460,7 @@ _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m)
459460
return -1;
460461
}
461462
if (m->level > 0) {
462-
m->level--;
463+
FT_ATOMIC_STORE_SIZE_RELAXED(m->level, m->level - 1);
463464
return 0;
464465
}
465466
assert(m->level == 0);

0 commit comments

Comments
 (0)