On Thu, 30 Mar 2023 19:00:03 GMT, Patricio Chilano Mateo 
<pchilanom...@openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   review: tweak in count_transitions_and_correct_jvmti_thread_states
>
> 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.

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

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

Reply via email to