[Python-bugs-list] [ python-Bugs-443614 ] enhancement for threading._Semaphore

[noreply@sourceforge.net]

Bugs item #443614, was opened at 2001-07-22 14:16
You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=105470&aid=443614&group_id=5470

Category: Python Library
Group: Feature Request
Status: Open
Resolution: Accepted
Priority: 5
Submitted By: Skip Montanaro (montanaro)
Assigned to: Skip Montanaro (montanaro)
Summary: enhancement for threading._Semaphore

Initial Comment:
The threading._Semaphore class subclasses
threading._Verbose
but takes no advantage of this, never calling
self._note.  In
addition, it seems to me that most of the time it would
be
a bug if a semaphore is released more times than it is
acquired.  The attached patch adds to calls to
self._note
similar to those in threading._RLock adds a new
instance
variable, __initial_value, and adds an assert to
_Semaphore.release to compare the current value of the
semaphore with its initial value.

If it is felt that the semantic change to the
_Semaphore
class caused by the addition of the assert statement
would
break code, I recommend that  _Semaphore be subclassed
with the release method overridden to include the
assert
check.


----------------------------------------------------------------------

>Comment By: Tim Peters (tim_one)
Date: 2001-08-18 13:36

Message:
Logged In: YES 
user_id=31435

Guido, in my experience semaphores are most often used when 
you have a fixed and known-in-advance number of a shared 
resource.  Like (say) 8 printers.  Then you init a 
semaphore to 8, and its current value is meant always to 
reflect the # of printers available.  Thus in the absence 
of printers popping into existence by magic, "0 <= 
current_value <= 8" is an invariant of correct usage.

OTOH, perhaps new printers may get installed over the life 
of a program run.  In that case the printer installer may 
well want to do a V without having done a P first, and so 
increase the maximum -- but then it would still be a logic 
bug if the program ever exceeded the new maximum.

BoundedSemaphore may be a good subclass name?

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-08-18 10:55

Message:
Logged In: YES 
user_id=6380

Skip, go ahead and check in the logging change.

I must be insufficiently familiar with usage patterns of
semaphores to appreciate the motivation for the maximum
check. Offhand, a subclass seems a good idea.

----------------------------------------------------------------------

Comment By: Skip Montanaro (montanaro)
Date: 2001-08-17 18:38

Message:
Logged In: YES 
user_id=44345

I've backed out the check for a maximum in my copy.
All it has now is the _note change.  I'll check that
in anytime you say so and resubmit the other change with
the optional constructor argument Tim mentioned and a
doc update.  Tim, do you have a preferred name, say,
"strict_maximum"?


----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2001-08-17 16:20

Message:
Logged In: YES 
user_id=31435

Guido, the comment in threading._Semaphore makes clear that 
you deliberately didn't want a maximum, so a question for 
you is whether you think differently now.  Since my old 
Sempahore class did have a maximum, it's clear what I 
thought <wink>.

In any case it's hard to approve of changing semantics 
now.  Skip should check in the logging (that can't be 
controversial -- I hope), and split the semantic change 
into a different patch.

A new optional argument on the constructor saying whether 
or not you want a maximum would be backward compatible.  
Both kinds of semaphores can be useful, but I'm not sure 
*so* useful that it justifies two distinct classes (even if 
one is a subclass of the other).

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-08-17 09:57

Message:
Logged In: YES 
user_id=6380

Looks good to me.

Tim, do you approve of this?  If so, assign to Skip for
checkin.

----------------------------------------------------------------------

You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=105470&aid=443614&group_id=5470