Some clarification about JVMTI_EVENT_THREAD_START and PRIMORDIAL phase

[Daniel D. Daugherty]

If there is going to be a spec change, then it would not affect JDK8.
I don't think anyone has decided how this issue should be resolved yet.

Dan


On 7/13/20 7:10 AM, Sergey Nazarkin wrote:
> Hi Daniel,
>
> Please correct me if I‘m wrong but the spec change will touch post-JDK8 releases only?
>
>
>
> Sergey Nazarkin
>
>
>
>
>> On Jul 9, 2020, at 19:25, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:
>>
>> On 7/9/20 9:07 AM, Jaroslav Bachorík wrote:
>>> Hello,
>>>
>>> Recently, after backporting JDK-8233197 to JDK8u I received a report
>>> from Sergey Nazarkin (cc) that the backport broke JVMTI spec basically
>>> emitting JVMTI_EVENT_THREAD_START events in primordial phase while the
>>> spec states that it should be emitted only in either start or live
>>> phase.
>>>
>>> But if I am reading this correctly
>>> (https://hg.openjdk.java.net/jdk8u/jdk8udev/hotspot/file/02b4fd2f9041/src/share/vm/prims/jvmtiEventController.cpp#l418)
>>> the spec is also violated there.
>>>
>>> EARLY_EVENT_BITS are including THREAD_START_BIT and it is not filtered
>>> out by THREAD_FILTERED_EVENT_BITS so it seems perfectly valid for
>>> JVMTI_EVENT_THREAD_START to appear during PRIMORDIAL phase.
>>>
>>> So I would like to get clarification before going in and start
>>> tinkering with the JVM initialization code order.
>>>
>>> Thanks!
>>>
>>> -JB-
>> Hi Jaroslav!
>>
>> It has been a long time my friend... :-)
>>
>> It looks like that change was made by this Jigsaw change:
>>
>>      JDK-8072745 Re-examine JVM TI execution phases and expectations
>>                  on what is allowed in each phase
>>      https://bugs.openjdk.java.net/browse/JDK-8072745
>>
>> These diffs in the bug reported jumped out at me:
>>
>> @@ -1022,6 +1032,10 @@
>>   void JvmtiExport::post_thread_start(JavaThread *thread) {
>>     assert(thread->thread_state() == _thread_in_vm, "must be in vm state");
>>
>> + if (JvmtiEnv::get_phase() <= JVMTI_PHASE_PRIMORDIAL) {
>> + return;
>> + }
>> +
>>     EVT_TRIG_TRACE(JVMTI_EVENT_THREAD_START, ("JVMTI [%s] Trg Thread Start event triggered",
>>                         JvmtiTrace::safe_get_thread_name(thread)));
>>
>> @@ -1050,6 +1064,10 @@
>>
>>
>>   void JvmtiExport::post_thread_end(JavaThread *thread) {
>> + if (JvmtiEnv::get_phase() <= JVMTI_PHASE_PRIMORDIAL) {
>> + return;
>> + }
>> +
>>     EVT_TRIG_TRACE(JVMTI_EVENT_THREAD_END, ("JVMTI [%s] Trg Thread End event triggered",
>>                         JvmtiTrace::safe_get_thread_name(thread)));
>>
>>
>> Here's the order of phases from jvmti.h:
>>
>>      /* Phases of execution */
>>
>> typedef enum {
>>      JVMTI_PHASE_ONLOAD = 1,
>>      JVMTI_PHASE_PRIMORDIAL = 2,
>>      JVMTI_PHASE_START = 6,
>>      JVMTI_PHASE_LIVE = 4,
>>      JVMTI_PHASE_DEAD = 8
>> } jvmtiPhase;
>>
>> However, the above diffs prevent the JVMTI_EVENT_THREAD_START
>> and JVMTI_EVENT_THREAD_END events from being posted earlier than
>> the JVMTI_PHASE_START phase... Unfortunately, I can't find the
>> actual changeset pushed to the Jigsaw report for JDK-8072745.
>>
>>
>> When you look at the current JvmtiExport::post_thread_start()
>> and JvmtiExport::post_thread_end() functions, you can see this:
>>
>>      1: void JvmtiExport::post_thread_start(JavaThread *thread) {
>> 36508:   if (JvmtiEnv::get_phase() < JVMTI_PHASE_PRIMORDIAL) {
>> 36508:     return;
>> 36508:   }
>>
>>      1: void JvmtiExport::post_thread_end(JavaThread *thread) {
>> 36508:   if (JvmtiEnv::get_phase() < JVMTI_PHASE_PRIMORDIAL) {
>> 36508:     return;
>> 36508:   }
>>
>> $ hg log -r 36508
>> changeset:   36508:5f9eee6b383b
>> user:        alanb
>> date:        Thu Mar 17 19:04:01 2016 +0000
>> summary:     8142968: Module System implementation
>>
>>
>> So the Jigsaw integration pushed a slightly different change
>> than is shown in JDK-8072745. I don't know whether the fix
>> for JDK-8072745 evolved before being pushed to the Jigsaw
>> repos or whether another JBS issue was used to modify the
>> code after JDK-8072745 was pushed.
>>
>> Short version: The semantics of the JVMTI_EVENT_THREAD_START
>> and JVMTI_EVENT_THREAD_END were changed by Jigsaw, but the
>> JVM/TI spec was not updated to reflect those changes.
>>
>> I've filed a new bug to track this issue:
>>
>>      JDK-8249158 THREAD_START and THREAD_END event posting changed
>>                  by modules without updating the spec
>>      https://bugs.openjdk.java.net/browse/JDK-8249158
>>
>> Dan