On Thu, 23 Jun 2022 16:13:58 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> 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 incrementally with one > additional commit since the last revision: > > 8286580 review: remove unneeded sentence in the comment Marked as reviewed by cjplummer (Reviewer). ------------- PR: https://git.openjdk.org/jdk19/pull/42
