RFR: 8306471: Add virtual threads support to JDWP ThreadReference.Stop and JDI ThreadReference.stop() [v4]

[Serguei Spitsyn]

On Tue, 2 May 2023 19:56:14 GMT, Chris Plummer <cjplummer at 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