On Tue, 16 May 2023 22:04:26 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> This enhancement adds `PopFrame` support for virtual threads. The spec >> defines minimal support that the JVMTI `PopFrame` can be used for a virtual >> thread suspended an an event. >> Actually, the `PopFrame` can supports suspended and mounted virtual threads. >> >> CSR (approved): https://bugs.openjdk.org/browse/JDK-8308001: add PopFrame >> support for virtual threads >> >> Testing: >> New test was developed: `serviceability/vthread/PopFrameTest`. >> Submitted mach5 tiers 1-6 are good. >> TBD: rerun mach5 tiers 1-6 at the end of review again if necessary. > > src/hotspot/share/prims/jvmtiEnv.cpp line 1886: > >> 1884: jvmtiError err = get_threadOop_and_JavaThread(tlh.list(), thread, >> &java_thread, &thread_obj); >> 1885: >> 1886: bool is_virtual = thread_obj != nullptr && >> thread_obj->is_a(vmClasses::BaseVirtualThread_klass()); > > I think it would be better to check 'err' and try to handle the error before > using java_thread and thread_obj. Thanks for the comment. It impacts the error reporting precedence which can break expectations from some tests. But I agree, it look more natural here to check the error first. Updated. Let's see if the all test runs are still okay. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14002#discussion_r1196147657
