On Thu, 30 Mar 2023 19:00:03 GMT, Patricio Chilano Mateo
<[email protected]> 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
