[jdk17] RFR: 8269865: Async UL needs to handle ERANGE on exceeding SEM_VALUE_MAX [v4]

[David Holmes]

On 8/07/2021 11:23 am, Xin Liu wrote:
> On Thu, 8 Jul 2021 00:23:17 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
>>>
>>>    Use the new API signal_overflow of semaphore
>>>    
>>>    This patch handles overflow scenerios for Posix and Windows.
>>>    MacOS platform doesn't have any error so we ignore it.
>>
>> src/hotspot/share/runtime/semaphore.hpp line 56:
>>
>>> 54:
>>> 55:   // Ignore error of overflow
>>> 56:   void signal_overflow(uint count = 1) {
>>
>> I'm a bit unsure about how to name and describe this given that the overflow check only relates to debug builds - in a product build both `signal` and `signal_overflow` are identical.
>>
>> I had missed previously that the `ignore_overflow` parameter was only in the impl API not the main public API.
> 
> Thanks you for reviewing this change.
> 
> The reason we only had ignore_overflow in impl API because I defined my own platform-independent Semaphore class, aka. AsyncLogSemaphore in previous revision.  I changed type of _sem from Sempahore to AsyncLogSemaphore. I "overloaded" signal(uint count) in that way so  I don't  pollute "public API".
> 
> 
>> I don't think this is necessary, just call wait(1, true) where needed.
> 
> I think your opinion is that it's not necessary.  how about I just add a second parameter ignore_overflow to Semaphore::signal with the default value = false?

Sorry I need to think more about the options here. I don't like 
modifying the public API for a debug-only condition, and I'm not sure 
how best to address that.

David

> -------------
> 
> PR: https://git.openjdk.java.net/jdk17/pull/216
>