On Wed, 22 Jun 2022 21:23:28 GMT, Serguei Spitsyn <[email protected]> wrote:
>> src/hotspot/share/prims/jvmtiImpl.cpp line 647:
>>
>>> 645: }
>>> 646: if (_set) {
>>> 647: if (fr.is_heap_frame()) { // we want this check after the check
>>> for JVMTI_ERROR_INVALID_SLOT
>>
>> It would be good to elaborate a bit more on this check. Say something about
>> it being a "safepoint poll on return in the oldest thawed frame", and why
>> that is a problem.
>
> Okay. I'd suggest this:
>
> if (_set) {
> if (fr.is_heap_frame()) { // we want this check after the check for
> JVMTI_ERROR_INVALID_SLOT
> assert(Continuation::is_frame_in_continuation(_jvf->thread(), fr),
> "sanity check");
> // The topmost frame of a mounted continuation can still be in the
> heap. This code is executed at safepoint.
> // The safepoint could be as we return to the return barrier but before
> we execute it (poll return).
> _result = JVMTI_ERROR_OPAQUE_FRAME; // deferred locals currently
> unsupported in continuations
> return;
> }
>
> Is it more clear?
I guess what I'm not understanding here is how/why "fr.is_heap_frame())"
translates into "we are not single stepping or at a breakpoint". I know we
already did the check earlier to make sure we are "depth == 0", and I
understand that in this code we are handling the special case of "depth == 0"
possibly not indicating we are truly in the topmost frame. From Ron's
explanation, the frame has been popped, but the callee frame has not been
thawed yet. Is the "fr.is_heap_frame())" check telling us the frame hasn't been
thawed. If so, I would call this out in the comment. Basically say something
like:
"If the topmost frame is a heap frame, then it hasn't been thawed. This can
happen if we are executing at a return barrier safepoint. The callee frame has
been popped, but the caller frame has not been thawed. We can't support a
setlocal in the callee frame at this point, because we aren't truly in the
callee yet."
Also, I think the following comment is misleading:
`deferred locals currently unsupported in continuations`
That's not true. They are supported when single stepping and at breakpoints, or
more accurately, in the topmost frame. Looks like this is copy-n-paste from the
following code with the same issue:
613 if (_set && _depth != 0 &&
Continuation::is_frame_in_continuation(_jvf->thread(), fr)) {
614 _result = JVMTI_ERROR_OPAQUE_FRAME; // deferred locals currently
unsupported in continuations
615 return;
616 }
-------------
PR: https://git.openjdk.org/jdk19/pull/42
