On Tue, 16 May 2023 22:02:45 GMT, Chris Plummer <cjplum...@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
