4 files changed, 29 insertions, 16 deletions

diff --git a/openjdkjvmti/generate-operator-out.py b/openjdkjvmti/generate-operator-out.py
new file mode 120000
index 0000000000..cc291d20c1
--- /dev/null
+++ b/openjdkjvmti/generate-operator-out.py
@@ -0,0 +1 @@
+../tools/generate-operator-out.py
\ No newline at end of file
diff --git a/openjdkjvmti/ti_phase.cc b/openjdkjvmti/ti_phase.cc
index e8c1ca7335..07cf31c354 100644
--- a/openjdkjvmti/ti_phase.cc
+++ b/openjdkjvmti/ti_phase.cc
@@ -72,9 +72,16 @@ struct PhaseUtil::PhaseCallback : public art::RuntimePhaseCallback {
         {
           ThreadUtil::CacheData();
           PhaseUtil::current_phase_ = JVMTI_PHASE_LIVE;
-          ScopedLocalRef<jthread> thread(GetJniEnv(), GetCurrentJThread());
-          art::ScopedThreadSuspension sts(art::Thread::Current(), art::ThreadState::kNative);
-          event_handler->DispatchEvent<ArtJvmtiEvent::kVmInit>(nullptr, GetJniEnv(), thread.get());
+          {
+            ScopedLocalRef<jthread> thread(GetJniEnv(), GetCurrentJThread());
+            art::ScopedThreadSuspension sts(art::Thread::Current(), art::ThreadState::kNative);
+            event_handler->DispatchEvent<ArtJvmtiEvent::kVmInit>(
+                nullptr, GetJniEnv(), thread.get());
+          }
+          // We need to have these events be ordered to match behavior expected by some real-world
+          // agents. The spec does not really require this but compatibility is a useful property to
+          // maintain.
+          ThreadUtil::VMInitEventSent();
         }
         break;
       case RuntimePhase::kDeath:
diff --git a/openjdkjvmti/ti_thread.cc b/openjdkjvmti/ti_thread.cc
index 6fa73f8a8c..b0a1a8556a 100644
--- a/openjdkjvmti/ti_thread.cc
+++ b/openjdkjvmti/ti_thread.cc
@@ -57,13 +57,14 @@ namespace openjdkjvmti {
 
 art::ArtField* ThreadUtil::context_class_loader_ = nullptr;
 
-struct ThreadCallback : public art::ThreadLifecycleCallback, public art::RuntimePhaseCallback {
+struct ThreadCallback : public art::ThreadLifecycleCallback {
   jthread GetThreadObject(art::Thread* self) REQUIRES_SHARED(art::Locks::mutator_lock_) {
     if (self->GetPeer() == nullptr) {
       return nullptr;
     }
     return self->GetJniEnv()->AddLocalReference<jthread>(self->GetPeer());
   }
+
   template <ArtJvmtiEvent kEvent>
   void Post(art::Thread* self) REQUIRES_SHARED(art::Locks::mutator_lock_) {
     DCHECK_EQ(self, art::Thread::Current());
@@ -96,15 +97,6 @@ struct ThreadCallback : public art::ThreadLifecycleCallback, public art::Runtime
     Post<ArtJvmtiEvent::kThreadEnd>(self);
   }
 
-  void NextRuntimePhase(RuntimePhase phase) OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_) {
-    if (phase == RuntimePhase::kInit) {
-      // We moved to VMInit. Report the main thread as started (it was attached early, and must
-      // not be reported until Init.
-      started = true;
-      Post<ArtJvmtiEvent::kThreadStart>(art::Thread::Current());
-    }
-  }
-
   EventHandler* event_handler = nullptr;
   bool started = false;
 };
@@ -121,10 +113,19 @@ void ThreadUtil::Register(EventHandler* handler) {
                                     art::ThreadState::kWaitingForDebuggerToAttach);
   art::ScopedSuspendAll ssa("Add thread callback");
   runtime->GetRuntimeCallbacks()->AddThreadLifecycleCallback(&gThreadCallback);
-  runtime->GetRuntimeCallbacks()->AddRuntimePhaseCallback(&gThreadCallback);
+}
+
+void ThreadUtil::VMInitEventSent() {
+  // We should have already started.
+  DCHECK(gThreadCallback.started);
+  // We moved to VMInit. Report the main thread as started (it was attached early, and must not be
+  // reported until Init.
+  gThreadCallback.Post<ArtJvmtiEvent::kThreadStart>(art::Thread::Current());
 }
 
 void ThreadUtil::CacheData() {
+  // We must have started since it is now safe to cache our data;
+  gThreadCallback.started = true;
   art::ScopedObjectAccess soa(art::Thread::Current());
   art::ObjPtr<art::mirror::Class> thread_class =
       soa.Decode<art::mirror::Class>(art::WellKnownClasses::java_lang_Thread);
@@ -140,7 +141,6 @@ void ThreadUtil::Unregister() {
   art::ScopedSuspendAll ssa("Remove thread callback");
   art::Runtime* runtime = art::Runtime::Current();
   runtime->GetRuntimeCallbacks()->RemoveThreadLifecycleCallback(&gThreadCallback);
-  runtime->GetRuntimeCallbacks()->RemoveRuntimePhaseCallback(&gThreadCallback);
 }
 
 jvmtiError ThreadUtil::GetCurrentThread(jvmtiEnv* env ATTRIBUTE_UNUSED, jthread* thread_ptr) {
diff --git a/openjdkjvmti/ti_thread.h b/openjdkjvmti/ti_thread.h
index 03c49d7788..a19974aa16 100644
--- a/openjdkjvmti/ti_thread.h
+++ b/openjdkjvmti/ti_thread.h
@@ -53,9 +53,14 @@ class ThreadUtil {
   static void Register(EventHandler* event_handler);
   static void Unregister();
 
-  // To be called when it is safe to cache data.
+  // To be called when it is safe to cache data. This means that we have at least entered the
+  // RuntimePhase::kInit but we might or might not have already called VMInit event.
   static void CacheData();
 
+  // Called just after we have sent the VMInit callback so that ThreadUtil can do final setup. This
+  // ensures that there are no timing issues between the two callbacks.
+  static void VMInitEventSent() REQUIRES_SHARED(art::Locks::mutator_lock_);
+
   // Handle a jvmtiEnv going away.
   static void RemoveEnvironment(jvmtiEnv* env);