From fe7ef86ec4c0ac2d869e541cd914c49d17fc4c4a Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 23:16:31 +0530 Subject: [PATCH 01/12] rlock --- Include/internal/pycore_lock.h | 3 +- Modules/_threadmodule.c | 132 ++++++--------------------------- Python/lock.c | 30 +++++++- 3 files changed, 52 insertions(+), 113 deletions(-) diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index e6da083b807ce5..55b44e13b126e6 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -160,8 +160,9 @@ typedef struct { PyAPI_FUNC(int) _PyRecursiveMutex_IsLockedByCurrentThread(_PyRecursiveMutex *m); PyAPI_FUNC(void) _PyRecursiveMutex_Lock(_PyRecursiveMutex *m); +extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags); PyAPI_FUNC(void) _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m); - +extern int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m); // A readers-writer (RW) lock. The lock supports multiple concurrent readers or // a single writer. The lock is write-preferring: if a writer is waiting while diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 9617f9cafe76ff..8af6ccc692e5e2 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -973,10 +973,7 @@ static PyType_Spec lock_type_spec = { typedef struct { PyObject_HEAD - PyThread_type_lock rlock_lock; - PyThread_ident_t rlock_owner; - unsigned long rlock_count; - PyObject *in_weakreflist; + _PyRecursiveMutex lock; } rlockobject; static int @@ -992,62 +989,27 @@ rlock_dealloc(PyObject *op) { rlockobject *self = (rlockobject*)op; PyObject_GC_UnTrack(self); - if (self->in_weakreflist != NULL) - PyObject_ClearWeakRefs((PyObject *) self); - /* self->rlock_lock can be NULL if PyThread_allocate_lock() failed - in rlock_new() */ - if (self->rlock_lock != NULL) { - /* Unlock the lock so it's safe to free it */ - if (self->rlock_count > 0) - PyThread_release_lock(self->rlock_lock); - - PyThread_free_lock(self->rlock_lock); - } PyTypeObject *tp = Py_TYPE(self); tp->tp_free(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(PyObject *op, PyObject *args, PyObject *kwds) { rlockobject *self = (rlockobject*)op; PyTime_t timeout; - PyThread_ident_t tid; - PyLockStatus r = PY_LOCK_ACQUIRED; if (lock_acquire_parse_args(args, kwds, &timeout) < 0) return NULL; - - tid = PyThread_get_thread_ident_ex(); - if (rlock_is_owned_by(self, tid)) { - unsigned long count = self->rlock_count + 1; - if (count <= self->rlock_count) { - PyErr_SetString(PyExc_OverflowError, - "Internal lock count overflowed"); - return NULL; - } - self->rlock_count = count; - Py_RETURN_TRUE; } - r = acquire_timed(self->rlock_lock, timeout); - if (r == PY_LOCK_ACQUIRED) { - assert(self->rlock_count == 0); - _Py_atomic_store_ullong_relaxed(&self->rlock_owner, tid); - self->rlock_count = 1; - } - else if (r == PY_LOCK_INTR) { + + PyLockStatus r = _PyRecursiveMutex_LockTimed(&self->lock, timeout, + _PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH); + if (r == PY_LOCK_INTR) { return NULL; } - return PyBool_FromLong(r == PY_LOCK_ACQUIRED); } @@ -1078,17 +1040,12 @@ static PyObject * rlock_release(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - PyThread_ident_t tid = PyThread_get_thread_ident_ex(); - if (!rlock_is_owned_by(self, tid)) { + if (!_PyRecursiveMutex_Unlock(&self->lock)) { PyErr_SetString(PyExc_RuntimeError, "cannot release un-acquired lock"); return NULL; } - if (--self->rlock_count == 0) { - _Py_atomic_store_ullong_relaxed(&self->rlock_owner, 0); - PyThread_release_lock(self->rlock_lock); - } Py_RETURN_NONE; } @@ -1123,18 +1080,7 @@ rlock_acquire_restore(PyObject *op, PyObject *args) &count, &owner)) return NULL; - if (!PyThread_acquire_lock(self->rlock_lock, 0)) { - Py_BEGIN_ALLOW_THREADS - r = PyThread_acquire_lock(self->rlock_lock, 1); - Py_END_ALLOW_THREADS - } - if (!r) { - PyErr_SetString(ThreadError, "couldn't acquire lock"); - return NULL; - } - assert(self->rlock_count == 0); - _Py_atomic_store_ullong_relaxed(&self->rlock_owner, owner); - self->rlock_count = count; + _PyRecursiveMutex_Lock(&self->lock); Py_RETURN_NONE; } @@ -1148,20 +1094,15 @@ static PyObject * rlock_release_save(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - PyThread_ident_t owner; - unsigned long count; - if (self->rlock_count == 0) { + PyThread_ident_t owner = _Py_atomic_load_ullong_relaxed(&self->lock.thread); + size_t count = _Py_atomic_load_ullong_relaxed(&self->lock.level); + PyThread_release_lock(self->rlock_lock); + if (_PyRecursiveMutex_TryUnlock(&self->lock) < 0) { PyErr_SetString(PyExc_RuntimeError, "cannot release un-acquired lock"); return NULL; } - - owner = self->rlock_owner; - count = self->rlock_count; - self->rlock_count = 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); } @@ -1175,10 +1116,10 @@ static PyObject * rlock_recursion_count(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - PyThread_ident_t tid = PyThread_get_thread_ident_ex(); - PyThread_ident_t owner = - _Py_atomic_load_ullong_relaxed(&self->rlock_owner); - return PyLong_FromUnsignedLong(owner == tid ? self->rlock_count : 0UL); + if (_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)) { + return PyLong_FromUnsignedLong(self->lock.level); + } + return PyLong_FromLong(0); } PyDoc_STRVAR(rlock_recursion_count_doc, @@ -1191,12 +1132,7 @@ static PyObject * rlock_is_owned(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - PyThread_ident_t tid = PyThread_get_thread_ident_ex(); - - if (rlock_is_owned_by(self, tid)) { - Py_RETURN_TRUE; - } - Py_RETURN_FALSE; + return PyBool_FromLong(_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)); } PyDoc_STRVAR(rlock_is_owned_doc, @@ -1212,16 +1148,7 @@ rlock_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (self == NULL) { return NULL; } - self->in_weakreflist = NULL; - self->rlock_owner = 0; - self->rlock_count = 0; - - self->rlock_lock = PyThread_allocate_lock(); - if (self->rlock_lock == NULL) { - Py_DECREF(self); - PyErr_SetString(ThreadError, "can't allocate lock"); - return NULL; - } + self->lock = (_PyRecursiveMutex){0}; return (PyObject *) self; } @@ -1229,13 +1156,13 @@ static PyObject * rlock_repr(PyObject *op) { rlockobject *self = (rlockobject*)op; - PyThread_ident_t owner = - _Py_atomic_load_ullong_relaxed(&self->rlock_owner); + PyThread_ident_t owner = _Py_atomic_load_ullong_relaxed(&self->lock.thread); + size_t count = _Py_atomic_load_ssize_relaxed(&self->lock.count); return PyUnicode_FromFormat( - "<%s %s object owner=%" PY_FORMAT_THREAD_IDENT_T " count=%lu at %p>", - self->rlock_count ? "locked" : "unlocked", + "<%s %s object owner=%" PY_FORMAT_THREAD_IDENT_T " count=%zu at %p>", + count ? "locked" : "unlocked", Py_TYPE(self)->tp_name, owner, - self->rlock_count, self); + count, self); } @@ -1243,14 +1170,7 @@ rlock_repr(PyObject *op) static PyObject * rlock__at_fork_reinit(rlockobject *self, PyObject *Py_UNUSED(args)) { - if (_PyThread_at_fork_reinit(&self->rlock_lock) < 0) { - PyErr_SetString(ThreadError, "failed to reinitialize lock at fork"); - return NULL; - } - - self->rlock_owner = 0; - self->rlock_count = 0; - + self->lock = (_PyRecursiveMutex){0}; Py_RETURN_NONE; } #endif /* HAVE_FORK */ @@ -1281,18 +1201,12 @@ static PyMethodDef rlock_methods[] = { }; -static PyMemberDef rlock_type_members[] = { - {"__weaklistoffset__", Py_T_PYSSIZET, offsetof(rlockobject, in_weakreflist), Py_READONLY}, - {NULL}, -}; - static PyType_Slot rlock_type_slots[] = { {Py_tp_dealloc, rlock_dealloc}, {Py_tp_repr, rlock_repr}, {Py_tp_methods, rlock_methods}, {Py_tp_alloc, PyType_GenericAlloc}, {Py_tp_new, rlock_new}, - {Py_tp_members, rlock_type_members}, {Py_tp_traverse, rlock_traverse}, {0, 0}, }; diff --git a/Python/lock.c b/Python/lock.c index 57675fe1873fa2..8f8964a09e59cf 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -377,21 +377,45 @@ _PyRecursiveMutex_Lock(_PyRecursiveMutex *m) assert(m->level == 0); } +PyLockStatus +_PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags) +{ + PyThread_ident_t thread = PyThread_get_thread_ident_ex(); + if (recursive_mutex_is_owned_by(m, thread)) { + m->level++; + return PY_LOCK_ACQUIRED; + } + PyLockStatus s = _PyMutex_LockTimed(&m->mutex, timeout, flags); + if (s == PY_LOCK_ACQUIRED) { + _Py_atomic_store_ullong_relaxed(&m->thread, thread); + assert(m->level == 0); + } + return s; +} + void _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m) { - PyThread_ident_t thread = PyThread_get_thread_ident_ex(); - if (!recursive_mutex_is_owned_by(m, thread)) { + if (_PyRecursiveMutex_TryUnlock(m) < 0) { Py_FatalError("unlocking a recursive mutex that is not owned by the" " current thread"); } +} + +int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m) +{ + PyThread_ident_t thread = PyThread_get_thread_ident_ex(); + if (!recursive_mutex_is_owned_by(m, thread)) { + return -1; + } if (m->level > 0) { m->level--; - return; + return 0; } assert(m->level == 0); _Py_atomic_store_ullong_relaxed(&m->thread, 0); PyMutex_Unlock(&m->mutex); + return 0; } #define _Py_WRITE_LOCKED 1 From 86b2d7427486c96bce4612bb5f7cacec06e70948 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 18:16:31 +0000 Subject: [PATCH 02/12] more stuff --- Modules/_threadmodule.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 8af6ccc692e5e2..c2b4b19e6641c0 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -726,11 +726,6 @@ lock_dealloc(PyObject *op) Py_DECREF(tp); } -static inline PyLockStatus -acquire_timed(PyThread_type_lock lock, PyTime_t timeout) -{ - return PyThread_acquire_lock_timed_with_retries(lock, timeout); -} static int lock_acquire_parse_args(PyObject *args, PyObject *kwds, @@ -989,6 +984,7 @@ rlock_dealloc(PyObject *op) { rlockobject *self = (rlockobject*)op; PyObject_GC_UnTrack(self); + PyObject_ClearWeakRefs((PyObject *) self); PyTypeObject *tp = Py_TYPE(self); tp->tp_free(self); Py_DECREF(tp); @@ -1001,7 +997,7 @@ rlock_acquire(PyObject *op, PyObject *args, PyObject *kwds) rlockobject *self = (rlockobject*)op; PyTime_t timeout; - if (lock_acquire_parse_args(args, kwds, &timeout) < 0) + if (lock_acquire_parse_args(args, kwds, &timeout) < 0) { return NULL; } @@ -1041,7 +1037,7 @@ rlock_release(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - if (!_PyRecursiveMutex_Unlock(&self->lock)) { + if (_PyRecursiveMutex_TryUnlock(&self->lock) < 0) { PyErr_SetString(PyExc_RuntimeError, "cannot release un-acquired lock"); return NULL; @@ -1074,7 +1070,6 @@ rlock_acquire_restore(PyObject *op, PyObject *args) rlockobject *self = (rlockobject*)op; PyThread_ident_t owner; unsigned long count; - int r = 1; if (!PyArg_ParseTuple(args, "(k" Py_PARSE_THREAD_IDENT_T "):_acquire_restore", &count, &owner)) @@ -1096,8 +1091,7 @@ rlock_release_save(PyObject *op, PyObject *Py_UNUSED(ignored)) rlockobject *self = (rlockobject*)op; PyThread_ident_t owner = _Py_atomic_load_ullong_relaxed(&self->lock.thread); - size_t count = _Py_atomic_load_ullong_relaxed(&self->lock.level); - PyThread_release_lock(self->rlock_lock); + size_t count = _Py_atomic_load_ulong_relaxed(&self->lock.level); if (_PyRecursiveMutex_TryUnlock(&self->lock) < 0) { PyErr_SetString(PyExc_RuntimeError, "cannot release un-acquired lock"); @@ -1117,7 +1111,7 @@ rlock_recursion_count(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; if (_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)) { - return PyLong_FromUnsignedLong(self->lock.level); + return PyLong_FromUnsignedLong(self->lock.level + 1); } return PyLong_FromLong(0); } @@ -1157,10 +1151,10 @@ rlock_repr(PyObject *op) { rlockobject *self = (rlockobject*)op; PyThread_ident_t owner = _Py_atomic_load_ullong_relaxed(&self->lock.thread); - size_t count = _Py_atomic_load_ssize_relaxed(&self->lock.count); + size_t count = _Py_atomic_load_ulong_relaxed(&self->lock.level); return PyUnicode_FromFormat( "<%s %s object owner=%" PY_FORMAT_THREAD_IDENT_T " count=%zu at %p>", - count ? "locked" : "unlocked", + owner ? "locked" : "unlocked", Py_TYPE(self)->tp_name, owner, count, self); } @@ -1215,7 +1209,7 @@ static PyType_Spec rlock_type_spec = { .name = "_thread.RLock", .basicsize = sizeof(rlockobject), .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | - Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_IMMUTABLETYPE), + Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_IMMUTABLETYPE | Py_TPFLAGS_MANAGED_WEAKREF), .slots = rlock_type_slots, }; From 8f01f3bc78bb79ad7bc9072f9cc27459956fba41 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 18:22:20 +0000 Subject: [PATCH 03/12] fix --- Python/lock.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Python/lock.c b/Python/lock.c index 8f8964a09e59cf..e91c4d9fd658ce 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -397,8 +397,7 @@ void _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m) { if (_PyRecursiveMutex_TryUnlock(m) < 0) { - Py_FatalError("unlocking a recursive mutex that is not owned by the" - " current thread"); + Py_FatalError("cannot unlock un-acquired lock"); } } @@ -413,8 +412,10 @@ int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m) return 0; } assert(m->level == 0); + if (_PyMutex_TryUnlock(&m->mutex) < 0){ + return -1; + }; _Py_atomic_store_ullong_relaxed(&m->thread, 0); - PyMutex_Unlock(&m->mutex); return 0; } From a569964f8ac73855017b5ef3450bfc97c0d7c2d4 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 18:38:55 +0000 Subject: [PATCH 04/12] fix rlock_recursion_count --- Modules/_threadmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index c2b4b19e6641c0..420f476d0f687b 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1111,7 +1111,7 @@ rlock_recursion_count(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; if (_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)) { - return PyLong_FromUnsignedLong(self->lock.level + 1); + return PyLong_FromSize_t(self->lock.level + 1); } return PyLong_FromLong(0); } From 34e9ba093525c22cf07e6140bc91944f118dbfe0 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 19:36:00 +0000 Subject: [PATCH 05/12] code review 1 --- Modules/_threadmodule.c | 14 +++++++++----- Python/lock.c | 12 +++++++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 420f476d0f687b..4a2e31f1a2a2fa 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1076,6 +1076,8 @@ rlock_acquire_restore(PyObject *op, PyObject *args) return NULL; _PyRecursiveMutex_Lock(&self->lock); + self->lock.thread = owner; + self->lock.level = count - 1; Py_RETURN_NONE; } @@ -1090,9 +1092,11 @@ rlock_release_save(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - PyThread_ident_t owner = _Py_atomic_load_ullong_relaxed(&self->lock.thread); - size_t count = _Py_atomic_load_ulong_relaxed(&self->lock.level); - if (_PyRecursiveMutex_TryUnlock(&self->lock) < 0) { + PyThread_ident_t owner = self->lock.thread; + size_t count = self->lock.level + 1; + + if (_PyRecursiveMutex_TryUnlock(&self->lock) < 0) + { PyErr_SetString(PyExc_RuntimeError, "cannot release un-acquired lock"); return NULL; @@ -1150,8 +1154,8 @@ static PyObject * rlock_repr(PyObject *op) { rlockobject *self = (rlockobject*)op; - PyThread_ident_t owner = _Py_atomic_load_ullong_relaxed(&self->lock.thread); - size_t count = _Py_atomic_load_ulong_relaxed(&self->lock.level); + PyThread_ident_t owner = self->lock.thread; + size_t count = self->lock.level + 1; return PyUnicode_FromFormat( "<%s %s object owner=%" PY_FORMAT_THREAD_IDENT_T " count=%zu at %p>", owner ? "locked" : "unlocked", diff --git a/Python/lock.c b/Python/lock.c index e91c4d9fd658ce..8f1f14d8127642 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -397,11 +397,13 @@ void _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m) { if (_PyRecursiveMutex_TryUnlock(m) < 0) { - Py_FatalError("cannot unlock un-acquired lock"); + Py_FatalError("unlocking a recursive mutex that is not " + "owned by the current thread"); } } -int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m) +int +_PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m) { PyThread_ident_t thread = PyThread_get_thread_ident_ex(); if (!recursive_mutex_is_owned_by(m, thread)) { @@ -412,10 +414,10 @@ int _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m) return 0; } assert(m->level == 0); - if (_PyMutex_TryUnlock(&m->mutex) < 0){ - return -1; - }; _Py_atomic_store_ullong_relaxed(&m->thread, 0); + if (_PyMutex_TryUnlock(&m->mutex) < 0) { + return -1; + } return 0; } From feeed9afa4879b58cffd26e2bc1b17c8e50d9d5d Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 20:12:12 +0000 Subject: [PATCH 06/12] fix rlock_release_save to reset level and thread --- Modules/_threadmodule.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 4a2e31f1a2a2fa..3077309358efb0 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1101,6 +1101,8 @@ rlock_release_save(PyObject *op, PyObject *Py_UNUSED(ignored)) "cannot release un-acquired lock"); return NULL; } + self->lock.thread = 0; + self->lock.level = 0; return Py_BuildValue("k" Py_PARSE_THREAD_IDENT_T, count, owner); } @@ -1130,7 +1132,8 @@ static PyObject * rlock_is_owned(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - return PyBool_FromLong(_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)); + long owned = _PyRecursiveMutex_IsLockedByCurrentThread(&self->lock); + return PyBool_FromLong(owned); } PyDoc_STRVAR(rlock_is_owned_doc, From edfe8c21a82c22d935888f077776b224ac199838 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 9 Oct 2024 02:02:39 +0530 Subject: [PATCH 07/12] fix whitespace --- Modules/_threadmodule.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 3077309358efb0..4cc36bf601a60a 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1006,6 +1006,7 @@ rlock_acquire(PyObject *op, PyObject *args, PyObject *kwds) if (r == PY_LOCK_INTR) { return NULL; } + return PyBool_FromLong(r == PY_LOCK_ACQUIRED); } From 1b5a708c2a4518327df0d7c7253892b421726fdf Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 20:43:53 +0000 Subject: [PATCH 08/12] add fix as suggested by Sam --- Modules/_threadmodule.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 4cc36bf601a60a..de26b811b60cc5 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1077,7 +1077,7 @@ rlock_acquire_restore(PyObject *op, PyObject *args) return NULL; _PyRecursiveMutex_Lock(&self->lock); - self->lock.thread = owner; + _Py_atomic_store_ullong_relaxed(&self->lock.thread, owner); self->lock.level = count - 1; Py_RETURN_NONE; } @@ -1093,17 +1093,16 @@ rlock_release_save(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - PyThread_ident_t owner = self->lock.thread; - size_t count = self->lock.level + 1; - - if (_PyRecursiveMutex_TryUnlock(&self->lock) < 0) - { + if (!_PyRecursiveMutex_IsLockedByCurrentThread(&self->lock)) { PyErr_SetString(PyExc_RuntimeError, "cannot release un-acquired lock"); return NULL; } - self->lock.thread = 0; - self->lock.level = 0; + + PyThread_ident_t owner = self->lock.thread; + size_t count = self->lock.level + 1; + self->lock.level = 0; // ensure the unlock releases the lock + _PyRecursiveMutex_Unlock(&self->lock); return Py_BuildValue("k" Py_PARSE_THREAD_IDENT_T, count, owner); } From e1b0b749ab9c0e082c56995b3eb5c08f30e539d3 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 21:00:22 +0000 Subject: [PATCH 09/12] Use PyMutex_Unlock in _PyRecursiveMutex_TryUnlock --- Python/lock.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Python/lock.c b/Python/lock.c index 8f1f14d8127642..b51a49df1e0cce 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -415,9 +415,7 @@ _PyRecursiveMutex_TryUnlock(_PyRecursiveMutex *m) } assert(m->level == 0); _Py_atomic_store_ullong_relaxed(&m->thread, 0); - if (_PyMutex_TryUnlock(&m->mutex) < 0) { - return -1; - } + PyMutex_Unlock(&m->mutex); return 0; } From 5e8f8dcd6064482b642c912e4346f7490dcb2e5d Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 21:28:54 +0000 Subject: [PATCH 10/12] fix rlock_is_owned on wasi --- Modules/_threadmodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index de26b811b60cc5..74b1819936774d 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1132,7 +1132,8 @@ static PyObject * rlock_is_owned(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - long owned = _PyRecursiveMutex_IsLockedByCurrentThread(&self->lock); + long owned = _PyRecursiveMutex_IsLockedByCurrentThread(&self->lock) + && PyMutex_IsLocked(&self->lock.mutex); return PyBool_FromLong(owned); } From 5edf49b6afd31482243caf2a3888196c447dbc90 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 9 Oct 2024 06:46:25 +0000 Subject: [PATCH 11/12] Revert "fix rlock_is_owned on wasi" This reverts commit 5e8f8dcd6064482b642c912e4346f7490dcb2e5d. --- Modules/_threadmodule.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 74b1819936774d..de26b811b60cc5 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1132,8 +1132,7 @@ static PyObject * rlock_is_owned(PyObject *op, PyObject *Py_UNUSED(ignored)) { rlockobject *self = (rlockobject*)op; - long owned = _PyRecursiveMutex_IsLockedByCurrentThread(&self->lock) - && PyMutex_IsLocked(&self->lock.mutex); + long owned = _PyRecursiveMutex_IsLockedByCurrentThread(&self->lock); return PyBool_FromLong(owned); } From 85ba63227602e704acc73c5f90e5fe7fe90ea1d6 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Mon, 14 Oct 2024 08:01:59 +0000 Subject: [PATCH 12/12] use Py_ssize_t for count --- Modules/_threadmodule.c | 10 +++++----- Python/lock.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index de26b811b60cc5..d4408aa9e42d9d 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1070,15 +1070,15 @@ rlock_acquire_restore(PyObject *op, PyObject *args) { rlockobject *self = (rlockobject*)op; PyThread_ident_t owner; - unsigned long count; + Py_ssize_t count; - if (!PyArg_ParseTuple(args, "(k" Py_PARSE_THREAD_IDENT_T "):_acquire_restore", + if (!PyArg_ParseTuple(args, "(n" Py_PARSE_THREAD_IDENT_T "):_acquire_restore", &count, &owner)) return NULL; _PyRecursiveMutex_Lock(&self->lock); _Py_atomic_store_ullong_relaxed(&self->lock.thread, owner); - self->lock.level = count - 1; + self->lock.level = (size_t)count - 1; Py_RETURN_NONE; } @@ -1100,10 +1100,10 @@ rlock_release_save(PyObject *op, PyObject *Py_UNUSED(ignored)) } PyThread_ident_t owner = self->lock.thread; - size_t count = self->lock.level + 1; + Py_ssize_t count = self->lock.level + 1; self->lock.level = 0; // ensure the unlock releases the lock _PyRecursiveMutex_Unlock(&self->lock); - return Py_BuildValue("k" Py_PARSE_THREAD_IDENT_T, count, owner); + return Py_BuildValue("n" Py_PARSE_THREAD_IDENT_T, count, owner); } PyDoc_STRVAR(rlock_release_save_doc, diff --git a/Python/lock.c b/Python/lock.c index b51a49df1e0cce..554c51d7780322 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -398,7 +398,7 @@ _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m) { if (_PyRecursiveMutex_TryUnlock(m) < 0) { Py_FatalError("unlocking a recursive mutex that is not " - "owned by the current thread"); + "owned by the current thread"); } }