RFR: 8296324: JVMTI GetStackTrace truncates vthread stack trace for agents loaded into running VM [v2]

[Serguei Spitsyn]

On Fri, 18 Nov 2022 06:15:39 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> If there are no command line agents, then on startup `vthread_notify_jvmti_events` is not set true. Because it is not true, when `javaClasses_init()` calls `init_static_notify_jvmti_events()`, it does nothing. The whole point of the code we are reviewing here is to make sure `init_static_notify_jvmti_events()` is called while `vthread_notify_jvmti_events == true` so it actually does something. However, the code here does not bother calling `init_static_notify_jvmti_events()` if the current thread is detached, but it does still set `vthread_notify_jvmti_events = true`. This means that if this code gets called a second time, this time with the current thread attached, it will not call `init_static_notify_jvmti_events()` due to `vthread_notify_jvmti_events == true`, but it seems it should be calling it.
>> 
>> What I believe to be the flaw here is that you call `set_notify_jvmti_events(true)` even if you don't call `init_static_notify_jvmti_events()`.
>
>> What I believe to be the flaw here is that you call `set_notify_jvmti_events(true)`
>> even if you don't call `init_static_notify_jvmti_events()`.
> 
> This only happen in a detached thread case which can be only at startup.
> It was implemented this way before my fix.
> The javaClasses has to be partially initialized before any call to `init_static_notify_jvmti_events()`:
> 
> void javaClasses_init() {
>   JavaClasses::compute_offsets();               <== This is needed
>   JavaClasses::check_offsets();
>   java_lang_VirtualThread::init_static_notify_jvmti_events();
>   FilteredFieldsMap::initialize();  // must be done after computing offsets.
> }
> 
> 
> For detached thread case (which is at starup) it is guaranteed that the `init_static_notify_jvmti_events()` is unconditionally called later at initialization stage. Please, see the chain of calls:
> `create_vm()  => init_globals() => javaClasses_init() => java_lang_VirtualThread::init_static_notify_jvmti_events()`.
> 
> There have to be no cases with a call to `set_notify_jvmti_events(true)` without a following call to `init_static_notify_jvmti_events()`:
> - detached thread case: the `init_static_notify_jvmti_events()` is called later at startup sequence
> - JavaThread case: the `init_static_notify_jvmti_events()` is always called explicitly after the call to `set_notify_jvmti_events(true)`
> 
> You can think about the agents which do not call `GetEnv()` in`AgentOnLoad/AgentOnAttach` or have no `AgentOnLoad/AgentOnAttach` entry points. Such agents can later call `GetEnv()` from a detached thread.
> Is it your concern?

I've pushed an update.
Please, let me know of you are okay with that.

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

PR: https://git.openjdk.org/jdk/pull/11204