JDK-8306034 : add support of virtual threads to JVMTI StopThread
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: jvmti
  • Affected Version: 21
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2023-04-15
  • Updated: 2023-05-17
  • Resolved: 2023-05-11
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 21
21 b23Fixed
Related Reports
CSR :  
JDK-8306434 - add support of virtual threads to JVMTI StopThread
Relates :  
JDK-8306324 - StopThread results in thread being marked as interrupted, leading to unexpected InterruptedException
Relates :  
JDK-8306471 - Add virtual threads support to JDWP ThreadReference.Stop and JDI ThreadReference.stop()
Relates :  
JDK-8307968 - serviceability/jvmti/vthread/StopThreadTest/StopThreadTest.java timed out
Relates :  
JDK-8306467 - Fix nsk/jdb/kill/kill001 to work with new JVMTI StopThread support for virtual threads.
Description
Need to implement support of virtual threds in JVMTI StopThread.
The spec update is:

diff --git a/src/hotspot/share/prims/jvmti.xml b/src/hotspot/share/prims/jvmti.xml
index 456876fbcb9..da93a5fcb68 100644
--- a/src/hotspot/share/prims/jvmti.xml
+++ b/src/hotspot/share/prims/jvmti.xml
@@ -1892,7 +1892,7 @@ jvmtiEnv *jvmti;
     <function id="StopThread" num="7">
       <synopsis>Stop Thread</synopsis>
       <description>
-        Send the specified asynchronous exception to the specified platform thread.
+        Send the specified asynchronous exception to the specified thread.
       </description>
       <origin>jvmdi</origin>
       <capabilities>
@@ -1903,8 +1903,10 @@ jvmtiEnv *jvmti;
           <jthread impl="noconvert"/>
             <description>
               The thread to stop.
-              The <code>thread</code> may not be a virtual thread. Otherwise, the error code
-              <errorlink id="JVMTI_ERROR_UNSUPPORTED_OPERATION"></errorlink> will be returned.
+              The <functionlink id="StopThread"></functionlink> function may be used to send
+              an asynchronous exception to a virtual thread when it is suspended at an event.
+              An implementation may support sending an asynchronous exception to a suspended
+              virtual thread in other cases.
              </description>
         </param>
         <param id="exception">
@@ -1915,8 +1917,12 @@ jvmtiEnv *jvmti;
         </param>
       </parameters>
       <errors>
-        <error id="JVMTI_ERROR_UNSUPPORTED_OPERATION">
-          <paramlink id="thread"/> is a virtual thread.
+        <error id="JVMTI_ERROR_THREAD_NOT_SUSPENDED">
+          Thread is a virtual thread and was not suspended and was not the current thread.
+        </error>
+        <error id="JVMTI_ERROR_OPAQUE_FRAME">
+          The thread is a suspended virtual thread and the implementation was unable
+          to throw an asynchronous exception from this frame.
         </error>
       </errors>
     </function>
@@ -11974,7 +11980,8 @@ myInit() {
       There are no Java programming language or JNI stack frames at the specified depth.
     </errorid>
     <errorid id="JVMTI_ERROR_OPAQUE_FRAME" num="32">
-      Information about the frame is not available (e.g. for native frames).
+      Information about the frame is not available (e.g. for native frames),
+      or the frame is not suitable for the requested operation.
     </errorid>
     <errorid id="JVMTI_ERROR_DUPLICATE" num="40">
       Item already set.
Comments
> As the virtualness of a thread cannot change, I think "is" is more correct here. > But there is a general inconsistency in the use of is/was that should be made consistent. Okay, thanks.
12-05-2023
Changeset: 51b8f3cf Author: Serguei Spitsyn <sspitsyn@openjdk.org> Date: 2023-05-11 17:48:39 +0000 URL: https://git.openjdk.org/jdk/commit/51b8f3cfb9df3444b6226a5d5cb7f01a9ab6db6c
11-05-2023
A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/13546 Date: 2023-04-19 22:54:35 +0000
19-04-2023
> As the virtualness of a thread cannot change, I think "is" is more correct here. > But there is a general inconsistency in the use of is/was that should be made consistent. Okay, thanks. > The JVMTI_ERROR_OPAQUE_FRAME seems to be only for handling the > "suspended at the wrong place" situation; the "not suspended" situation > would return JVMTI_ERROR_THREAD_NOT_SUSPENDED. Exactly. > I don't find "handle an async exception" to be "better defined" in this case. > If it is very strict then it should say "and was not suspended at an event". > If you want to allow for additional cases then I think "the implementation is > unable to throw an asynchronous exception from this frame" generalizes it > nicely and, as I said, uses wording consistent with other uses of this error code. David, I will create a CSR shortly. Let's move this discussion there. Your suggestion sounds good to me but we may need to polish it a little bit. > ...nicely and, as I said, uses wording consistent with other uses of this error code. The plan is to make it consistent with other cases like PopFrame, ForceEarlyReturn and SetLocal where it is possible. So that the StopThread will be some kind of example for other cases with OPAQUE_FRAME. One difference is that the StopThread spec requires suspension of target virtual threads but not platform threads.
19-04-2023
> But what about a minor correction?: "Thread is" => "Thread was" > "Thread was a virtual thread and was not suspended and was not the current thread." As the virtualness of a thread cannot change, I think "is" is more correct here. But there is a general inconsistency in the use of is/was that should be made consistent. > Thank you for the suggestion. > 1) Virtual thread has to be suspended, so I guess, your suggestion needs to be corrected accordingly. > 2) (Alan and Chris will correct me if needed.) We considered something similar but wanted it a little bit better defined. The JVMTI_ERROR_OPAQUE_FRAME seems to be only for handling the "suspended at the wrong place" situation; the "not suspended" situation would return JVMTI_ERROR_THREAD_NOT_SUSPENDED. I don't find "handle an async exception" to be "better defined" in this case. If it is very strict then it should say "and was not suspended at an event". If you want to allow for additional cases then I think "the implementation is unable to throw an asynchronous exception from this frame" generalizes it nicely and, as I said, uses wording consistent with other uses of this error code.
18-04-2023
> > + An implementation may support sending an asynchronous exception to a suspended > > + virtual thread in other cases. > > How/where does an implementation indicate that this is, or is not, the case? The goal of this statement is to relax the JCK expectations and allow support for more suspended cases beyond suspended at an event. It is possible to support all suspended+mounted cases for virtual threads but we can't say anything about "mounted" as it is an unverified implementation detail.
18-04-2023
> > + <error id="JVMTI_ERROR_OPAQUE_FRAME"> > > + Thread is a virtual thread and is not suspended at a frame that can handle an async exception. > > Given the requirement is "suspended at an event" should this not refer to "event"? > Otherwise "handle an async exception" is a bit vague - what does "handle" mean? > Given other uses of this error code I would suggest: > > "The thread is a virtual thread and the implementation is unable to throw an asynchronous exception from this frame." Thank you for the suggestion. 1) Virtual thread has to be suspended, so I guess, your suggestion needs to be corrected accordingly. 2) (Alan and Chris will correct me if needed.) We considered something similar but wanted it a little bit better defined.
18-04-2023
> Suggestion, for consistency with other parts of the spec: > "Thread is a virtual thread and was not suspended and was not the current thread." Good suggestion, thanks. But what about a minor correction?: "Thread is" => "Thread was" "Thread was a virtual thread and was not suspended and was not the current thread." > BTW Loom should probably have expanded/modified the general definition of "opaque frame": > JVMTI_ERROR_OPAQUE_FRAME (32) > Information about the frame is not available (e.g. for native frames) > > something like: > JVMTI_ERROR_OPAQUE_FRAME (32) > Information about the frame is not available (e.g. for native frames), or the frame is not suitable for the requested operation Good suggestion, thanks.
17-04-2023
> + An implementation may support sending an asynchronous exception to a suspended > + virtual thread in other cases. How/where does an implementation indicate that this is, or is not, the case? > + Thread is a virtual thread and is not the current thread or suspended. Suggestion, for consistency with other parts of the spec: "Thread is a virtual thread and was not suspended and was not the current thread." > + <error id="JVMTI_ERROR_OPAQUE_FRAME"> > + Thread is a virtual thread and is not suspended at a frame that can handle an async exception. Given the requirement is "suspended at an event" should this not refer to "event"? Otherwise "handle an async exception" is a bit vague - what does "handle" mean? Given other uses of this error code I would suggest: "The thread is a virtual thread and the implementation is unable to throw an asynchronous exception from this frame. " BTW Loom should probably have expanded/modified the general definition of "opaque frame": JVMTI_ERROR_OPAQUE_FRAME (32) Information about the frame is not available (e.g. for native frames) something like: JVMTI_ERROR_OPAQUE_FRAME (32) Information about the frame is not available (e.g. for native frames), or the frame is not suitable for the requested operation
16-04-2023