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

David Holmes dholmes at openjdk.java.net
Thu Jul 8 00:26:52 UTC 2021


On Wed, 7 Jul 2021 08:08:10 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> This patch solved the sempahore overflow issue with errno ERANGE or EOVERFLOW.
>> Previously, we have asymmetric p/v operations for semaphore _sem. Each iteration
>> only decrements _sem 1 but dequeues N messages. If logging threads keep preempting
>> async logging thread, it may cause the value of _sem accumulates until overflow! 
>> 
>> The patch corrects the value of _sem after write(). n messages are dequeued/processed.
>> We need to invoke _sem.wait() max(n-1, 1) time. This ensures that each iteration
>> decrements n instead of 1.
>
> 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.

Figuring out the best way to handle this is proving to be quite tricky - sorry. I need to think some more about it. Unfortunately I'm away for a few days after today.

David

src/hotspot/share/runtime/semaphore.hpp line 53:

> 51:   ~Semaphore() {}
> 52: 
> 53:   void signal(uint count = 1) { _impl.signal(count, false /* ignore_overflow */); }

Please change comment to:

`/* don't ignore overflow */`

as that describes what the `false` argument actually means, rather than just showing what parameter it is providing the value for.

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.

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

Changes requested by dholmes (Reviewer).

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


More information about the hotspot-runtime-dev mailing list