On Thu, 17 Nov 2022 18:41:08 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> add @requires vm.jvmti to VirtualStackTraceTest > > src/hotspot/share/prims/jvmtiExport.cpp line 390: > >> 388: ThreadInVMfromNative tiv(JavaThread::current()); >> 389: java_lang_VirtualThread::init_static_notify_jvmti_events(); >> 390: } > > Doesn't this logic mean that if the first pass through this code is made with > an unattached thread, then that will prevent subsequent passes from calling > `init_static_notify_jvmti_events`, even if the thread is attached. The reason > is because `java_lang_VirtualThread::set_notify_jvmti_events(true);` will > already have been done, so you won't pass the `if (!java_lang_VirtualThread > ::notify_jvmti_events())` check. Enabling the `notify_jvmti_events` is an optimization to avoid having this notification overhead with JVMTI virtual thread mount state transitions when it is not needed. We need to enable it only once and never disable it if enabled. The first attempt to enable it is at startup if there was any agent loaded with command line options. In such a case, the get_jvmti_interface() is called in a context of `AgentOnLoad()` in unattached thread. It only sets the `java_lang_VirtualThread::set_notify_jvmti_events(true)` The `init_static_notify_jvmti_events()` is called from the `javaClasses_init()`. The agents that are loaded into running VM are initialized with the `AgentOnAttach()`. In this case, we can't rely on the `javaClasses_init()` and so, have to explicitly call the `init_static_notify_jvmti_events()`. I feels like this can be simplified. I keep thinking about the best way to do it. Probably, the pair <set_notify_jvmti_events, init_static_notify_jvmti_events> can be replaced with just function. The problem is that we can't use the `ThreadInVMfromNative` helper for unattached thread. ------------- PR: https://git.openjdk.org/jdk/pull/11204
