RFR: 8087322: Implement a Semaphore utility class

[Stefan Karlsson]

On 2015-06-12 21:36, Kim Barrett wrote:
> On Jun 12, 2015, at 2:45 PM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>> On 2015-06-12 20:08, Kim Barrett wrote:
>>> src/os/solaris/vm/os_solaris.cpp
>>> 2288 void Semaphore::signal() {
>>> 2289   int ret = sema_post(&_semaphore);
>>> 2290
>>> 2291   assert(ret == 0, err_msg("Semaphore signal failed: %d", errno));
>>> 2292 }
>>>
>>> sema_post is documented as returning the error code directly, rather
>>> than "returning" it in errno.  Fixing that would also fix the
>>> presently assigned but never used state of ret in non-debug builds.
>>>
>> You might be right, and I'll probably need to write a test that overflows the semaphore, but the man page says:
>> ---
>> RETURN VALUES
>>
>> Upon successful completion, 0 is returned; otherwise, a non-zero value indicates an error.
>>
>>
>> ...
>> The sema_trywait() function will fail if::
>>
>> EBUSY
>> The semaphore pointed to by sp has a zero count.
>>
>> The sema_post() function will fail if:
>>
>> EOVERFLOW
>> The semaphore value pointed to by sp exceeds SEM_VALUE_MAX .
>>
>> ...
>>
>>               if (sema_trywait(&tellers) != 0)
>>                     if (errno == EAGAIN){ /* no teller available */
>>
>> ---
>>
>> So, according to the example the error number is returned in errno, AFAICT.
> OK, the description in the documentation I’m looking at is unclear.  Maybe there is more
> recent documentation?  (What I’m looking at says last revised in 1998, but this kind of stuff
> tends to be quite stable.)
>
> You are probably correct, but having the semantics hidden away in examples is <expletive deleted>.

And now that I read the example above I see that it checks against 
EAGAIN, but the sema_post documentation mentions EBUSY. So I'm confused.

>
>>> src/share/vm/utilities/semaphore.hpp
>>>
>>> The Semaphore class should not be copyable or copy-assignable.
>>>
>> The same goes for Monitor and Mutex, right? I'll fix this for Semaphore.
> And probably a whole lot of other classes.  This is one of those hygiene things whose
> lack irritates me about this code base.  I don’t know if I should be trying to change it
> or just get used to the way it is.
>
Maybe we should create a couple of RFEs and clean up some of the 
classes, just to get things moving in the right direction?

Thanks,
StefanK