On Wed, 27 Mar 2024 05:12:53 GMT, Chris Plummer <[email protected]> wrote:
>> 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.
Thank you for the comment, Chris. You are right.
I've already figured my mistake/confusion (shared it on our team meeting) but
tested my update which has been just pushed. My analysis in the bug report is
incorrect, I'll update it soon. In fact, I don't really know what was the root
cause of this deadlock because I can't reproduce the issue. Also, it is not
easy to figure out by the code observation. The only idea I have is that it was
a spurious wakeup on the `wait()` at line 52 in the Breakpoint handler of the
native agent. I've added a couple of checks for robustness and added more
tracing to get more details if this happen again.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18419#discussion_r1540566744
