This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Condition._is_owned is wrong
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: duplicate
Dependencies: Superseder: threading.Condition._is_owned() is wrong when using threading.Lock
View: 25516
Assigned To: Nosy List: Antony.Lee, berker.peksag, mark.dickinson, neologix, tim.peters
Priority: normal Keywords:

Created on 2014-01-14 00:55 by Antony.Lee, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Messages (4)
msg208063 - (view) Author: Antony Lee (Antony.Lee) * Date: 2014-01-14 00:55
I believe that the implementation of Condition._is_owned is wrong, as mentioned here: https://mail.python.org/pipermail/python-list/2012-October/632682.html.  Specifically, the two return values (True and False) should be inverted.

I guess this slipped through as this private function would only be called if one passed to a Condition object a (R)Lock-like object that doesn't define _is_owned.  (I noticed this when I tried to pass a custom-written reader-writer lock to a Condition object.)  Technically, the docs says that "If the lock argument is given and not None, it must be a Lock or RLock object", but in practice anything that defines acquire(), release() (and possibly _is_owned()) should work.
msg208131 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2014-01-15 02:56
They certainly should _not_ be swapped, as explained clearly in the message following the one you referenced.  For the first half:

        if self._lock.acquire(0):

succeeds if and only if the lock is not held by _any_ thread at the time.  In that case, the lock is _acquired_ as a side-effect of performing the test, and the code goes on to restore the lock again to its initially unacquired state, and (correctly) return False:

            self._lock.release()
            return False

The second half of the code is certainly wrong:

        else:
            return True

The docs state:

        # Return True if lock is owned by current_thread.

but the code actually returns True if the lock is currently owned by _any_ thread.  All that is also explained in the thread you pointed at.

However, I don't see any possibility of fixing that.  There is simply no way to know, for an arbitrary "lock-like" object, which thread owns it.

Indeed, on Windows it's not even possible for a threading.Lock.  For example (here under Python3, but I'm sure it's the same under Python2):

>>> import threading as th
>>> lock = th.Lock()
>>> c = th.Condition(lock)
>>> def hmm():
...   print("is_owned:", c._is_owned())
...
>>> t = th.Thread(target=hmm)
>>> t.start()
is_owned: False
>>> lock.acquire()
True
>>> t = th.Thread(target=hmm)
>>> t.start()
is_owned: True

Note that the last line showed True despite that the lock is _not_ owned by the thread doing the print - it's owned by the main program thread.  That's because threading.Lock._is_owned doesn't exist under Windows, so it's falling into the second case of the Condition._is_owned() implementation at issue here.

Also note that if the True and False cases were swapped, _none_ of the output would make any sense at all ;-)
msg210582 - (view) Author: Antony Lee (Antony.Lee) * Date: 2014-02-08 01:42
For the second half, the same behavior applies under Linux -- basically, a simple Lock doesn't have a notion of owning thread and can the be unlocked by any thread.

However, the first half is not correct if the lock used is a *RLock-like* object (that is, it has a notion of ownership, and once acquired it can be reacquired and released only by the owning thread).  If that is the case, then _lock.acquire(0) will succeed if the current thread already owns the lock (or if no one owns it) and fail if the lock is owned by another thread.

So perhaps it may not be possible to do really better, but the docs could (should?) mention that custom implementations of locks passed to Condition objects should implement their own version of _is_owned.
msg264493 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-04-29 12:24
issue 25516 is a duplicate, but I'm going to close this one since issue 25516 has a patch.
History
Date User Action Args
2022-04-11 14:57:56adminsetgithub: 64446
2016-04-29 12:24:45berker.peksagsetstatus: open -> closed

superseder: threading.Condition._is_owned() is wrong when using threading.Lock

nosy: + berker.peksag
messages: + msg264493
resolution: duplicate
stage: resolved
2014-02-08 01:42:45Antony.Leesetmessages: + msg210582
2014-01-15 02:56:20tim.peterssetmessages: + msg208131
2014-01-15 00:04:42pitrousetnosy: + neologix

versions: + Python 2.7, Python 3.3, Python 3.4
2014-01-14 10:57:55mark.dickinsonsetnosy: + tim.peters, mark.dickinson
2014-01-14 00:55:21Antony.Leecreate