On Wed, 16 Jul 2025 03:32:34 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Fix how ThreadReference.popFrame() and ThreadReference.forceEarlyReturn deal >> with JDWP OPAQUE_FRAME error. >> >> Before virtual threads, OpaqueFrameException did not exist and these API >> always threw NativeMethodException when JDWP OPAQUE_FRAME error was >> returned. For virtual threads OpaqueFrameException was added to handle the >> case where a virtual thread was not suspended at an event, so the JDI >> implementation was updated to throw OpaqueFrameException if it detected that >> a native method was not the cause. It turns out however that JVMTI (and >> therefore JDWP) can return OPAQUE_FRAME error for reasons other than a >> native method or the special virtual thread case, and for platform threads >> we were incorrectly throwing NativeMethodException in these cases. This PR >> fixes that. For platform threads we now only throw NativeMethodException if >> a native method is detected, and otherwise throw OpaqueFrameException. >> >> The spec language is also being cleaned up to better align with JVMTI. >> Rather than calling out all the reasons for OpaqueFrameException, a more >> generic explanation is given. >> >> This is somewhat of a preliminary PR so I can get some feedback. I still >> need to do a CR and complete testing. > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > update based on feedback I've posted one concern on the spec updates. Otherwise, it looks okay to me. I guess, a CSR will be filed for this update. I'm not sure what do you mean by CR in the description. Do I understand this right that a potential JDWP spec update we touched out of this PR will be considered separately? src/jdk.jdi/share/classes/com/sun/jdi/ThreadReference.java line 406: > 404: * > 405: * @throws OpaqueFrameException if target VM is unable to pop this > frame > 406: * (e.g. a virtual thread is not suspended at an event). Nit: I'm not sure the comment at line 406 is fully correct. The `IncompatibleThreadStateException` will be thrown if a virtual thread is not suspended. So, the `OpaqueFrameException` will be thrown only when a virtual thread is suspended but not at an event. The same concern should apply for the line 488. ------------- Marked as reviewed by sspitsyn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/26335#pullrequestreview-3028456818 PR Review Comment: https://git.openjdk.org/jdk/pull/26335#discussion_r2212652535