RFR: 8345266: java/util/concurrent/locks/StampedLock/OOMEInStampedLock.java JTREG_TEST_THREAD_FACTORY=Virtual fails with OOME [v2]

[Patricio Chilano Mateo]

On Tue, 17 Dec 2024 11:52:34 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> The update to Continuation::try_preempt looks good to me. Initially I thought it might be problematic to clear the exception in the face of an async exception from JVMTI StopThread but that is limited to the suspended case so can't happen.
> 
And we check for those in `JvmtiUnmountBeginMark()`. If no async exception is installed then no new JVMTI StopThread operation will be able to progress since we already started the VTMS transition.

> Tests for OOME scenarios have a history of being troublesome. This one looks manageable but I worry a bit that it might need further work in the future to keep stable.
>
Yes, I tried to be careful when writing the test to consider corner cases. I guess we could give it a chance and if we see issues down the road we can remove it.

> test/jdk/java/lang/Thread/virtual/MonitorOOM.java line 35:
> 
>> 33: /*
>> 34:  * @test id=wait
>> 35:  * @bug 8345266
> 
> We don't have to list the @bug tag in all test descriptions if you don't want. This comes up when adding more test cases to an existing test where we add to the @bug tag.

Removed.

> test/jdk/java/lang/Thread/virtual/MonitorOOM.java line 45:
> 
>> 43: import java.util.concurrent.atomic.AtomicReference;
>> 44: 
>> 45: public class MonitorOOM {
> 
> For naming, something like MonitorEnterWaitOOME might be clearer.

Renamed to MonitorEnterWaitOOME.

> test/jdk/java/lang/Thread/virtual/MonitorOOM.java line 65:
> 
>> 63:                     synchronized (lock) {
>> 64:                         if (testWait) {
>> 65:                             lock.wait(5);
> 
> This is a timed wait, I suppose we should also test the untimed wait.

Added the untimed case.

> test/jdk/java/lang/Thread/virtual/MonitorOOM.java line 79:
> 
>> 77:             canFillHeap.set(true);
>> 78:             awaitTrue(heapFilled);
>> 79:             loopMillis(100); // allow vthread to reach synchronized
> 
> 100ms may not be enough, I wonder if this could poll the thread state until BLOCKED.

I was worried about calling anything else that might allocate memory. But polling the state should be fine, I changed it.

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

PR Comment: https://git.openjdk.org/jdk/pull/22780#issuecomment-2549919237
PR Review Comment: https://git.openjdk.org/jdk/pull/22780#discussion_r1889388418
PR Review Comment: https://git.openjdk.org/jdk/pull/22780#discussion_r1889388587
PR Review Comment: https://git.openjdk.org/jdk/pull/22780#discussion_r1889388726
PR Review Comment: https://git.openjdk.org/jdk/pull/22780#discussion_r1889389071