Skip to content

Commit 67f6e08

Browse files
gh-125139: use _PyRecursiveMutex in _thread.RLock (#125144)
1 parent 5217328 commit 67f6e08

File tree

3 files changed

+63
-122
lines changed

3 files changed

+63
-122
lines changed

Include/internal/pycore_lock.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,9 @@ typedef struct {
160160

161161
PyAPI_FUNC(int) _PyRecursiveMutex_IsLockedByCurrentThread(_PyRecursiveMutex *m);
162162
PyAPI_FUNC(void) _PyRecursiveMutex_Lock(_PyRecursiveMutex *m);
163+
extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags);
163164
PyAPI_FUNC(void) _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m);
164-
165+
extern int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m);
165166

166167
// A readers-writer (RW) lock. The lock supports multiple concurrent readers or
167168
// a single writer. The lock is write-preferring: if a writer is waiting while

Modules/_threadmodule.c

Lines changed: 33 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -726,11 +726,6 @@ lock_dealloc(PyObject *op)
726726
Py_DECREF(tp);
727727
}
728728

729-
static inline PyLockStatus
730-
acquire_timed(PyThread_type_lock lock, PyTime_t timeout)
731-
{
732-
return PyThread_acquire_lock_timed_with_retries(lock, timeout);
733-
}
734729

735730
static int
736731
lock_acquire_parse_args(PyObject *args, PyObject *kwds,
@@ -973,10 +968,7 @@ static PyType_Spec lock_type_spec = {
973968

974969
typedef struct {
975970
PyObject_HEAD
976-
PyThread_type_lock rlock_lock;
977-
PyThread_ident_t rlock_owner;
978-
unsigned long rlock_count;
979-
PyObject *in_weakreflist;
971+
_PyRecursiveMutex lock;
980972
} rlockobject;
981973

982974
static int
@@ -992,59 +984,26 @@ rlock_dealloc(PyObject *op)
992984
{
993985
rlockobject *self = (rlockobject*)op;
994986
PyObject_GC_UnTrack(self);
995-
if (self->in_weakreflist != NULL)
996-
PyObject_ClearWeakRefs((PyObject *) self);
997-
/* self->rlock_lock can be NULL if PyThread_allocate_lock() failed
998-
in rlock_new() */
999-
if (self->rlock_lock != NULL) {
1000-
/* Unlock the lock so it's safe to free it */
1001-
if (self->rlock_count > 0)
1002-
PyThread_release_lock(self->rlock_lock);
1003-
1004-
PyThread_free_lock(self->rlock_lock);
1005-
}
987+
PyObject_ClearWeakRefs((PyObject *) self);
1006988
PyTypeObject *tp = Py_TYPE(self);
1007989
tp->tp_free(self);
1008990
Py_DECREF(tp);
1009991
}
1010992

1011-
static bool
1012-
rlock_is_owned_by(rlockobject *self, PyThread_ident_t tid)
1013-
{
1014-
PyThread_ident_t owner_tid =
1015-
_Py_atomic_load_ullong_relaxed(&self->rlock_owner);
1016-
return owner_tid == tid && self->rlock_count > 0;
1017-
}
1018993

1019994
static PyObject *
1020995
rlock_acquire(PyObject *op, PyObject *args, PyObject *kwds)
1021996
{
1022997
rlockobject *self = (rlockobject*)op;
1023998
PyTime_t timeout;
1024-
PyThread_ident_t tid;
1025-
PyLockStatus r = PY_LOCK_ACQUIRED;
1026999

1027-
if (lock_acquire_parse_args(args, kwds, &timeout) < 0)
1000+
if (lock_acquire_parse_args(args, kwds, &timeout) < 0) {
10281001
return NULL;
1029-
1030-
tid = PyThread_get_thread_ident_ex();
1031-
if (rlock_is_owned_by(self, tid)) {
1032-
unsigned long count = self->rlock_count + 1;
1033-
if (count <= self->rlock_count) {
1034-
PyErr_SetString(PyExc_OverflowError,
1035-
"Internal lock count overflowed");
1036-
return NULL;
1037-
}
1038-
self->rlock_count = count;
1039-
Py_RETURN_TRUE;
1040-
}
1041-
r = acquire_timed(self->rlock_lock, timeout);
1042-
if (r == PY_LOCK_ACQUIRED) {
1043-
assert(self->rlock_count == 0);
1044-
_Py_atomic_store_ullong_relaxed(&self->rlock_owner, tid);
1045-
self->rlock_count = 1;
10461002
}
1047-
else if (r == PY_LOCK_INTR) {
1003+
1004+
PyLockStatus r = _PyRecursiveMutex_LockTimed(&self->lock, timeout,
1005+
_PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH);
1006+
if (r == PY_LOCK_INTR) {
10481007
return NULL;
10491008
}
10501009

@@ -1078,17 +1037,12 @@ static PyObject *
10781037
rlock_release(PyObject *op, PyObject *Py_UNUSED(ignored))
10791038
{
10801039
rlockobject *self = (rlockobject*)op;
1081-
PyThread_ident_t tid = PyThread_get_thread_ident_ex();
10821040

1083-
if (!rlock_is_owned_by(self, tid)) {
1041+
if (_PyRecursiveMutex_TryUnlock(&self->lock) < 0) {
10841042
PyErr_SetString(PyExc_RuntimeError,
10851043
"cannot release un-acquired lock");
10861044
return NULL;
10871045
}
1088-
if (--self->rlock_count == 0) {
1089-
_Py_atomic_store_ullong_relaxed(&self->rlock_owner, 0);
1090-
PyThread_release_lock(self->rlock_lock);
1091-
}
10921046
Py_RETURN_NONE;
10931047
}
10941048

@@ -1116,25 +1070,15 @@ rlock_acquire_restore(PyObject *op, PyObject *args)
11161070
{
11171071
rlockobject *self = (rlockobject*)op;
11181072
PyThread_ident_t owner;
1119-
unsigned long count;
1120-
int r = 1;
1073+
Py_ssize_t count;
11211074

1122-
if (!PyArg_ParseTuple(args, "(k" Py_PARSE_THREAD_IDENT_T "):_acquire_restore",
1075+
if (!PyArg_ParseTuple(args, "(n" Py_PARSE_THREAD_IDENT_T "):_acquire_restore",
11231076
&count, &owner))
11241077
return NULL;
11251078

1126-
if (!PyThread_acquire_lock(self->rlock_lock, 0)) {
1127-
Py_BEGIN_ALLOW_THREADS
1128-
r = PyThread_acquire_lock(self->rlock_lock, 1);
1129-
Py_END_ALLOW_THREADS
1130-
}
1131-
if (!r) {
1132-
PyErr_SetString(ThreadError, "couldn't acquire lock");
1133-
return NULL;
1134-
}
1135-
assert(self->rlock_count == 0);
1136-
_Py_atomic_store_ullong_relaxed(&self->rlock_owner, owner);
1137-
self->rlock_count = count;
1079+
_PyRecursiveMutex_Lock(&self->lock);
1080+
_Py_atomic_store_ullong_relaxed(&self->lock.thread, owner);
1081+
self->lock.level = (size_t)count - 1;
11381082
Py_RETURN_NONE;
11391083
}
11401084

@@ -1148,21 +1092,18 @@ static PyObject *
11481092
rlock_release_save(PyObject *op, PyObject *Py_UNUSED(ignored))
11491093
{
11501094
rlockobject *self = (rlockobject*)op;
1151-
PyThread_ident_t owner;
1152-
unsigned long count;
11531095

1154-
if (self->rlock_count == 0) {
1096+
if (!_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)) {
11551097
PyErr_SetString(PyExc_RuntimeError,
11561098
"cannot release un-acquired lock");
11571099
return NULL;
11581100
}
11591101

1160-
owner = self->rlock_owner;
1161-
count = self->rlock_count;
1162-
self->rlock_count = 0;
1163-
_Py_atomic_store_ullong_relaxed(&self->rlock_owner, 0);
1164-
PyThread_release_lock(self->rlock_lock);
1165-
return Py_BuildValue("k" Py_PARSE_THREAD_IDENT_T, count, owner);
1102+
PyThread_ident_t owner = self->lock.thread;
1103+
Py_ssize_t count = self->lock.level + 1;
1104+
self->lock.level = 0; // ensure the unlock releases the lock
1105+
_PyRecursiveMutex_Unlock(&self->lock);
1106+
return Py_BuildValue("n" Py_PARSE_THREAD_IDENT_T, count, owner);
11661107
}
11671108

11681109
PyDoc_STRVAR(rlock_release_save_doc,
@@ -1175,10 +1116,10 @@ static PyObject *
11751116
rlock_recursion_count(PyObject *op, PyObject *Py_UNUSED(ignored))
11761117
{
11771118
rlockobject *self = (rlockobject*)op;
1178-
PyThread_ident_t tid = PyThread_get_thread_ident_ex();
1179-
PyThread_ident_t owner =
1180-
_Py_atomic_load_ullong_relaxed(&self->rlock_owner);
1181-
return PyLong_FromUnsignedLong(owner == tid ? self->rlock_count : 0UL);
1119+
if (_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)) {
1120+
return PyLong_FromSize_t(self->lock.level + 1);
1121+
}
1122+
return PyLong_FromLong(0);
11821123
}
11831124

11841125
PyDoc_STRVAR(rlock_recursion_count_doc,
@@ -1191,12 +1132,8 @@ static PyObject *
11911132
rlock_is_owned(PyObject *op, PyObject *Py_UNUSED(ignored))
11921133
{
11931134
rlockobject *self = (rlockobject*)op;
1194-
PyThread_ident_t tid = PyThread_get_thread_ident_ex();
1195-
1196-
if (rlock_is_owned_by(self, tid)) {
1197-
Py_RETURN_TRUE;
1198-
}
1199-
Py_RETURN_FALSE;
1135+
long owned = _PyRecursiveMutex_IsLockedByCurrentThread(&self->lock);
1136+
return PyBool_FromLong(owned);
12001137
}
12011138

12021139
PyDoc_STRVAR(rlock_is_owned_doc,
@@ -1212,45 +1149,29 @@ rlock_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
12121149
if (self == NULL) {
12131150
return NULL;
12141151
}
1215-
self->in_weakreflist = NULL;
1216-
self->rlock_owner = 0;
1217-
self->rlock_count = 0;
1218-
1219-
self->rlock_lock = PyThread_allocate_lock();
1220-
if (self->rlock_lock == NULL) {
1221-
Py_DECREF(self);
1222-
PyErr_SetString(ThreadError, "can't allocate lock");
1223-
return NULL;
1224-
}
1152+
self->lock = (_PyRecursiveMutex){0};
12251153
return (PyObject *) self;
12261154
}
12271155

12281156
static PyObject *
12291157
rlock_repr(PyObject *op)
12301158
{
12311159
rlockobject *self = (rlockobject*)op;
1232-
PyThread_ident_t owner =
1233-
_Py_atomic_load_ullong_relaxed(&self->rlock_owner);
1160+
PyThread_ident_t owner = self->lock.thread;
1161+
size_t count = self->lock.level + 1;
12341162
return PyUnicode_FromFormat(
1235-
"<%s %s object owner=%" PY_FORMAT_THREAD_IDENT_T " count=%lu at %p>",
1236-
self->rlock_count ? "locked" : "unlocked",
1163+
"<%s %s object owner=%" PY_FORMAT_THREAD_IDENT_T " count=%zu at %p>",
1164+
owner ? "locked" : "unlocked",
12371165
Py_TYPE(self)->tp_name, owner,
1238-
self->rlock_count, self);
1166+
count, self);
12391167
}
12401168

12411169

12421170
#ifdef HAVE_FORK
12431171
static PyObject *
12441172
rlock__at_fork_reinit(rlockobject *self, PyObject *Py_UNUSED(args))
12451173
{
1246-
if (_PyThread_at_fork_reinit(&self->rlock_lock) < 0) {
1247-
PyErr_SetString(ThreadError, "failed to reinitialize lock at fork");
1248-
return NULL;
1249-
}
1250-
1251-
self->rlock_owner = 0;
1252-
self->rlock_count = 0;
1253-
1174+
self->lock = (_PyRecursiveMutex){0};
12541175
Py_RETURN_NONE;
12551176
}
12561177
#endif /* HAVE_FORK */
@@ -1281,18 +1202,12 @@ static PyMethodDef rlock_methods[] = {
12811202
};
12821203

12831204

1284-
static PyMemberDef rlock_type_members[] = {
1285-
{"__weaklistoffset__", Py_T_PYSSIZET, offsetof(rlockobject, in_weakreflist), Py_READONLY},
1286-
{NULL},
1287-
};
1288-
12891205
static PyType_Slot rlock_type_slots[] = {
12901206
{Py_tp_dealloc, rlock_dealloc},
12911207
{Py_tp_repr, rlock_repr},
12921208
{Py_tp_methods, rlock_methods},
12931209
{Py_tp_alloc, PyType_GenericAlloc},
12941210
{Py_tp_new, rlock_new},
1295-
{Py_tp_members, rlock_type_members},
12961211
{Py_tp_traverse, rlock_traverse},
12971212
{0, 0},
12981213
};
@@ -1301,7 +1216,7 @@ static PyType_Spec rlock_type_spec = {
13011216
.name = "_thread.RLock",
13021217
.basicsize = sizeof(rlockobject),
13031218
.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE |
1304-
Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_IMMUTABLETYPE),
1219+
Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_IMMUTABLETYPE | Py_TPFLAGS_MANAGED_WEAKREF),
13051220
.slots = rlock_type_slots,
13061221
};
13071222

Python/lock.c

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -377,21 +377,46 @@ _PyRecursiveMutex_Lock(_PyRecursiveMutex *m)
377377
assert(m->level == 0);
378378
}
379379

380+
PyLockStatus
381+
_PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags)
382+
{
383+
PyThread_ident_t thread = PyThread_get_thread_ident_ex();
384+
if (recursive_mutex_is_owned_by(m, thread)) {
385+
m->level++;
386+
return PY_LOCK_ACQUIRED;
387+
}
388+
PyLockStatus s = _PyMutex_LockTimed(&m->mutex, timeout, flags);
389+
if (s == PY_LOCK_ACQUIRED) {
390+
_Py_atomic_store_ullong_relaxed(&m->thread, thread);
391+
assert(m->level == 0);
392+
}
393+
return s;
394+
}
395+
380396
void
381397
_PyRecursiveMutex_Unlock(_PyRecursiveMutex *m)
398+
{
399+
if (_PyRecursiveMutex_TryUnlock(m) < 0) {
400+
Py_FatalError("unlocking a recursive mutex that is not "
401+
"owned by the current thread");
402+
}
403+
}
404+
405+
int
406+
_PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m)
382407
{
383408
PyThread_ident_t thread = PyThread_get_thread_ident_ex();
384409
if (!recursive_mutex_is_owned_by(m, thread)) {
385-
Py_FatalError("unlocking a recursive mutex that is not owned by the"
386-
" current thread");
410+
return -1;
387411
}
388412
if (m->level > 0) {
389413
m->level--;
390-
return;
414+
return 0;
391415
}
392416
assert(m->level == 0);
393417
_Py_atomic_store_ullong_relaxed(&m->thread, 0);
394418
PyMutex_Unlock(&m->mutex);
419+
return 0;
395420
}
396421

397422
#define _Py_WRITE_LOCKED 1

0 commit comments

Comments
 (0)