[concurrency-interest] Semaphore doc bug [Was: Enforcing ordered execution of critical sections?]

Doug Lea dl at cs.oswego.edu
Mon Dec 22 19:48:47 EST 2014


On 12/22/2014 02:19 PM, Joe Bowbeer wrote:

> I point out a few places where I think the Semaphore javadoc could be improved
> below.

Thanks!

>
> 1. This comment in the class description needs an even strong warning?
>
> "This class also provides convenience methods to acquire and release multiple
> permits at a time. Beware of the increased risk of indefinite postponement when
> these methods are used without fairness set true."

I can't think of anything better than to recast my posted example. Yes?

  * <p>This class also provides convenience methods to {@link
  * #acquire(int) acquire} and {@link #release(int) release} multiple
  * permits at a time. These methods are generally more efficient and
  * effective than loops. However, they do not establish any preference
  * order. For example, if thread A invokes @code{s.acquire(3}) and
  * thread B invokes @code{s.acquire(2)}, and two permits become
  * available, then there is no guarantee that thread B will obtain
  * them unless its acquire came first and Semaphore @code{s} is in
  * fair mode.
  *

> ... acquire(int)
> I think an additional "and" helps readability,

I agree; done.

> 3. The release(int) documentation should make it clearer that *only* one waiting
> thread is selected?
>
> "If any threads are trying to acquire permits, then one is selected and given
> the permits that were just released."

Just clarifying the referent might work at least as well ...

      * If any threads are trying to acquire permits, then one thread is

...with less risk of mis-interpreting or not reading the rest of
the method.

> 4. This statement in the release(int) documentation emboldened me:
>
> "There is no requirement that a thread that releases a permit must have acquired
> that permit by calling acquire. Correct usage of a semaphore is established by
> programming convention in the application."
>
> I'm not sure what to make of the second sentence.  Would this be a good place to
> advise against adding permits that cannot be acquired by any waiting thread?

We could just kill the second sentence if you think it does more harm
than good. I'm not sure why we included it. But I think the above
class-level explanation is a better place to put the non-prioritization
disclaimer.

-Doug



More information about the Concurrency-interest mailing list