From ac1aa30bc7641f4c022fcedbbabff88e84bc5ba9 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 18:28:17 +0530 Subject: [PATCH 1/5] use PyMutex --- Modules/_threadmodule.c | 59 +++++++++-------------------------------- 1 file changed, 13 insertions(+), 46 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index b3ed8e7bc56b9e..074bad3aabd288 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -701,9 +701,7 @@ static PyType_Spec ThreadHandle_Type_spec = { typedef struct { PyObject_HEAD - PyThread_type_lock lock_lock; - PyObject *in_weakreflist; - char locked; /* for sanity checking */ + PyMutex lock; } lockobject; static int @@ -717,15 +715,7 @@ static void lock_dealloc(lockobject *self) { PyObject_GC_UnTrack(self); - if (self->in_weakreflist != NULL) { - PyObject_ClearWeakRefs((PyObject *) self); - } - if (self->lock_lock != NULL) { - /* Unlock the lock so it's safe to free it */ - if (self->locked) - PyThread_release_lock(self->lock_lock); - PyThread_free_lock(self->lock_lock); - } + PyObject_ClearWeakRefs((PyObject *) self); PyTypeObject *tp = Py_TYPE(self); tp->tp_free((PyObject*)self); Py_DECREF(tp); @@ -790,13 +780,11 @@ lock_PyThread_acquire_lock(lockobject *self, PyObject *args, PyObject *kwds) if (lock_acquire_parse_args(args, kwds, &timeout) < 0) return NULL; - PyLockStatus r = acquire_timed(self->lock_lock, timeout); + PyLockStatus r = _PyMutex_LockTimed(&self->lock, timeout, _PY_LOCK_DETACH); if (r == PY_LOCK_INTR) { return NULL; } - if (r == PY_LOCK_ACQUIRED) - self->locked = 1; return PyBool_FromLong(r == PY_LOCK_ACQUIRED); } @@ -827,13 +815,11 @@ static PyObject * lock_PyThread_release_lock(lockobject *self, PyObject *Py_UNUSED(ignored)) { /* Sanity check: the lock must be locked */ - if (!self->locked) { + if (_PyMutex_TryUnlock(&self->lock) < 0) { PyErr_SetString(ThreadError, "release unlocked lock"); return NULL; } - self->locked = 0; - PyThread_release_lock(self->lock_lock); Py_RETURN_NONE; } @@ -860,7 +846,7 @@ Release the lock."); static PyObject * lock_locked_lock(lockobject *self, PyObject *Py_UNUSED(ignored)) { - return PyBool_FromLong((long)self->locked); + return PyBool_FromLong(PyMutex_IsLocked(&self->lock)); } PyDoc_STRVAR(locked_doc, @@ -879,24 +865,20 @@ static PyObject * lock_repr(lockobject *self) { return PyUnicode_FromFormat("<%s %s object at %p>", - self->locked ? "locked" : "unlocked", Py_TYPE(self)->tp_name, self); + PyMutex_IsLocked(&self->lock) ? "locked" : "unlocked", Py_TYPE(self)->tp_name, self); } #ifdef HAVE_FORK static PyObject * -lock__at_fork_reinit(lockobject *self, PyObject *Py_UNUSED(args)) +lock__at_fork_reinit(PyObject *op, PyObject *Py_UNUSED(args)) { - if (_PyThread_at_fork_reinit(&self->lock_lock) < 0) { - PyErr_SetString(ThreadError, "failed to reinitialize lock at fork"); - return NULL; - } - - self->locked = 0; - + lockobject *self = (lockobject *)op; + self->lock = (PyMutex){0}; Py_RETURN_NONE; } #endif /* HAVE_FORK */ + static lockobject *newlockobject(PyObject *module); static PyObject * @@ -937,7 +919,7 @@ static PyMethodDef lock_methods[] = { {"__exit__", (PyCFunction)lock_PyThread_release_lock, METH_VARARGS, lock_exit_doc}, #ifdef HAVE_FORK - {"_at_fork_reinit", (PyCFunction)lock__at_fork_reinit, + {"_at_fork_reinit", (PyCFunction)lock__at_fork_reinit, METH_NOARGS, NULL}, #endif {NULL, NULL} /* sentinel */ @@ -958,18 +940,12 @@ A lock is not owned by the thread that locked it; another thread may\n\ unlock it. A thread attempting to lock a lock that it has already locked\n\ will block until another thread unlocks it. Deadlocks may ensue."); -static PyMemberDef lock_type_members[] = { - {"__weaklistoffset__", Py_T_PYSSIZET, offsetof(lockobject, in_weakreflist), Py_READONLY}, - {NULL}, -}; - static PyType_Slot lock_type_slots[] = { {Py_tp_dealloc, (destructor)lock_dealloc}, {Py_tp_repr, (reprfunc)lock_repr}, {Py_tp_doc, (void *)lock_doc}, {Py_tp_methods, lock_methods}, {Py_tp_traverse, lock_traverse}, - {Py_tp_members, lock_type_members}, {Py_tp_new, lock_new}, {0, 0} }; @@ -978,7 +954,7 @@ static PyType_Spec lock_type_spec = { .name = "_thread.lock", .basicsize = sizeof(lockobject), .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | - Py_TPFLAGS_IMMUTABLETYPE), + Py_TPFLAGS_IMMUTABLETYPE | Py_TPFLAGS_MANAGED_WEAKREF), .slots = lock_type_slots, }; @@ -1320,16 +1296,7 @@ newlockobject(PyObject *module) if (self == NULL) { return NULL; } - - self->lock_lock = PyThread_allocate_lock(); - self->locked = 0; - self->in_weakreflist = NULL; - - if (self->lock_lock == NULL) { - Py_DECREF(self); - PyErr_SetString(ThreadError, "can't allocate lock"); - return NULL; - } + self->lock = (PyMutex){0}; return self; } From d8e8c75b1d4523242c210ed16b68d93ce460c865 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 13:49:10 +0000 Subject: [PATCH 2/5] fix merge --- Modules/_threadmodule.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 6c742e3742d1f3..869cac86424eba 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -883,19 +883,8 @@ lock_repr(PyObject *op) static PyObject * lock__at_fork_reinit(PyObject *op, PyObject *Py_UNUSED(args)) { -<<<<<<< HEAD lockobject *self = (lockobject *)op; self->lock = (PyMutex){0}; -======= - lockobject *self = (lockobject*)op; - if (_PyThread_at_fork_reinit(&self->lock_lock) < 0) { - PyErr_SetString(ThreadError, "failed to reinitialize lock at fork"); - return NULL; - } - - self->locked = 0; - ->>>>>>> 5967dd8a4de60a418de84d1d1d9efc063ad12c47 Py_RETURN_NONE; } #endif /* HAVE_FORK */ @@ -941,11 +930,7 @@ static PyMethodDef lock_methods[] = { {"__exit__", lock_PyThread_release_lock, METH_VARARGS, lock_exit_doc}, #ifdef HAVE_FORK -<<<<<<< HEAD {"_at_fork_reinit", (PyCFunction)lock__at_fork_reinit, -======= - {"_at_fork_reinit", lock__at_fork_reinit, ->>>>>>> 5967dd8a4de60a418de84d1d1d9efc063ad12c47 METH_NOARGS, NULL}, #endif {NULL, NULL} /* sentinel */ From a7ad2f825152b38d42b9c1979f9a12ffbad0518b Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 13:59:09 +0000 Subject: [PATCH 3/5] fix signals --- Modules/_threadmodule.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 869cac86424eba..bff996135e3703 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -788,8 +788,10 @@ lock_PyThread_acquire_lock(PyObject *op, PyObject *args, PyObject *kwds) return NULL; } - PyLockStatus r = _PyMutex_LockTimed(&self->lock, timeout, _PY_LOCK_DETACH); - if (r == PY_LOCK_INTR) { + PyLockStatus r = _PyMutex_LockTimed(&self->lock, timeout, + _PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH); + if (r == PY_LOCK_INTR) + { return NULL; } From 8b3435cc268cf8601c84f9db078d7e61defdd1b7 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 14:00:43 +0000 Subject: [PATCH 4/5] format --- Modules/_threadmodule.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index bff996135e3703..e93e4c6753f151 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -790,8 +790,7 @@ lock_PyThread_acquire_lock(PyObject *op, PyObject *args, PyObject *kwds) PyLockStatus r = _PyMutex_LockTimed(&self->lock, timeout, _PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH); - if (r == PY_LOCK_INTR) - { + if (r == PY_LOCK_INTR) { return NULL; } @@ -932,7 +931,7 @@ static PyMethodDef lock_methods[] = { {"__exit__", lock_PyThread_release_lock, METH_VARARGS, lock_exit_doc}, #ifdef HAVE_FORK - {"_at_fork_reinit", (PyCFunction)lock__at_fork_reinit, + {"_at_fork_reinit", lock__at_fork_reinit, METH_NOARGS, NULL}, #endif {NULL, NULL} /* sentinel */ From ae4bec65f6fa9c61778772676b402bc9200d5b48 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Tue, 8 Oct 2024 14:14:40 +0000 Subject: [PATCH 5/5] code review --- Modules/_threadmodule.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index e93e4c6753f151..9617f9cafe76ff 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -885,12 +885,11 @@ static PyObject * lock__at_fork_reinit(PyObject *op, PyObject *Py_UNUSED(args)) { lockobject *self = (lockobject *)op; - self->lock = (PyMutex){0}; + _PyMutex_at_fork_reinit(&self->lock); Py_RETURN_NONE; } #endif /* HAVE_FORK */ - static lockobject *newlockobject(PyObject *module); static PyObject *