> The JVMTI SetLocalXXX requires the target virtual thread suspended at a > breakpoint or single step event. > > This is the relevant statement from the "Local Variable" section: > > `The SetLocalXXX functions may be used to set the value of a local variable > in the topmost frame of a virtual thread suspended at a breakpoint or single > step event. > ` > This fix is to return the JVMTI_ERROR_OPAQUE_FRAME in cases when the target > thread is not at a breakpoint or single step event. In this case the assert > described in the bug report is avoided: > > > open/src/hotspot/share/runtime/vframe.cpp:300 > # assert(stack_chunk() == __null) failed: Not supported for heap frames > > Also, this is an analysis from Ron: > > The problem occurs because the thread is suspended at a safepoint poll on > return in the oldest thawed frame. The safepoint happens after the frame is > popped but before it returns to the return barrier, thawing the caller (and > so the stack looks as if we're in enterSpecial). In that case the virtual > thread's topmost frame is still frozen on the heap, even though it is mounted. > > However, the spec says that a set local operation should succeed for the > topmost frame of a mounted virtual thread only if the thread is suspended *at > a breakpoint or a single-step event*, and I don't think we can stop at that > safepoint in that case. > > If so, the fix is simple: if we're trying to set, even if the virtual thread > is mounted and the depth is zero, if the frame is a heap frame, we should > return an OPAQUE_FRAME error. The test should be changed as well to accept > such an error if the thread is suspended not at a breakpoint/single-step. > > > Changes `GetSetLocalTest` test are taken from the repo-loom repository. It > was an update from Leonid which is needed to reproduce this problem. > > The fix was tested with thousands of runs of the `GetSetLocalTest` on Linux, > Windows and MacOs. > > Will also submit nsk.jvmti and nsk.jdi test runs on 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 four additional commits since the last revision: - resolved 8286580 review comments; problem-listed test GetSetLocalTest under 8286836 - Merge branch 'master' into br2 - remove _depth == 0 clause from sanity check assert - 8286580: serviceability/jvmti/vthread/GetSetLocalTest failed with assert: Not supported for heap frames ------------- Changes: - all: https://git.openjdk.org/jdk19/pull/42/files - new: https://git.openjdk.org/jdk19/pull/42/files/bd1a901c..ca7c3b3a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk19&pr=42&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk19&pr=42&range=01-02 Stats: 845 lines in 50 files changed: 524 ins; 86 del; 235 mod Patch: https://git.openjdk.org/jdk19/pull/42.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/42/head:pull/42 PR: https://git.openjdk.org/jdk19/pull/42
