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

[Andrey Turbanov]

On Wed, 19 Apr 2023 23:40:56 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

> Note this PR depends on the #13546 PR for the following:
> 
> [JDK-8306434](https://bugs.openjdk.org/browse/JDK-8306434): add support of virtual threads to JVMTI StopThread
> 
> So it can't be finalized and push until after JDK-8306434 is pushed. There will also be GHA failures until then. If JDK-8306434 results in any additional spec changes, they will likely impact this CR also.
> 
> 
> [JDK-8306034](https://bugs.openjdk.org/browse/JDK-8306034) is adding some virtual thread support to JVMTI StopThread. This will allow JDWP ThreadReference.Stop and JDI ThreadReference.stop() to have the same level support for virtual threads.
> 
> Mostly this is a spec update for JDWP and JDI, but there are also some minor changes needed to the ThreadReference.stop() handling of JDWP errors, and throwing the appropriate exceptions. Also some minor cleanup in jdb. The debug agent doesn't need changes since JVMTI errors are just passed through as the corresponding JDWP errors, and the code for this is already in place. These errors are not new nor need any special handling.
> 
> Our existing testing for ThreadReference.stop() is fairly weak:
> 
> - nsk/jdb/kill/kill001, which tests stop() when the thread is suspended at a breakpoint. It will get problem listed by [JDK-8306034](https://bugs.openjdk.org/browse/JDK-8306034). I have fixes for it already working and will push it separately.
> - nsk/jdi/stop/stop001, which is problem listed and only tests when the thread is blocked in Object.wait()
> - nsk/jdi/stop/stop002, which only tests that throwing an invalid exception fails properly
> 
> I decided to take stop002 and make it a more thorough test of ThreadReference.stop(). See the comment at the top for a list of what is tested. As for reviewing this test, it's probably best to look at it as a completely new test rather than looking at diffs since so much has been changed and added.

test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop/stop002.java line 169:

> 167:                 thrRef.stop(throwableRef);
> 168:                 log.display("TEST #2 PASSED: stop() call succeeded.");
> 169:             } catch(Exception ue) {

nit
Suggestion:

            } catch (Exception ue) {

test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop/stop002.java line 192:

> 190:                     log.display("TEST #3 PASSED: stop() call succeeded.");
> 191:                 }
> 192:             } catch(Exception ue) {

nit
Suggestion:

            } catch (Exception ue) {

test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop/stop002.java line 223:

> 221:                     log.display("TEST #4 PASSED: stop() call succeeded.");
> 222:                 }
> 223:             } catch(Throwable ue) {

Suggestion:

            } catch (Throwable ue) {

test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop/stop002.java line 255:

> 253:                     log.display("TEST #5 PASSED: stop() call for suspended thread succeeded");
> 254:                 }
> 255:             } catch(Throwable ue) {

Suggestion:

            } catch (Throwable ue) {

test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop/stop002.java line 274:

> 272:         } finally {
> 273:             // Force the debuggee out of both loops
> 274:             if (objRef != null && stopLoop1 != null && stopLoop2 !=  null) {

Suggestion:

            if (objRef != null && stopLoop1 != null && stopLoop2 != null) {

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13548#discussion_r1174633416
PR Review Comment: https://git.openjdk.org/jdk/pull/13548#discussion_r1174633435
PR Review Comment: https://git.openjdk.org/jdk/pull/13548#discussion_r1174633460
PR Review Comment: https://git.openjdk.org/jdk/pull/13548#discussion_r1174633464
PR Review Comment: https://git.openjdk.org/jdk/pull/13548#discussion_r1174633486