classification
Title: Add a threading.Condition.wait_for() method
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: Nosy List: georg.brandl, jyasskin, kristjan.jonsson, pitrou
Priority: normal Keywords: patch

Created on 2010-10-31 04:46 by kristjan.jonsson, last changed 2010-11-18 12:47 by kristjan.jonsson. This issue is now closed.

Files
File name Uploaded Description Edit
wait_for.patch kristjan.jonsson, 2010-10-31 04:46 review
wait_for2.patch kristjan.jonsson, 2010-11-13 10:29
Messages (13)
msg120036 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2010-10-31 04:46
The attached patch adds a wait_for method to condition objects.
It can simplify code that uses condition variables since it relieves the user from writing a condition loop.  In addition it simplifies timeout logic which otherwise has to correctly deal with non-successful wakeups.

We also modify the barrier to use it, giving more robust timeout behaviour.

(btw, the use of time.time() in threading is unfortunate, since it has low resolution and is affected by a user adjusting the clock. On Windows one would want time.clock(), but that function measures CPU time on unix.  Do we need a proper time.wallclock() function available on all platforms?)
msg120126 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2010-11-01 13:54
The wait_for() method is basically a distillation of the Semaphore.acquire() method, which tries to intelligently handle a non-trivial timeout.
With this method it is now possible to simplify Semaphore.acquire, although I didn't want to do so in this patch to keep its scope smaller.
msg120128 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-01 14:01
This looks useful indeed.
msg120133 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2010-11-01 15:22
* This method will confuse some people who will think that cond.wait(pred) will wake up when pred becomes true regardless of whether they call cond.notifyAll(). You should warn them about this in the documentation. (This confusion happens inside Google, despite our documentation, but the method's worth having even though not everyone will read the docs.) You should also mention that 'predicate' runs with the Condition acquired.

Then +1 whether or not you do anything in response to the below comments.

* There's a small risk of confusion with C++0x's wait_for method, which behaves like the current Condition.wait (waiting for a timeout). They used "wait_for" because they also have a "wait_until" that waits until a deadline. I don't think this potential confusion is a big deal.

* This expands the interface needed to duck-type as a Condition. Maybe you could also add a threading.wait_for(Condition, predicate, timeout) that implements the same thing using just the Condition's .wait() method? I'm not certain that'll be the best name as a threading method, but I don't have a better proposal.
msg120195 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2010-11-02 00:29
Good points, Jeffrey.  Documentation can be improved and cond.wait_for(pred) is indeed not guaranteed to wake up when predicate is true unless someone calls notifyAll.

I spent some time thinking of a name.  I tried wait_predicate and predicate_wait, but wait_for seemed natural.  Any other ideas?
How about wait_until_true?

My original method had this as a free function, but I moved it into the Condition because I could see no other kind of primitive that would use it.  I agree that it is unfortunate to pull what is essentially a utility function into the Condition variable, so I am leaning towards keeping it a module function.
msg120214 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-02 11:29
> I spent some time thinking of a name.  I tried wait_predicate and
> predicate_wait, but wait_for seemed natural.  Any other ideas?
> How about wait_until_true?

wait_for is ok IMO.

> My original method had this as a free function, but I moved it into
> the Condition because I could see no other kind of primitive that
> would use it.  I agree that it is unfortunate to pull what is
> essentially a utility function into the Condition variable, so I am
> leaning towards keeping it a module function.

I'm not sure I see the point. It's an operation on a Condition variable,
so it's natural to have it as a Condition method. A module function
would feel rather weird.
msg120371 - (view) Author: Jeffrey Yasskin (jyasskin) * (Python committer) Date: 2010-11-04 02:04
On Tue, Nov 2, 2010 at 4:29 AM, Antoine Pitrou <report@bugs.python.org> wrote:
>> I spent some time thinking of a name.  I tried wait_predicate and
>> predicate_wait, but wait_for seemed natural.  Any other ideas?
>> How about wait_until_true?
>
> wait_for is ok IMO.

Agreed.

>> My original method had this as a free function, but I moved it into
>> the Condition because I could see no other kind of primitive that
>> would use it.  I agree that it is unfortunate to pull what is
>> essentially a utility function into the Condition variable, so I am
>> leaning towards keeping it a module function.
>
> I'm not sure I see the point. It's an operation on a Condition variable,
> so it's natural to have it as a Condition method. A module function
> would feel rather weird.

Yeah, it should primarily be used as a Condition method. I was
suggesting implementing that Condition method in terms of a threading
function, which would also help other people trying to, say, mock
Condition objects. But that's not a big deal, and I could be wrong
about whether it's useful at all. As I said earlier, I'm happy with
this patch either way. (Note that Condition.wait_for is helpful to
people mocking Condition anyway, since the number of calls is much
more fixed than the calls to Condition.wait.)
msg121126 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2010-11-13 10:29
Ok, here is a new patch which slightly expands the documentation and improves the timeout unittest.
If there are no objections I'll then commit this shortly.
msg121129 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-11-13 10:39
Good, but please wait until after the a4 freeze.
msg121130 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-13 12:04
> Ok, here is a new patch which slightly expands the documentation and improves the timeout unittest.
> If there are no objections I'll then commit this shortly.

Again, I think you should use a larger timeout value than 0.1, to avoid
intermittent buildbot failures.
msg121171 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2010-11-14 02:20
Good point, Antoine.  I'm always trying to keep those timeouts low, however, to avoid having the testsuite duration grow too much with every test :)

I think we can probably fix the issue by having the lock_tests.Bunch() function only return when all threads have checked in, thus avoiding thread startup delays affecting our timeouts.
msg121231 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-15 14:39
> Good point, Antoine.  I'm always trying to keep those timeouts low,
> however, to avoid having the testsuite duration grow too much with
> every test :)

Well, better to have slower tests than intermittently failing ones, I
say.
msg121454 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2010-11-18 12:47
Committed as revision 86510
History
Date User Action Args
2010-11-18 12:47:13kristjan.jonssonsetstatus: open -> closed
resolution: accepted
messages: + msg121454
2010-11-15 14:39:05pitrousetmessages: + msg121231
2010-11-14 02:20:34kristjan.jonssonsetmessages: + msg121171
2010-11-13 12:04:56pitrousetmessages: + msg121130
2010-11-13 10:39:56georg.brandlsetmessages: + msg121129
2010-11-13 10:29:35kristjan.jonssonsetfiles: + wait_for2.patch

messages: + msg121126
2010-11-04 02:04:58jyasskinsetmessages: + msg120371
2010-11-02 11:29:03pitrousetmessages: + msg120214
2010-11-02 00:29:12kristjan.jonssonsetmessages: + msg120195
2010-11-01 15:22:50jyasskinsetmessages: + msg120133
2010-11-01 14:01:20pitrousetnosy: + jyasskin, pitrou
messages: + msg120128
2010-11-01 13:54:12kristjan.jonssonsetmessages: + msg120126
2010-10-31 08:02:05rhettingersettitle: Add a thrading.Condition.wait_for() method -> Add a threading.Condition.wait_for() method
2010-10-31 04:46:48kristjan.jonssoncreate