On Wed, 10 Jun 2026 02:06:39 GMT, Kirill Shirokov <[email protected]> wrote:

>> Viktor Klang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Indentation and comment refinement
>
> test/jdk/java/util/concurrent/tck/SemaphoreTest.java line 708:
> 
>> 706:             final int permitsAvailable = s.availablePermits();
>> 707:             done.set(true);                       // Ensure that tasks 
>> can exit
>> 708:             assertTrue(permitsAvailable < width); // Some permits 
>> should've been taken
> 
> I think it is better to check for `permitsAvailable == 0` here, so we catch a 
> case where only some of threads are stuck. For this particular bug all the 
> threads will be livelocked spinning on the same Node, but this test may have 
> additional coverage if we assume that due to an unknown bug only some of them 
> can be stuck. Just a thought.
> 
> Additionally, it would be great to check the stacks of the running acquire() 
> threads to see if some of are RUNNABLE and still in `cleanQueue()`.

We can't guarantee that all permits will have been taken by the time of the 
assertion, and I'd rather avoid spurious false positives.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31436#discussion_r3424381142

Reply via email to