From 9638a86b8e2486aa666635c820f1e75448f576ed Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Fri, 30 Oct 2015 03:10:07 +0200 Subject: [PATCH] Issue #25516: threading.Condition._is_owned() fails when condition is held by another thread. Replace lock's locked bit with thread id and implement _is_owned in_thread and _dummy_thread modules. --- Lib/_dummy_thread.py | 2 ++ Lib/threading.py | 16 ++-------------- Modules/_threadmodule.c | 42 ++++++++++++++++++++++++++++++------------ 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/Lib/_dummy_thread.py b/Lib/_dummy_thread.py index 36e5f38..a62ff3e 100644 --- a/Lib/_dummy_thread.py +++ b/Lib/_dummy_thread.py @@ -140,6 +140,8 @@ class LockType(object): def locked(self): return self.locked_status + _is_owned = locked + def __repr__(self): return "<%s %s.%s object at %s>" % ( "locked" if self.locked_status else "unlocked", diff --git a/Lib/threading.py b/Lib/threading.py index 828019d..479a483 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -219,7 +219,7 @@ class Condition: self.release = lock.release # If the lock defines _release_save() and/or _acquire_restore(), # these override the default implementations (which just call - # release() and acquire() on the lock). Ditto for _is_owned(). + # release() and acquire() on the lock). try: self._release_save = lock._release_save except AttributeError: @@ -228,10 +228,7 @@ class Condition: self._acquire_restore = lock._acquire_restore except AttributeError: pass - try: - self._is_owned = lock._is_owned - except AttributeError: - pass + self._is_owned = lock._is_owned self._waiters = _deque() def __enter__(self): @@ -249,15 +246,6 @@ class Condition: def _acquire_restore(self, x): self._lock.acquire() # Ignore saved state - def _is_owned(self): - # Return True if lock is owned by current_thread. - # This method is called only if _lock doesn't have _is_owned(). - if self._lock.acquire(0): - self._lock.release() - return False - else: - return True - def wait(self, timeout=None): """Wait until notified or until a timeout occurs. diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index bcb3aee..a892113 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -25,7 +25,7 @@ typedef struct { PyObject_HEAD PyThread_type_lock lock_lock; PyObject *in_weakreflist; - char locked; /* for sanity checking */ + long lock_owner; /* for sanity checking */ } lockobject; static void @@ -35,7 +35,7 @@ lock_dealloc(lockobject *self) PyObject_ClearWeakRefs((PyObject *) self); if (self->lock_lock != NULL) { /* Unlock the lock so it's safe to free it */ - if (self->locked) + if (self->lock_owner) PyThread_release_lock(self->lock_lock); PyThread_free_lock(self->lock_lock); } @@ -154,7 +154,7 @@ lock_PyThread_acquire_lock(lockobject *self, PyObject *args, PyObject *kwds) } if (r == PY_LOCK_ACQUIRED) - self->locked = 1; + self->lock_owner = PyThread_get_thread_ident(); return PyBool_FromLong(r == PY_LOCK_ACQUIRED); } @@ -173,13 +173,13 @@ static PyObject * lock_PyThread_release_lock(lockobject *self) { /* Sanity check: the lock must be locked */ - if (!self->locked) { + if (!self->lock_owner) { PyErr_SetString(ThreadError, "release unlocked lock"); return NULL; } PyThread_release_lock(self->lock_lock); - self->locked = 0; + self->lock_owner = 0; Py_INCREF(Py_None); return Py_None; } @@ -195,7 +195,7 @@ but it needn't be locked by the same thread that unlocks it."); static PyObject * lock_locked_lock(lockobject *self) { - return PyBool_FromLong((long)self->locked); + return PyBool_FromLong(self->lock_owner); } PyDoc_STRVAR(locked_doc, @@ -207,10 +207,27 @@ Return whether the lock is in the locked state."); static PyObject * lock_repr(lockobject *self) { - return PyUnicode_FromFormat("<%s %s object at %p>", - self->locked ? "locked" : "unlocked", Py_TYPE(self)->tp_name, self); + return PyUnicode_FromFormat("<%s %s object owner=%ld at %p>", + self->lock_owner ? "locked" : "unlocked", Py_TYPE(self)->tp_name, + self->lock_owner, self); } +static PyObject * +lock_is_owned(lockobject *self) +{ + long tid = PyThread_get_thread_ident(); + + if (self->lock_owner == tid) { + Py_RETURN_TRUE; + } + Py_RETURN_FALSE; +} + +PyDoc_STRVAR(lock_is_owned_doc, +"_is_owned() -> bool\n\ +\n\ +For internal use by `threading.Condition`."); + static PyMethodDef lock_methods[] = { {"acquire_lock", (PyCFunction)lock_PyThread_acquire_lock, METH_VARARGS | METH_KEYWORDS, acquire_doc}, @@ -228,6 +245,8 @@ static PyMethodDef lock_methods[] = { METH_VARARGS | METH_KEYWORDS, acquire_doc}, {"__exit__", (PyCFunction)lock_PyThread_release_lock, METH_VARARGS, release_doc}, + {"_is_owned", (PyCFunction)lock_is_owned, + METH_NOARGS, lock_is_owned_doc}, {NULL, NULL} /* sentinel */ }; @@ -472,7 +491,6 @@ rlock_repr(rlockobject *self) self->rlock_count, self); } - static PyMethodDef rlock_methods[] = { {"acquire", (PyCFunction)rlock_acquire, METH_VARARGS | METH_KEYWORDS, rlock_acquire_doc}, @@ -542,7 +560,7 @@ newlockobject(void) if (self == NULL) return NULL; self->lock_lock = PyThread_allocate_lock(); - self->locked = 0; + self->lock_owner = 0; self->in_weakreflist = NULL; if (self->lock_lock == NULL) { Py_DECREF(self); @@ -1188,9 +1206,9 @@ release_sentinel(void *wr) if (obj != Py_None) { assert(Py_TYPE(obj) == &Locktype); lock = (lockobject *) obj; - if (lock->locked) { + if (lock->lock_owner) { PyThread_release_lock(lock->lock_lock); - lock->locked = 0; + lock->lock_owner = 0; } } /* Deallocating a weakref with a NULL callback only calls -- 2.4.3