Bug 1117956 - [RFE] pthreading.Condition default lock must be RLock
Summary: [RFE] pthreading.Condition default lock must be RLock
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: ovirt-distribution
Classification: oVirt
Component: python-pthreading
Version: python-pthreading-0.1.3
Hardware: Unspecified
OS: Unspecified
unspecified
medium vote
Target Milestone: ---
: ---
Assignee: Yaniv Bronhaim
QA Contact: Pavel Stehlik
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-07-09 17:16 UTC by Nir Soffer
Modified: 2017-01-16 12:35 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-01-16 12:35:33 UTC
oVirt Team: Infra
oourfali: ovirt-future?
sherold: Triaged+
rule-engine: planning_ack?
oourfali: devel_ack?
rule-engine: testing_ack?


Attachments (Terms of Use)

Description Nir Soffer 2014-07-09 17:16:38 UTC
Description of problem:

pthreading.Condition uses Lock by default, but threading.Condition uses RLock. This prevents usage of pthreading.Condition as a drop-in replacement for threading.Condition.

Version-Release number of selected component (if applicable):
pthreading 1.3

Comment 1 Nir Soffer 2014-07-09 17:22:27 UTC
The relevant code:

101 class Condition(object):
102     """
103     Condition class mimics Python native threading.Condition() API
104     on top of the POSIX thread conditional variable synchronization
105     primitive.
106     """
107     def __init__(self, lock=None):
108         self.__lock = lock if lock else Lock()

Python code:

254 class _Condition(_Verbose):
255     """Condition variables allow one or more threads to wait until they are
256        notified by another thread.
257     """
258 
259     def __init__(self, lock=None, verbose=None):
261         if lock is None:
262             lock = RLock()
263         self.__lock = lock

Users of this class can assume that a Condition has a recursive lock, and they
will be surprised when their code will deadlock because pthreading changed the default.

Comment 2 Saggi Mizrahi 2014-07-13 08:01:28 UTC
It would require us to implement the internal interfaces required for that kind of thing to work.

You have to make condition.wait() to actually release the lock up to it's last reference and that do a wait() and when reacquire the lock get the ref count to where it's original count.

Since it hasn't caused an issue, ever, for a few versions now. I don't think it's 3.5. Please move to 3.6.

Comment 3 Nir Soffer 2014-10-12 14:24:51 UTC
For vdsm, we could not care less about Python questionable default, and I prefer that we do not add code to pthreading.RLock that makes it work with conditions.

The best solution would be to break the compatibility with Python Condition (nobody needs that), and prevent creation of Condition without a lock, or with a RLock, because this usage is bad idea and for vdsm we should not allow it.

Comment 5 Red Hat Bugzilla Rules Engine 2015-11-30 22:37:28 UTC
Bug tickets must have version flags set prior to targeting them to a release. Please ask maintainer to set the correct version flags and only then set the target milestone.

Comment 6 Nir Soffer 2017-01-13 13:20:42 UTC
I think we should close this, as pthreading default is fine for vdsm, and we are
we want to kill pthreading anyway moving to python 3.


Note You need to log in before you can comment on or make changes to this bug.