On Wed, 19 Aug 2026 23:28:00 GMT, Patricio Chilano Mateo
<[email protected]> wrote:
>> Chris Plummer has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> verify we got OpaqueFrameException when no exception thrown on debuggee
>> side
>
> test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop/stop002.java line
> 234:
>
>> 232: thrRef.stop(throwableRef);
>> 233: log.display("TEST #4 PASSED: stop() call succeeded.");
>> 234: objRef.setValue(gotOpaqueFrameException,
>> vm.mirrorOf(false));
>
> Isn't the field already initialized to false?
I wanted to be explicit and not rely on default initialization.
> test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop/stop002.java line
> 248:
>
>> 246: log.complain("TEST #4 FAILED: caught unexpected " + ue);
>> 247: tot_res = Consts.TEST_FAILED;
>> 248: objRef.setValue(gotOpaqueFrameException,
>> vm.mirrorOf(false));
>
> Also don't understand why this is needed.
Once again, just being explicit.
> test/hotspot/jtreg/vmTestbase/nsk/jdi/ThreadReference/stop/stop002t.java line
> 151:
>
>> 149: } else {
>> 150: log.complain("TEST #4: Failed to throw expected
>> exception and " +
>> 151: "failed to throw debugger side
>> OpaqueFrameException");
>
> We should have a `return Consts.TEST_FAILED;` for this case too. Also, I
> think the output msg is confusing because if the frame is interpreted there
> is no `OpaqueFrameException` that should be expected. I would just remove
> this `else` branch and change the condition in the original `if` to
> `vthreadMode && gotOpaqueFrameException`.
I'll fix it to return Consts.TEST_FAILED. I think the message is correct and
provides the necessary added detail that both no exception was thrown and there
was no OpaqueFrameException on the debugger side. That gets lost if we combine
this with the non-vthread case.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32403#discussion_r3817640396
PR Review Comment: https://git.openjdk.org/jdk/pull/32403#discussion_r3817641908
PR Review Comment: https://git.openjdk.org/jdk/pull/32403#discussion_r3817728680
