RFR: 8297286: runtime/vthread tests crashing after JDK-8296324 [v13]

[Patricio Chilano Mateo]

On Thu, 30 Mar 2023 21:48:22 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> test/hotspot/jtreg/serviceability/jvmti/vthread/ToggleNotifyJvmtiTest/ToggleNotifyJvmtiTest.java line 129:
>> 
>>> 127:     }
>>> 128: 
>>> 129:     static synchronized private void startThreads() {
>> 
>> So making this method synchronized instead of startThread() will make it less likely that we will face the previous issue, but it is still timing dependent, because the call to start the launcher can return before the launcher reaches here. It will be very unlikely given the sleeps but if we want to guard against any surprises we could have a variable set in startThreads() and in finishThreads() we check and wait until that variable is set.
>
> Thank you for the concern.
> The `startThread()` below which is called from `startThreads()` has the call to `thread.ensureReady()` which waits until the target tested thread really starts and sets the `threadReady` field. So, there is no race condition as the `startThreads()` and `finishThreads()` are synchronized methods.
> 
> 
>     static private void startThread(int i) {
>         String name = "TestedThread" + i;
>         TestedThread thread = new TestedThread(name);
>         vts[i] = Thread.ofVirtual().name(name).start(thread);
>         thread.ensureReady();
>         threads[i] = thread;
>         log("# Java: started vthread: " + name);
>     }
> 
>     static synchronized private void startThreads() {
>         log("\n# Java: Starting vthreads");
>         for (int i = 0; i < VTHREADS_CNT; i++) {
>             sleep(1);
>             startThread(i);
>         }
>     }
> 
>     static private synchronized void finishThreads() {
>         try {
>             for (int i = 0; i < VTHREADS_CNT; i++) {
>                 TestedThread thread = threads[i];
>                 thread.letFinish();
>                 vts[i].join();
>             }
>         } catch (InterruptedException e) {
>             throw new RuntimeException(e);
>         }
>     }
> 
> Please, let me know if I'm missing anything.

So the race I am talking about is between the main thread running finishThreads() and the launcher thread running startThreads(). The main thread could execute finishThreads() before the launcher executes startThreads(). If you comment out the two first sleeps in run_test_cycle() you can actually see the issue. Again, given that the sleeps are there it is an unlikely scheduling, but if we want to avoid depending on timing we can add that extra synchronization.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13133#discussion_r1153872917