On Wed, 16 Jul 2025 00:26:16 GMT, Alex Menkov <[email protected]> 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.
>
> 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)
Are you suggesting renaming the classes? This is a pretty conventional naming
when you have classes implementing a spec defined in an interface class. There
are a lot more than just StackFrame and Method that are doing this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26335#discussion_r2209018611
