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

[Serguei Spitsyn]

On Thu, 30 Mar 2023 23:11:09 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> 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.

Sorry, I understood you incorrectly. You are right, there is this kind of race here.
I've rearranges this area a little bit, and hope, it is cleaner now.
Now, both `startVirtualThreads()` and `finishVirtualThreads()` are invoked on the main thread, so they do not need to be synchronized any more.  Also, the call to `ensureReady()` are moved to `finishVirtualThreads()` right before the call to `letFinish()`.

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

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