On Tue, 2 May 2023 19:56:14 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> src/jdk.jdi/share/classes/com/sun/jdi/ThreadReference.java line 132: >> >>> 130: * @throws OpaqueFrameException if the thread is a suspended >>> 131: * virtual thread and the implementation was unable to throw an >>> 132: * asynchronous exception from this frame >> >> The same comment as for the jdwp spec: >> Should it be aligned with JVMTI and some other places in your fix and say >> "from the current frame" instead of "from this frame"? > > I'm waiting for your JVMTI PR to finish review. I don't want to have to > change this more than once. Okay. >> src/jdk.jdi/share/classes/com/sun/tools/jdi/ThreadReferenceImpl.java line >> 279: >> >>> 277: case JDWP.Error.OPAQUE_FRAME: >>> 278: assert isVirtual(); // can only happen with virtual >>> threads >>> 279: throw new OpaqueFrameException(); >> >> Should the OpaqueFrameException also provide a message? > > The implementation tends to only include an exception message when it helps > to clarify the reason for the exception. The spec only gives one possible > reason for this exception, so no clarification should be needed. Plus it > would be hard to meaningfully convey this reason in a short message. Here's > what the spec says: > > * @throws OpaqueFrameException if the thread is a suspended > * virtual thread and the implementation was unable to throw an > * asynchronous exception from this frame Okay, thanks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13548#discussion_r1183022704 PR Review Comment: https://git.openjdk.org/jdk/pull/13548#discussion_r1183023149
