Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Include/cpython/pyatomic.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions Include/cpython/pyatomic_gcc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions Include/cpython/pyatomic_msc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 16 additions & 0 deletions Include/cpython/pyatomic_std.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions Include/internal/pycore_pyatomic_ft_wrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down Expand Up @@ -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) \
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 18 additions & 0 deletions Lib/test/test_free_threading/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix data race on the recursion count of :class:`threading.RLock` in the
:term:`free-threaded build`.
8 changes: 5 additions & 3 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions Python/lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Loading