On Tue, 15 Jul 2025 22:07:40 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.
It looks good.
I added cleanup suggestion in the implementation
src/jdk.jdi/share/classes/com/sun/tools/jdi/StackFrameImpl.java line 401:
> 399: // previous frame is native, in which case we throw
> NativeMethodException
> 400: for (int i = 0; i < 2; i++) {
> 401: StackFrameImpl sf;
There is nothing implementation-specific here.
I'd suggest to:
- `StackFrameImpl` -> `StackFrame`;
- `MethodImpl` -> `Method`;
- remove `validateStackFrame` at line 408 ('MethodImpl.location()' calls it)
-------------
PR Comment: https://git.openjdk.org/jdk/pull/26335#issuecomment-3076343535
PR Review Comment: https://git.openjdk.org/jdk/pull/26335#discussion_r2208948088
