On Wed, 27 Mar 2024 02:57:40 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> This PR fixes a synchronization issue in the test:
>>   `test/hotspot/jtreg/serviceability/jvmti/vthread/PopFrameTest`
>>   
>> The method `notifyAtBreakpoint()` can notify the `TestTask` thread when it 
>> has not reached an expected breakpoint yet.
>> The fix is to add a call to the method `ensureAtBreakpoint()` one more time 
>> in the `B2` sub-test. It is needed after the top-most frame was popped with 
>> the JVMTI `PopFrame`, and the target thread needs to reach the breakpoint 
>> again after its execution was resumed.
>> 
>> The time is very intermittent. At least, I was not able to reproduce the 
>> timeout failure in thousands of mach5 runs with the `-Xcomp` option.
>> 
>> Testing:
>>  - Run the test 
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/PopFrameTest` thousands 
>> times in mach5
>
> Serguei Spitsyn has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - “Merge”
>  - review: updated test with one more call to notifyAtBreakpoint to reset the 
> native state
>  - 8328665: serviceability/jvmti/vthread/PopFrameTest failed with a timeout

test/hotspot/jtreg/serviceability/jvmti/vthread/PopFrameTest/PopFrameTest.java 
line 147:

> 145:             }
> 146:             log("Main #B.2: got expected JVMTI_ERROR_NONE");
> 147:             notifyAtBreakpoint(); // needed to reset the native state

This doesn't make much sense to me.  The purpose of `notifyAtBreakpoint()` is 
to unblock the Breakpoint event handler, which is waiting for this notify. 
However, the event handler can only ever be entered once, yet we have a 
previous `noitfyBreakpoint()` above and a 3rd `notifyAtBreakpoint()` below. It 
seems that this call and the one below are no-ops. I don't see how 
bp_sync_reached ever gets set true again after the first `notifyAtBreakpoint()` 
call is made.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18419#discussion_r1540476191

Reply via email to