Details

Description

According to description for java.util.concurrent.Semaphore it is ok to create a Semaphore with negative amount of permits. Moreover, all Semaphore's methods (including reducePermits, release, etc) works with negative amount of permits correctly (don't give permits until amount became more than zero, etc). Except drainPermits.
If Semaphore has less than zero permits, drainPermits returns negative value (that maybe considered as ok), but also drainPermits raises permits to zero, that is definitely an error.
Suggested fix is quite simple:
Current code:
final int drainPermits() {
for (;;) {
int current = getState();
if (current == 0 || compareAndSetState(current, 0))
return current;
}
}

In general, negative amount of permits should be considered as absence of permits. It only has impact to how many permits we should add (release) until new permits became available. In that case docs for drainPermits is ok now. But I agree that maybe we should add a section about negative permits to overall Semaphore documentation (right now negative permits are mentioned only for constructors)

Sergey Kuksenko
added a comment - 2016-11-04 13:14 In general, negative amount of permits should be considered as absence of permits. It only has impact to how many permits we should add (release) until new permits became available. In that case docs for drainPermits is ok now. But I agree that maybe we should add a section about negative permits to overall Semaphore documentation (right now negative permits are mentioned only for constructors)

Doug Lea
added a comment - 2016-11-07 16:38 I agree that the current (intentional) behavior should be clarified in documentation. Just adding the following seems to suffice:
*** 603,608 ****
--- 603,609 ----
/**
* Acquires and returns all permits that are immediately available.
+ * Upon return, zero permits are available.
*
* @return the number of permits acquired
*/