On Thu, 30 Mar 2023 23:11:09 GMT, Patricio Chilano Mateo 
<pchilanom...@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

Reply via email to