On Thu, 17 Jul 2025 08:26:54 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
> 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? I meant CSR. I'll fix that. JDWP will be updated by #26336. > 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. We are talking about the difference between "not suspended" vs "not suspended at an event". The former results in IncompatibleThreadStateException. For the latter I felt the wording implied it was suspended, but not at an event. Maybe it should just say "suspended, but not at an event"? The JVMTI spec does not have this issue because it doesn't mention virtual threads as part of the OPAQUE_ERROR description. It instead mentions the method being native as an example of what can cause OPAQUE_ERROR. We can't do that here because a native method results in NativeMethodException, not OpaqueFrameException. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26335#issuecomment-3084355508 PR Review Comment: https://git.openjdk.org/jdk/pull/26335#discussion_r2213550664
