On Thu, 15 Aug 2024 19:02:38 GMT, Viktor Klang <vkl...@openjdk.org> wrote:

>> My suspicion is that Condition::await() throws before having successfully 
>> reacquired the lock, and this exception is swallowed because Lock::unlock() 
>> then throws when invoke with an IllegalMonitorStateException as the current 
>> thread was not reestablished as the owner.
>> 
>> So this PR is intended to harden the reacquisition of await-ing methods in 
>> AQS and AQLS so that instead of throwing they spin-loop trying to reacquire 
>> the lock—at least a thread dump would show where the Thread is stuck trying 
>> to reacquire.
>> 
>> There's also the option of attempting to log a JFR event on the first 
>> reacquisition failure, but that might need to be done with a pre-created 
>> event as the cause of the failing acquisition may be an OOME.
>> 
>> I also needed to harden the TestDisposerRace itself, as it currently tries 
>> to provoke OOMEs but then proceeds to want to allocate objects used for the 
>> test itself. I'm not super-happy about the changes there, but they should be 
>> safer than before.
>> 
>> The first RuntimeException/Error encountered will be rethrown upon success 
>> to facilitate debugging.
>
> src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
>  line 290:
> 
>> 288:                 acquire(node, arg, false, false, false, 0L);
>> 289:                 break;
>> 290:             } catch (Error | RuntimeException ex) {
> 
> @DougLea @AlanBateman The first question is if this set of exception are the 
> way to go, or we should go with Throwable

One option is to record the first encountered exception instance, which can 
then be rethrown once acquire succeeds?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20602#discussion_r1718994076

Reply via email to