RFR: 8308237: add JDWP and JDI virtual thread support for ThreadReference.PopFrames

Alan Bateman alanb at openjdk.org
Wed May 17 08:35:45 UTC 2023


On Tue, 16 May 2023 22:02:45 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

> This is a follow-on to [JDK-8264699](https://bugs.openjdk.org/browse/JDK-8264699) which adds JVMTI PopFrames support for virtual thread. For JDWP and JDI this is mostly a spec update, although JDI needs minor changes to properly throw the correct exception. Note this PR needs JDK-8264699 in order to function properly, so there may be some GHA failures until JDK-8264699 is pushed.
> 
> There are a large number of tests that can now be removed from the problem list. Also, one test needs to be modified to no longer expect OpaqueFrameException for virtual threads. It was just revereted back to it's previous form before the OpaqueFrameException support was added for virtual threads.
> 
> As you can see from the problemlist update, there are quite a few tests for popFrames() support. However, there are still two coverage gaps:
> 
> - There is no test for throwing NativeMethodException (even for platform threads)
> - There is no test case for throwing OpaqueFrameException when the virtual thread is suspended but not mounted.
> 
> I may eventually add one or both tests to the PR, or I may just file separate CRs for them for now.

src/jdk.jdi/share/classes/com/sun/jdi/ThreadReference.java line 380:

> 378:      * This method may be used to pop frames of a virtual thread when
> 379:      * it is suspended at an event. An implementation may support popping
> 380:      * the frames of a suspended virtual thread in other cases.

This text looks okay but I notice a bit further up that it says "The specified thread must be suspend.". I assume that should be "This thread must be suspended."

src/jdk.jdi/share/classes/com/sun/tools/jdi/StackFrameImpl.java line 412:

> 410:                     } else {
> 411:                         throw new OpaqueFrameException();
> 412:                     }

The code to select NME vs. OFE looks right.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14022#discussion_r1196139627
PR Review Comment: https://git.openjdk.org/jdk/pull/14022#discussion_r1196141721


More information about the serviceability-dev mailing list