On Wed, 27 Mar 2024 20:06:54 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review: improve diagnostics and reliability
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/PopFrameTest/PopFrameTest.java
> line 148:
>
>> 146: log("Main #B.2: got expected JVMTI_ERROR_NONE");
>> 147: resumeThread(testTaskThread);
>> 148:
>
> You probably don't need this minor edit or the copyright update any more.
>
> However, it's still unclear to me how the ensureAtBreakpoint() below is
> suppose to work if we already called notifyAtBreakpoint() between the 1st and
> 2nd ensureAtBreakpoint(). From what I can tell, notifyAtBreakpoint() clears
> the flag that ensureAtBreakpoint() checks for, and there is no 2nd breakpoint
> to set it again. I would expect the ensureAtBreakpoint() below to always wait
> indefinitely, but that doesn't appear to be the case. So how is this working?
> I must be missing something.
Thanks, Chris. I noticed that some an additional cleanup is needed in the Java
part.
The `TestTask.run()` has two calls to the method `B()`:
public void run() {
log("TestTask.run: started");
A();
sleep(1); // to cause yield
prepareAgent(TestTask.class, false); // No doPopFrame
B();
sleep(1); // to cause yield
prepareAgent(TestTask.class, true); // doPopFrame
B();
sleep(1); // to cause yield
C();
finished = true;
}
The second `B()` call does the JVMTI `PopFrame` call on its own thread in the
`Breakpoint()` event hadler.
Please, note, `true` in the `prepareAgent` parameters:
`prepareAgent(TestTask.class, true); // doPopFrame`
So, the second `ensureAtBreakpoint()` is a sync point with it.
> test/hotspot/jtreg/serviceability/jvmti/vthread/PopFrameTest/libPopFrameTest.cpp
> line 58:
>
>> 56: LOG("Breakpoint: In method TestTask.B(): after sync section\n");
>> 57:
>> 58: if (do_pop_frame != JNI_FALSE) {
>
> Suggestion:
>
> if (do_pop_frame) {
Okay, thanks. I can do this change.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18419#discussion_r1543825742
PR Review Comment: https://git.openjdk.org/jdk/pull/18419#discussion_r1543826541
