diff -r 4c2557ab304e -r 127ca611f19b src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp
--- a/src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp	Mon Nov 25 15:21:44 2019 +0000
+++ b/src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp	Mon Nov 25 18:38:01 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -32,7 +32,9 @@
 #include "jfr/support/jfrEventClass.hpp"
 #include "logging/log.hpp"
 #include "memory/resourceArea.hpp"
+#include "prims/jvmtiEnvBase.hpp"
 #include "prims/jvmtiExport.hpp"
+#include "prims/jvmtiUtil.hpp"
 #include "runtime/interfaceSupport.inline.hpp"
 #include "runtime/thread.inline.hpp"
 #include "utilities/exceptions.hpp"
@@ -52,19 +54,17 @@
   }
 }
 
-static jvmtiError set_event_notification_mode(jvmtiEventMode mode,
-                                              jvmtiEvent event,
-                                              jthread event_thread,
-                                              ...) {
-  if (jfr_jvmti_env == NULL) {
-    return JVMTI_ERROR_NONE;
-  }
+static bool set_event_notification_mode(jvmtiEventMode mode,
+                                        jvmtiEvent event,
+                                        jthread event_thread,
+                                        ...) {
+  assert(jfr_jvmti_env != NULL, "invariant");
   const jvmtiError jvmti_ret_code = jfr_jvmti_env->SetEventNotificationMode(mode, event, event_thread);
   check_jvmti_error(jfr_jvmti_env, jvmti_ret_code, "SetEventNotificationMode");
-  return jvmti_ret_code;
+  return jvmti_ret_code == JVMTI_ERROR_NONE;
 }
 
-static jvmtiError update_class_file_load_hook_event(jvmtiEventMode mode) {
+static bool update_class_file_load_hook_event(jvmtiEventMode mode) {
   return set_event_notification_mode(mode, JVMTI_EVENT_CLASS_FILE_LOAD_HOOK, NULL);
 }
 
@@ -117,12 +117,23 @@
   return classes;
 }
 
-static void log_and_throw(TRAPS) {
+// caller needs ResourceMark
+static void log_and_throw(jvmtiError error, TRAPS) {
   if (!HAS_PENDING_EXCEPTION) {
     DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(THREAD));
     ThreadInVMfromNative tvmfn((JavaThread*)THREAD);
-    log_error(jfr, system)("JfrJvmtiAgent::retransformClasses failed");
-    JfrJavaSupport::throw_class_format_error("JfrJvmtiAgent::retransformClasses failed", THREAD);
+    const char base_error_msg[] = "JfrJvmtiAgent::retransformClasses failed: ";
+    size_t length = sizeof base_error_msg; // includes terminating null
+    const char* const jvmti_error_name = JvmtiUtil::error_name(error);
+    assert(jvmti_error_name != NULL, "invariant");
+    length += strlen(jvmti_error_name);
+    char* error_msg = NEW_RESOURCE_ARRAY(char, length);
+    jio_snprintf(error_msg, length, "%s%s", base_error_msg, jvmti_error_name);
+    if (JVMTI_ERROR_INVALID_CLASS_FORMAT == error) {
+      JfrJavaSupport::throw_class_format_error(error_msg, THREAD);
+    } else {
+      JfrJavaSupport::throw_runtime_exception(error_msg, THREAD);
+    }
   }
 }
 
@@ -137,12 +148,15 @@
   }
 }
 
+static bool is_valid_jvmti_phase() {
+  return JvmtiEnvBase::get_phase() == JVMTI_PHASE_LIVE;
+}
+
 void JfrJvmtiAgent::retransform_classes(JNIEnv* env, jobjectArray classes_array, TRAPS) {
   assert(env != NULL, "invariant");
+  assert(classes_array != NULL, "invariant");
+  assert(is_valid_jvmti_phase(), "invariant");
   DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(THREAD));
-  if (classes_array == NULL) {
-    return;
-  }
   const jint classes_count = env->GetArrayLength(classes_array);
   if (classes_count <= 0) {
     return;
@@ -153,27 +167,27 @@
   for (jint i = 0; i < classes_count; i++) {
     jclass clz = (jclass)env->GetObjectArrayElement(classes_array, i);
     check_exception_and_log(env, THREAD);
-
+    classes[i] = clz;
+  }
+  {
     // inspecting the oop/klass requires a thread transition
-    {
-      ThreadInVMfromNative transition((JavaThread*)THREAD);
-      if (JdkJfrEvent::is_a(clz)) {
-        // should have been tagged already
-        assert(JdkJfrEvent::is_subklass(clz), "invariant");
-      } else {
+    ThreadInVMfromNative transition((JavaThread*)THREAD);
+    for (jint i = 0; i < classes_count; ++i) {
+      jclass clz = classes[i];
+      if (!JdkJfrEvent::is_a(clz)) {
         // outside the event hierarchy
         JdkJfrEvent::tag_as_host(clz);
       }
     }
-
-    classes[i] = clz;
   }
-  if (jfr_jvmti_env->RetransformClasses(classes_count, classes) != JVMTI_ERROR_NONE) {
-    log_and_throw(THREAD);
+  DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(THREAD));
+  const jvmtiError result = jfr_jvmti_env->RetransformClasses(classes_count, classes);
+  if (result != JVMTI_ERROR_NONE) {
+    log_and_throw(result, THREAD);
   }
 }
 
-static jvmtiError register_callbacks(JavaThread* jt) {
+static bool register_callbacks(JavaThread* jt) {
   assert(jfr_jvmti_env != NULL, "invariant");
   DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(jt));
   jvmtiEventCallbacks callbacks;
@@ -182,10 +196,10 @@
   callbacks.ClassFileLoadHook = jfr_on_class_file_load_hook;
   const jvmtiError jvmti_ret_code = jfr_jvmti_env->SetEventCallbacks(&callbacks, sizeof(callbacks));
   check_jvmti_error(jfr_jvmti_env, jvmti_ret_code, "SetEventCallbacks");
-  return jvmti_ret_code;
+  return jvmti_ret_code == JVMTI_ERROR_NONE;
 }
 
-static jvmtiError register_capabilities(JavaThread* jt) {
+static bool register_capabilities(JavaThread* jt) {
   assert(jfr_jvmti_env != NULL, "invariant");
   DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(jt));
   jvmtiCapabilities capabilities;
@@ -195,7 +209,7 @@
   capabilities.can_retransform_any_class = 1;
   const jvmtiError jvmti_ret_code = jfr_jvmti_env->AddCapabilities(&capabilities);
   check_jvmti_error(jfr_jvmti_env, jvmti_ret_code, "Add Capabilities");
-  return jvmti_ret_code;
+  return jvmti_ret_code == JVMTI_ERROR_NONE;
 }
 
 static jint create_jvmti_env(JavaThread* jt) {
@@ -206,16 +220,14 @@
   return vm->GetEnv((void **)&jfr_jvmti_env, JVMTI_VERSION);
 }
 
-static jvmtiError unregister_callbacks(JavaThread* jt) {
-  if (jfr_jvmti_env == NULL) {
-    return JVMTI_ERROR_NONE;
-  }
+static bool unregister_callbacks(JavaThread* jt) {
+  assert(jfr_jvmti_env != NULL, "invariant");
   jvmtiEventCallbacks callbacks;
   /* Set empty callbacks */
   memset(&callbacks, 0, sizeof(callbacks));
   const jvmtiError jvmti_ret_code = jfr_jvmti_env->SetEventCallbacks(&callbacks, sizeof(callbacks));
   check_jvmti_error(jfr_jvmti_env, jvmti_ret_code, "SetEventCallbacks");
-  return jvmti_ret_code;
+  return jvmti_ret_code == JVMTI_ERROR_NONE;
 }
 
 JfrJvmtiAgent::JfrJvmtiAgent() {}
@@ -223,20 +235,17 @@
 JfrJvmtiAgent::~JfrJvmtiAgent() {
   JavaThread* jt = current_java_thread();
   DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(jt));
-  ThreadToNativeFromVM transition(jt);
-  update_class_file_load_hook_event(JVMTI_DISABLE);
-  unregister_callbacks(jt);
   if (jfr_jvmti_env != NULL) {
+    ThreadToNativeFromVM transition(jt);
+    update_class_file_load_hook_event(JVMTI_DISABLE);
+    unregister_callbacks(jt);
     jfr_jvmti_env->DisposeEnvironment();
     jfr_jvmti_env = NULL;
   }
-  agent = NULL;
 }
 
-static bool initialize() {
-  JavaThread* const jt = current_java_thread();
+static bool initialize(JavaThread* jt) {
   assert(jt != NULL, "invariant");
-  assert(jt->thread_state() == _thread_in_vm, "invariant");
   DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(jt));
   ThreadToNativeFromVM transition(jt);
   if (create_jvmti_env(jt) != JNI_OK) {
@@ -244,25 +253,36 @@
     return false;
   }
   assert(jfr_jvmti_env != NULL, "invariant");
-  if (register_capabilities(jt) != JVMTI_ERROR_NONE) {
+  if (!register_capabilities(jt)) {
+    return false;
+  }
+  if (!register_callbacks(jt)) {
     return false;
   }
-  if (register_callbacks(jt) != JVMTI_ERROR_NONE) {
-    return false;
-  }
-  if (update_class_file_load_hook_event(JVMTI_ENABLE) != JVMTI_ERROR_NONE) {
-    return false;
-  }
-  return true;
+  return update_class_file_load_hook_event(JVMTI_ENABLE);
+}
+
+static void log_and_throw_illegal_state_exception(TRAPS) {
+  DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(THREAD));
+  const char* const illegal_state_msg = "An attempt was made to start JFR too early in the VM initialization sequence.";
+  log_error(jfr, system)(illegal_state_msg);
+  log_error(jfr, system)("JFR uses JVMTI RetransformClasses and requires the JVMTI state to have entered JVMTI_PHASE_LIVE.");
+  log_error(jfr, system)("Please initialize JFR in response to event JVMTI_EVENT_VM_INIT instead of JVMTI_EVENT_VM_START.");
+  JfrJavaSupport::throw_illegal_state_exception(illegal_state_msg, THREAD);
 }
 
 bool JfrJvmtiAgent::create() {
-  assert(jfr_jvmti_env == NULL, "invariant");
+  assert(agent == NULL, "invariant");
+  JavaThread* const jt = current_java_thread();
+  if (!is_valid_jvmti_phase()) {
+    log_and_throw_illegal_state_exception(jt);
+    return false;
+  }
   agent = new JfrJvmtiAgent();
   if (agent == NULL) {
     return false;
   }
-  if (!initialize()) {
+  if (!initialize(jt)) {
     delete agent;
     agent = NULL;
     return false;
@@ -276,4 +296,3 @@
     agent = NULL;
   }
 }
-