Thank you, Dan, for translating this to an issue! -JB-
On Tue, Jul 14, 2020 at 6:08 PM Daniel D. Daugherty <[email protected]> wrote: > > 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 > > <[email protected]> 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 > >>>> <[email protected]> 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 >
