Skip to content

gh-114271: Make thread._rlock thread-safe in free-threaded builds#115102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
colesbury merged 1 commit into from
Feb 16, 2024
Merged

gh-114271: Make thread._rlock thread-safe in free-threaded builds #115102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -360,6 +360,8 @@ _Py_atomic_load_ssize_relaxed(const Py_ssize_t *obj);
static inline void *
_Py_atomic_load_ptr_relaxed(const void *obj);

static inline unsigned long long
_Py_atomic_load_ullong_relaxed(const unsigned long long *obj);

// --- _Py_atomic_store ------------------------------------------------------
// Atomically performs `*obj = value` (sequential consistency)
Expand DownExpand Up@@ -452,6 +454,10 @@ _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_ullong_relaxed(unsigned long long *obj,
unsigned long long value);


// --- _Py_atomic_load_ptr_acquire / _Py_atomic_store_ptr_release ------------

Expand Down
9 changes: 9 additions & 0 deletions Include/cpython/pyatomic_gcc.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -358,6 +358,10 @@ static inline void *
_Py_atomic_load_ptr_relaxed(const void *obj)
{return (void *)__atomic_load_n((const void **)obj, __ATOMIC_RELAXED)}

static inline unsigned long long
_Py_atomic_load_ullong_relaxed(const unsigned long long *obj)
{return __atomic_load_n(obj, __ATOMIC_RELAXED)}


// --- _Py_atomic_store ------------------------------------------------------

Expand DownExpand Up@@ -476,6 +480,11 @@ 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_ullong_relaxed(unsigned long long *obj,
unsigned long long value)
{__atomic_store_n(obj, value, __ATOMIC_RELAXED)}


// --- _Py_atomic_load_ptr_acquire / _Py_atomic_store_ptr_release ------------

Expand Down
14 changes: 14 additions & 0 deletions Include/cpython/pyatomic_msc.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -712,6 +712,12 @@ _Py_atomic_load_ptr_relaxed(const void *obj)
return *(void * volatile *)obj;
}

static inline unsigned long long
_Py_atomic_load_ullong_relaxed(const unsigned long long *obj)
{
return *(volatile unsigned long long *)obj;
}


// --- _Py_atomic_store ------------------------------------------------------

Expand DownExpand Up@@ -886,6 +892,14 @@ _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_ullong_relaxed(unsigned long long *obj,
unsigned long long value)
{
*(volatile unsigned long long *)obj = value;
}


// --- _Py_atomic_load_ptr_acquire / _Py_atomic_store_ptr_release ------------

static inline void *
Expand Down
17 changes: 17 additions & 0 deletions Include/cpython/pyatomic_std.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -619,6 +619,14 @@ _Py_atomic_load_ptr_relaxed(const void *obj)
memory_order_relaxed);
}

static inline unsigned long long
_Py_atomic_load_ullong_relaxed(const unsigned long long *obj)
{
_Py_USING_STD;
return atomic_load_explicit((const _Atomic(unsigned long long)*)obj,
memory_order_relaxed);
}


// --- _Py_atomic_store ------------------------------------------------------

Expand DownExpand Up@@ -835,6 +843,15 @@ _Py_atomic_store_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value)
memory_order_relaxed);
}

static inline void
_Py_atomic_store_ullong_relaxed(unsigned long long *obj,
unsigned long long value)
{
_Py_USING_STD;
atomic_store_explicit((_Atomic(unsigned long long)*)obj, value,
memory_order_relaxed);
}


// --- _Py_atomic_load_ptr_acquire / _Py_atomic_store_ptr_release ------------

Expand Down
32 changes: 22 additions & 10 deletions Modules/_threadmodule.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@
#include "pycore_sysmodule.h" // _PySys_GetAttr()
#include "pycore_weakref.h" // _PyWeakref_GET_REF()

#include <stdbool.h>
#include <stddef.h> // offsetof()
#ifdef HAVE_SIGNAL_H
# include <signal.h> // SIGINT
Expand DownExpand Up@@ -470,6 +471,14 @@ rlock_dealloc(rlockobject *self)
Py_DECREF(tp);
}

static bool
rlock_is_owned_by(rlockobject *self, PyThread_ident_t tid)
{
PyThread_ident_t owner_tid =
_Py_atomic_load_ullong_relaxed(&self->rlock_owner);
return owner_tid == tid && self->rlock_count > 0;
}

static PyObject *
rlock_acquire(rlockobject *self, PyObject *args, PyObject *kwds)
{
Expand All@@ -481,7 +490,7 @@ rlock_acquire(rlockobject *self, PyObject *args, PyObject *kwds)
return NULL;

tid = PyThread_get_thread_ident_ex();
if (self->rlock_count > 0 && tid == self->rlock_owner){
if (rlock_is_owned_by(self, tid)){
unsigned long count = self->rlock_count + 1;
if (count <= self->rlock_count){
PyErr_SetString(PyExc_OverflowError,
Expand All@@ -494,7 +503,7 @@ rlock_acquire(rlockobject *self, PyObject *args, PyObject *kwds)
r = acquire_timed(self->rlock_lock, timeout);
if (r == PY_LOCK_ACQUIRED){
assert(self->rlock_count == 0);
self->rlock_owner = tid;
_Py_atomic_store_ullong_relaxed(&self->rlock_owner, tid);
self->rlock_count = 1;
}
else if (r == PY_LOCK_INTR){
Expand DownExpand Up@@ -525,13 +534,13 @@ rlock_release(rlockobject *self, PyObject *Py_UNUSED(ignored))
{
PyThread_ident_t tid = PyThread_get_thread_ident_ex();

if (self->rlock_count == 0 || self->rlock_owner != tid){
if (!rlock_is_owned_by(self, tid)){
PyErr_SetString(PyExc_RuntimeError,
"cannot release un-acquired lock");
return NULL;
}
if (--self->rlock_count == 0){
self->rlock_owner = 0;
_Py_atomic_store_ullong_relaxed(&self->rlock_owner, 0);
PyThread_release_lock(self->rlock_lock);
}
Py_RETURN_NONE;
Expand DownExpand Up@@ -570,7 +579,7 @@ rlock_acquire_restore(rlockobject *self, PyObject *args)
return NULL;
}
assert(self->rlock_count == 0);
self->rlock_owner = owner;
_Py_atomic_store_ullong_relaxed(&self->rlock_owner, owner);
self->rlock_count = count;
Py_RETURN_NONE;
}
Expand All@@ -595,7 +604,7 @@ rlock_release_save(rlockobject *self, PyObject *Py_UNUSED(ignored))
owner = self->rlock_owner;
count = self->rlock_count;
self->rlock_count = 0;
self->rlock_owner = 0;
_Py_atomic_store_ullong_relaxed(&self->rlock_owner, 0);
PyThread_release_lock(self->rlock_lock);
return Py_BuildValue("k" Py_PARSE_THREAD_IDENT_T, count, owner);
}
Expand All@@ -609,8 +618,9 @@ static PyObject *
rlock_recursion_count(rlockobject *self, PyObject *Py_UNUSED(ignored))
{
PyThread_ident_t tid = PyThread_get_thread_ident_ex();
return PyLong_FromUnsignedLong(
self->rlock_owner == tid ? self->rlock_count : 0UL);
PyThread_ident_t owner =
_Py_atomic_load_ullong_relaxed(&self->rlock_owner);
return PyLong_FromUnsignedLong(owner == tid ? self->rlock_count : 0UL);
}

PyDoc_STRVAR(rlock_recursion_count_doc,
Expand All@@ -623,7 +633,7 @@ rlock_is_owned(rlockobject *self, PyObject *Py_UNUSED(ignored))
{
PyThread_ident_t tid = PyThread_get_thread_ident_ex();

if (self->rlock_count > 0 && self->rlock_owner == tid){
if (rlock_is_owned_by(self, tid)){
Py_RETURN_TRUE;
}
Py_RETURN_FALSE;
Expand DownExpand Up@@ -657,10 +667,12 @@ rlock_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
static PyObject *
rlock_repr(rlockobject *self)
{
PyThread_ident_t owner =
_Py_atomic_load_ullong_relaxed(&self->rlock_owner);
return PyUnicode_FromFormat(
"<%s %s object owner=%" PY_FORMAT_THREAD_IDENT_T " count=%lu at %p>",
self->rlock_count ? "locked" : "unlocked",
Py_TYPE(self)->tp_name, self->rlock_owner,
Py_TYPE(self)->tp_name, owner,
self->rlock_count, self);
}

Expand Down