Some clarification about JVMTI_EVENT_THREAD_START and PRIMORDIAL phase

[Daniel D. Daugherty]

Jarsolav,

I'm copying this reply to the bug (JDK-8249158). I figure folks should
continue the conversation there.

Dan


On 7/14/20 9:31 AM, Jaroslav Bachorík wrote:
> Thank you all for chiming in!
>
> My main concern was that the JDK 8 JVMTI spec
> (https://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#ThreadStart)
> was contradicting what was actually put to the EARLY_EVENT_BITS
> variable and therefore which events were allowed in PRIMORDIAL and
> ONLOAD (https://hg.openjdk.java.net/jdk8u/jdk8u-dev/hotspot/file/02b4fd2f9041/src/share/vm/prims/jvmtiEventController.cpp#l100)
>
> So, if I went and remove `THREAD_START_BIT | THREAD_END_BIT` part from
> the EARLY_EVENT_BITS in order to fix the assumed regression due to
> JDK-8233197 I would be actually following the spec, right?
>
> Cheers,
>
> -JB-
>
> On Mon, Jul 13, 2020 at 4:02 PM Daniel D. Daugherty
> <daniel.daugherty at oracle.com> wrote:
>> 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