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.

Author tim.peters
Recipients
Date 2006-03-24.00:17:10
SpamBayes Score
Marked as misclassified
Message-id
In-reply-to
Content
Logged In: YES 
user_id=31435

I marked this as Accepted, but there are some things I'd
like to see changed:

- A Condition is best named after the predicate it
represents.  So, e.g., instead of the generic "waiter", a
better name would be "all_tasks_done".  When you eventually
.notify() the Condition, you're notifing its wait()er that
"all tasks (may be) done", and "all tasks (may be) done" is
what the wait()er is waiting _for_.  "all_tasks_done.wait()"
 makes that much clearer than "waiter.wait()".

- A Condition.wait() can be interrupted by (at least)
KeyboardInterrupt, so the acquire/release around a
Condition.wait() call should always be in a try/finally (so
that the Condition is release()d no matter what).  All other
Condition.wait()s in Queue do protect themselves this way. 
I don't see a need for try/finally around other uses, except
possibly that:

- Given the intended semantics, it would be good to raise an
exception if .unfinished_tasks becomes negative; i.e., make
it a detected programmer error if task_done() is called "too
often" (although again the Condition needs to be release()d
no matter what, and a try/finally may be expedient toward
that end).

- Since any number of threads _may_ be waiting in
Queue.join(), yes, .notifyAll() is better.  The other
conditions in Queue don't do that because there's a key
difference:  at most one thread waiting on not_full or
not_empty can make progress when one of those is "signaled",
so it would be wasteful to wake up more than one thread
waiting on those.  In contrast, all threads waiting on
.waiter can make progress when all tasks are in fact done. 
You can do that with a notifyAll() in task_done(), or by
adding a notify() near the end of join() (then all threads
waiting on this condition will get notified in domino
fashion).  The notifyAll() way is "purer".

- It's inevitable that someone will ask Queue.join() to grow
an optional timeout argument.  OK by me if that waits ;-).
History
Date User Action Args
2007-08-23 15:47:09adminlinkissue1455676 messages
2007-08-23 15:47:09admincreate