RFR: 8268638: semaphores of AsyncLogWriter may be broken when JVM is exiting. [v2]

[David Holmes]

On Mon, 14 Jun 2021 23:38:01 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> src/hotspot/share/logging/logAsyncWriter.cpp line 36:
>> 
>>> 34:  public:
>>> 35:   AsyncLogLocker(Semaphore* sem): _sem(sem) {
>>> 36:     assert(sem != nullptr && sem->value() == 1,
>> 
>> The value() check is not necessary. Please do not pollute the Semaphore API with this.
>> 
>> Rather than passing the Semaphore in it should just be accessed directly as there is only one Semaphore to ever be used here. Make AsyncLogLocker a friend class to allow access.
>
> Thanks.  I remove Semaphore::value() and declare friend class SemaphoreLocker there. 
> 
> My intention is to do a sanity check. A semaphore can be used to implement a low-level lock if and only if its value is 1. It means that only one thread can enter the critical region at any time.  However, it's error-prone for the RAII class to accept any arbitrary semaphore. It's because its value could be any non-negative number.

I guess I wasn't clear enough.

Please do not make any changes to the Semaphore class. This "value" is not needed in general and not needed for your sanity checking.

The Semaphore _lock is defined in the AsyncLogWriter class and initialized to 1 so it can be used to mimic a mutual exclusion lock. This does not need to be sanity checked as it is all handled within a small chunk of code. The AsyncLogLocker does not take a semaphore, it is a friend of AsyncLogWriter and accesses _lock directly.

Thanks,
David

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

PR: https://git.openjdk.java.net/jdk/pull/4479