# HG changeset patch
# User jbachorik
# Date 1591927196 -3600
# Node ID 30fb8c8cceb96360b96aa48a5b31f4a1d29bd36d
# Parent  89fb452b3688436102529316cfc5a42f3dffec2d
8233197: Invert JvmtiExport::post_vm_initialized() and Jfr:on_vm_start() start-up order for correct option parsing
8246703: [TESTBUG] Add test for JDK-8233197
Reviewed-by: aph, adinn, neugens

diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/instrumentation/jfrJvmtiAgent.cpp
--- a/src/share/vm/jfr/instrumentation/jfrJvmtiAgent.cpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/instrumentation/jfrJvmtiAgent.cpp	Fri Jun 12 02:59:56 2020 +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
@@ -31,7 +31,9 @@
 #include "jfr/recorder/service/jfrOptionSet.hpp"
 #include "jfr/support/jfrEventClass.hpp"
 #include "memory/resourceArea.hpp"
+#include "prims/jvmtiEnvBase.hpp"
 #include "prims/jvmtiExport.hpp"
+#include "prims/jvmtiUtil.hpp"
 #include "runtime/interfaceSupport.hpp"
 #include "runtime/thread.inline.hpp"
 #include "utilities/exceptions.hpp"
@@ -51,19 +53,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);
 }
 
@@ -116,12 +116,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);
-    if (true) tty->print_cr("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);
+    }
   }
 }
 
@@ -136,12 +147,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;
@@ -152,27 +166,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;
@@ -181,10 +195,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;
@@ -194,7 +208,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) {
@@ -205,16 +219,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() {}
@@ -222,20 +234,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) {
@@ -243,25 +252,38 @@
     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) != JVMTI_ERROR_NONE) {
+  if (!register_callbacks(jt)) {
     return false;
   }
-  if (update_class_file_load_hook_event(JVMTI_ENABLE) != JVMTI_ERROR_NONE) {
-    return false;
+  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.";
+  if (true) {
+    tty->print_cr("%s\n", illegal_state_msg);
+    tty->print_cr("JFR uses JVMTI RetransformClasses and requires the JVMTI state to have entered JVMTI_PHASE_LIVE.\n");
+    tty->print_cr("Please initialize JFR in response to event JVMTI_EVENT_VM_INIT instead of JVMTI_EVENT_VM_START.\n");
   }
-  return true;
+  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;
@@ -275,4 +297,3 @@
     agent = NULL;
   }
 }
-
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/jfr.cpp
--- a/src/share/vm/jfr/jfr.cpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/jfr.cpp	Fri Jun 12 02:59:56 2020 +0100
@@ -32,6 +32,7 @@
 #include "jfr/recorder/service/jfrOptionSet.hpp"
 #include "jfr/support/jfrThreadLocal.hpp"
 #include "runtime/java.hpp"
+#include "utilities/defaultStream.hpp"
 
 bool Jfr::is_enabled() {
   return JfrRecorder::is_enabled();
@@ -45,15 +46,21 @@
   return JfrRecorder::is_recording();
 }
 
-void Jfr::on_vm_init() {
-  if (!JfrRecorder::on_vm_init()) {
-    vm_exit_during_initialization("Failure when starting JFR on_vm_init");
+void Jfr::on_create_vm_1() {
+  if (!JfrRecorder::on_create_vm_1()) {
+    vm_exit_during_initialization("Failure when starting JFR on_create_vm_1");
   }
 }
 
-void Jfr::on_vm_start() {
-  if (!JfrRecorder::on_vm_start()) {
-    vm_exit_during_initialization("Failure when starting JFR on_vm_start");
+void Jfr::on_create_vm_2() {
+  if (!JfrRecorder::on_create_vm_2()) {
+    vm_exit_during_initialization("Failure when starting JFR on_create_vm_2");
+  }
+}
+
+void Jfr::on_create_vm_3() {
+  if (!JfrRecorder::on_create_vm_3()) {
+    vm_exit_during_initialization("Failure when starting JFR on_create_vm_3");
   }
 }
 
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/jfr.hpp
--- a/src/share/vm/jfr/jfr.hpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/jfr.hpp	Fri Jun 12 02:59:56 2020 +0100
@@ -43,8 +43,9 @@
   static bool is_enabled();
   static bool is_disabled();
   static bool is_recording();
-  static void on_vm_init();
-  static void on_vm_start();
+  static void on_create_vm_1();
+  static void on_create_vm_2();
+  static void on_create_vm_3();
   static void on_unloading_classes();
   static void on_thread_start(Thread* thread);
   static void on_thread_exit(Thread* thread);
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/jni/jfrJavaSupport.cpp
--- a/src/share/vm/jfr/jni/jfrJavaSupport.cpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/jni/jfrJavaSupport.cpp	Fri Jun 12 02:59:56 2020 +0100
@@ -515,6 +515,10 @@
   create_and_throw(vmSymbols::java_lang_ClassFormatError(), message, THREAD);
 }
 
+void JfrJavaSupport::throw_runtime_exception(const char* message, TRAPS) {
+  create_and_throw(vmSymbols::java_lang_RuntimeException(), message, THREAD);
+}
+
 void JfrJavaSupport::abort(jstring errorMsg, Thread* t) {
   DEBUG_ONLY(check_java_thread_in_vm(t));
 
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/jni/jfrJavaSupport.hpp
--- a/src/share/vm/jfr/jni/jfrJavaSupport.hpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/jni/jfrJavaSupport.hpp	Fri Jun 12 02:59:56 2020 +0100
@@ -81,6 +81,7 @@
   static void throw_internal_error(const char* message, TRAPS);
   static void throw_out_of_memory_error(const char* message, TRAPS);
   static void throw_class_format_error(const char* message, TRAPS);
+  static void throw_runtime_exception(const char* message, TRAPS);
 
   static jlong jfr_thread_id(jobject target_thread);
 
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/jni/jfrJniMethod.cpp
--- a/src/share/vm/jfr/jni/jfrJniMethod.cpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/jni/jfrJniMethod.cpp	Fri Jun 12 02:59:56 2020 +0100
@@ -191,7 +191,9 @@
     return JNI_TRUE;
   }
   if (!JfrRecorder::create(simulate_failure == JNI_TRUE)) {
-    JfrJavaSupport::throw_illegal_state_exception("Unable to start Jfr", thread);
+    if (!thread->has_pending_exception()) {
+      JfrJavaSupport::throw_illegal_state_exception("Unable to start Jfr", thread);
+    }
     return JNI_FALSE;
   }
   return JNI_TRUE;
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/recorder/jfrRecorder.cpp
--- a/src/share/vm/jfr/recorder/jfrRecorder.cpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/recorder/jfrRecorder.cpp	Fri Jun 12 02:59:56 2020 +0100
@@ -44,6 +44,9 @@
 #include "runtime/handles.inline.hpp"
 #include "runtime/globals_extension.hpp"
 #include "utilities/growableArray.hpp"
+#ifdef ASSERT
+#include "prims/jvmtiEnvBase.hpp"
+#endif
 
 bool JfrRecorder::_shutting_down = false;
 
@@ -57,7 +60,9 @@
 
 static bool enable() {
   assert(!_enabled, "invariant");
-  FLAG_SET_MGMT(bool, FlightRecorder, true);
+  if (!FlightRecorder) {
+    FLAG_SET_MGMT(bool, FlightRecorder, true);
+  }
   _enabled = FlightRecorder;
   assert(_enabled, "invariant");
   return _enabled;
@@ -67,7 +72,7 @@
   return _enabled;
 }
 
-bool JfrRecorder::on_vm_init() {
+bool JfrRecorder::on_create_vm_1() {
   if (!is_disabled()) {
     if (FlightRecorder || StartFlightRecording != NULL) {
       enable();
@@ -92,7 +97,7 @@
 
 static void teardown_startup_support() {
   release_recordings();
-  JfrOptionSet::release_startup_recording_options();
+  JfrOptionSet::release_start_flight_recording_options();
 }
 
 // Parsing options here to detect errors as soon as possible
@@ -110,7 +115,7 @@
 }
 
 static bool validate_recording_options(TRAPS) {
-  const GrowableArray<const char*>* options = JfrOptionSet::startup_recording_options();
+  const GrowableArray<const char*>* options = JfrOptionSet::start_flight_recording_options();
   if (options == NULL) {
     return true;
   }
@@ -143,7 +148,7 @@
   return true;
 }
 
-static bool launch_recordings(TRAPS) {
+static bool launch_command_line_recordings(TRAPS) {
   bool result = true;
   if (dcmd_recordings_array != NULL) {
     const int length = dcmd_recordings_array->length();
@@ -161,7 +166,7 @@
 
 static bool is_cds_dump_requested() {
   // we will not be able to launch recordings if a cds dump is being requested
-  if (DumpSharedSpaces && (JfrOptionSet::startup_recording_options() != NULL)) {
+  if (DumpSharedSpaces && (JfrOptionSet::start_flight_recording_options() != NULL)) {
     warning("JFR will be disabled during CDS dumping");
     teardown_startup_support();
     return true;
@@ -169,7 +174,7 @@
   return false;
 }
 
-bool JfrRecorder::on_vm_start() {
+bool JfrRecorder::on_create_vm_2() {
   if (is_cds_dump_requested()) {
     return true;
   }
@@ -196,9 +201,13 @@
   if (!is_enabled()) {
     return true;
   }
+  return true;
+}
 
-  return launch_recordings(thread);
-}
+bool JfrRecorder::on_create_vm_3() {
+  assert(JvmtiEnvBase::get_phase() == JVMTI_PHASE_LIVE, "invalid init sequence");
+  return launch_command_line_recordings(Thread::current());
+ }
 
 static bool _created = false;
 
@@ -266,7 +275,6 @@
 }
 
 // subsystems
-static JfrJvmtiAgent* _jvmti_agent = NULL;
 static JfrPostBox* _post_box = NULL;
 static JfrStorage* _storage = NULL;
 static JfrCheckpointManager* _checkpoint_manager = NULL;
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/recorder/jfrRecorder.hpp
--- a/src/share/vm/jfr/recorder/jfrRecorder.hpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/recorder/jfrRecorder.hpp	Fri Jun 12 02:59:56 2020 +0100
@@ -40,6 +40,9 @@
  private:
   static bool _shutting_down;
 
+  static bool on_create_vm_1();
+  static bool on_create_vm_2();
+  static bool on_create_vm_3();
   static bool create_checkpoint_manager();
   static bool create_chunk_repository();
   static bool create_java_event_writer();
@@ -54,8 +57,6 @@
   static bool create_components();
   static void destroy_components();
   static void on_recorder_thread_exit();
-  static bool on_vm_start();
-  static bool on_vm_init();
 
  public:
   static bool is_enabled();
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/recorder/service/jfrOptionSet.cpp
--- a/src/share/vm/jfr/recorder/service/jfrOptionSet.cpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/recorder/service/jfrOptionSet.cpp	Fri Jun 12 02:59:56 2020 +0100
@@ -679,7 +679,7 @@
   return false;
 }
 
-static GrowableArray<const char*>* startup_recording_options_array = NULL;
+static GrowableArray<const char*>* start_flight_recording_options_array = NULL;
 
 bool JfrOptionSet::parse_start_flight_recording_option(const JavaVMOption** option, char* delimiter) {
   assert(option != NULL, "invariant");
@@ -702,28 +702,28 @@
   assert(value != NULL, "invariant");
   const size_t value_length = strlen(value);
 
-  if (startup_recording_options_array == NULL) {
-    startup_recording_options_array = new (ResourceObj::C_HEAP, mtTracing) GrowableArray<const char*>(8, true, mtTracing);
+  if (start_flight_recording_options_array == NULL) {
+    start_flight_recording_options_array = new (ResourceObj::C_HEAP, mtTracing) GrowableArray<const char*>(8, true, mtTracing);
   }
-  assert(startup_recording_options_array != NULL, "invariant");
+  assert(start_flight_recording_options_array != NULL, "invariant");
   char* const startup_value = NEW_C_HEAP_ARRAY(char, value_length + 1, mtTracing);
   strncpy(startup_value, value, value_length + 1);
   assert(strncmp(startup_value, value, value_length) == 0, "invariant");
-  startup_recording_options_array->append(startup_value);
+  start_flight_recording_options_array->append(startup_value);
   return false;
 }
 
-const GrowableArray<const char*>* JfrOptionSet::startup_recording_options() {
-  return startup_recording_options_array;
+const GrowableArray<const char*>* JfrOptionSet::start_flight_recording_options() {
+  return start_flight_recording_options_array;
 }
 
-void JfrOptionSet::release_startup_recording_options() {
-  if (startup_recording_options_array != NULL) {
-    const int length = startup_recording_options_array->length();
+void JfrOptionSet::release_start_flight_recording_options() {
+  if (start_flight_recording_options_array != NULL) {
+    const int length = start_flight_recording_options_array->length();
     for (int i = 0; i < length; ++i) {
-      FREE_C_HEAP_ARRAY(char, startup_recording_options_array->at(i), mtTracing);
+      FREE_C_HEAP_ARRAY(char, start_flight_recording_options_array->at(i), mtTracing);
     }
-    delete startup_recording_options_array;
-    startup_recording_options_array = NULL;
+    delete start_flight_recording_options_array;
+    start_flight_recording_options_array = NULL;
   }
 }
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/jfr/recorder/service/jfrOptionSet.hpp
--- a/src/share/vm/jfr/recorder/service/jfrOptionSet.hpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/jfr/recorder/service/jfrOptionSet.hpp	Fri Jun 12 02:59:56 2020 +0100
@@ -80,8 +80,8 @@
 
   static bool parse_flight_recorder_option(const JavaVMOption** option, char* delimiter);
   static bool parse_start_flight_recording_option(const JavaVMOption** option, char* delimiter);
-  static const GrowableArray<const char*>* startup_recording_options();
-  static void release_startup_recording_options();
+  static const GrowableArray<const char*>* start_flight_recording_options();
+  static void release_start_flight_recording_options();
 };
 
 #endif // SHARE_VM_JFR_RECORDER_SERVICE_JFROPTIONSET_HPP
diff -r 89fb452b3688 -r 30fb8c8cceb9 src/share/vm/runtime/thread.cpp
--- a/src/share/vm/runtime/thread.cpp	Sun Jun 07 18:57:54 2020 +0100
+++ b/src/share/vm/runtime/thread.cpp	Fri Jun 12 02:59:56 2020 +0100
@@ -3440,7 +3440,7 @@
     return status;
   }
 
-  JFR_ONLY(Jfr::on_vm_init();)
+  JFR_ONLY(Jfr::on_create_vm_1();)
 
   // Should be done after the heap is fully created
   main_thread->cache_global_variables();
@@ -3491,12 +3491,16 @@
     ShouldNotReachHere();
   }
 
+#if !INCLUDE_JFR
+  // if JFR is not enabled at the build time keep the original JvmtiExport location
+
   // Always call even when there are not JVMTI environments yet, since environments
   // may be attached late and JVMTI must track phases of VM execution
   JvmtiExport::enter_start_phase();
 
   // Notify JVMTI agents that VM has started (JNI is up) - nop if no agents.
   JvmtiExport::post_vm_start();
+#endif
 
   {
     TraceTime timer("Initialize java.lang classes", TraceStartupTime);
@@ -3543,6 +3547,19 @@
     initialize_class(vmSymbols::java_lang_IllegalArgumentException(), CHECK_0);
   }
 
+  JFR_ONLY(
+    Jfr::on_create_vm_2();
+
+    // if JFR is enabled at build time the JVMTI needs to be handled only after on_create_vm_2() call
+
+    // Always call even when there are not JVMTI environments yet, since environments
+    // may be attached late and JVMTI must track phases of VM execution
+    JvmtiExport::enter_start_phase();
+
+    // Notify JVMTI agents that VM has started (JNI is up) - nop if no agents.
+    JvmtiExport::post_vm_start();
+  )
+
   // See        : bugid 4211085.
   // Background : the static initializer of java.lang.Compiler tries to read
   //              property"java.compiler" and read & write property "java.vm.info".
@@ -3637,7 +3654,7 @@
   // Notify JVMTI agents that VM initialization is complete - nop if no agents.
   JvmtiExport::post_vm_initialized();
 
-  JFR_ONLY(Jfr::on_vm_start();)
+  JFR_ONLY(Jfr::on_create_vm_3();)
 
   if (CleanChunkPoolAsync) {
     Chunk::start_chunk_pool_cleaner_task();
diff -r 89fb452b3688 -r 30fb8c8cceb9 test/runtime/8233197/T.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/runtime/8233197/T.java	Fri Jun 12 02:59:56 2020 +0100
@@ -0,0 +1,11 @@
+public class T
+{
+  public static void main(String[] args) throws Exception
+  {
+    for (int i = 0; i < 50; i++) {
+      System.out.print("+");
+      Thread.sleep(1);
+    }
+    System.out.println();
+  }
+}
diff -r 89fb452b3688 -r 30fb8c8cceb9 test/runtime/8233197/Test8233197.sh
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/runtime/8233197/Test8233197.sh	Fri Jun 12 02:59:56 2020 +0100
@@ -0,0 +1,153 @@
+#!/bin/sh
+
+# Copyright (c) 2013, 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
+# under the terms of the GNU General Public License version 2 only, as
+# published by the Free Software Foundation.
+#
+# This code is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+# version 2 for more details (a copy is included in the LICENSE file that
+# accompanied this code).
+#
+# You should have received a copy of the GNU General Public License version
+# 2 along with this work; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+# or visit www.oracle.com if you need additional information or have any
+# questions.
+
+##
+## @test Test8233197.sh
+## @bug 8233197
+## @summary Check that JFR subsystem can be initialized from VMStart JVMTI event
+## @compile T.java
+## @run shell Test8233197.sh
+##
+
+set -x
+if [ "${TESTSRC}" = "" ]
+then
+  TESTSRC=${PWD}
+  echo "TESTSRC not set.  Using "${TESTSRC}" as default"
+fi
+echo "TESTSRC=${TESTSRC}"
+## Adding common setup Variables for running shell tests.
+. ${TESTSRC}/../../test_env.sh
+
+# set platform-dependent variables
+OS=`uname -s`
+case "$OS" in
+  Linux)
+    gcc_cmd=`which gcc`
+    if [ "x$gcc_cmd" == "x" ]; then
+        echo "WARNING: gcc not found. Cannot execute test." 2>&1
+        exit 0;
+    fi
+    NULL=/dev/null
+    PS=":"
+    FS="/"
+    ;;
+  * )
+    echo "Test passed; only valid for Linux"
+    exit 0;
+    ;;
+esac
+
+${TESTJAVA}${FS}bin${FS}java ${TESTVMOPTS} -Xinternalversion > vm_version.out 2>&1
+
+# Bitness:
+# Cannot simply look at TESTVMOPTS as -d64 is not
+# passed if there is only a 64-bit JVM available.
+
+grep "64-Bit" vm_version.out > ${NULL}
+if [ "$?" = "0" ]
+then
+  COMP_FLAG="-m64"
+else
+  COMP_FLAG="-m32"
+fi
+
+
+# Architecture:
+# Translate uname output to JVM directory name, but permit testing
+# 32-bit x86 on an x64 platform.
+ARCH=`uname -m`
+case "$ARCH" in
+  x86_64)
+    if [ "$COMP_FLAG" = "-m32" ]; then
+      ARCH=i386
+    else
+      ARCH=amd64
+    fi
+    ;;
+  ppc64)
+    if [ "$COMP_FLAG" = "-m32" ]; then
+      ARCH=ppc
+    else
+      ARCH=ppc64
+    fi
+    ;;
+  sparc64)
+    if [ "$COMP_FLAG" = "-m32" ]; then
+      ARCH=sparc
+    else
+      ARCH=sparc64
+    fi
+    ;;
+  arm*)
+    # 32-bit ARM machine: compiler may not recognise -m32
+    COMP_FLAG=""
+    ARCH=arm
+    ;;
+  aarch64)
+    # 64-bit arm machine, could be testing 32 or 64-bit:
+    if [ "$COMP_FLAG" = "-m32" ]; then
+      ARCH=arm
+    else
+      ARCH=aarch64
+    fi
+    ;;
+  i586)
+    ARCH=i386
+    ;;
+  i686)
+    ARCH=i386
+    ;;
+  # Assuming other ARCH values need no translation
+esac
+
+
+# VM type: need to know server or client
+VMTYPE=client
+grep Server vm_version.out > ${NULL}
+if [ "$?" = "0" ]
+then
+  VMTYPE=server
+fi
+
+
+LD_LIBRARY_PATH=.:${COMPILEJAVA}/jre/lib/${ARCH}/${VMTYPE}:/usr/lib:$LD_LIBRARY_PATH
+export LD_LIBRARY_PATH
+
+cp ${TESTSRC}${FS}libJvmtiAgent.c .
+
+# Copy the result of our @compile action:
+cp ${TESTCLASSES}${FS}T.class .
+
+echo "Architecture: ${ARCH}"
+echo "Compilation flag: ${COMP_FLAG}"
+echo "VM type: ${VMTYPE}"
+
+$gcc_cmd -DLINUX ${COMP_FLAG} -Wl, -g -fno-strict-aliasing -fPIC -fno-omit-frame-pointer -W -Wall  -Wno-unused -Wno-parentheses -c -o libJvmtiAgent.o \
+    -I${COMPILEJAVA}/include -I${COMPILEJAVA}/include/linux \
+    -L${COMPILEJAVA}/jre/lib/${ARCH}/${VMTYPE} \
+    libJvmtiAgent.c
+$gcc_cmd -shared -o libJvmtiAgent.so libJvmtiAgent.o
+
+"$TESTJAVA/bin/java" $TESTVMOPTS -agentlib:JvmtiAgent -cp $(pwd) T > T.out
+exit $?
\ No newline at end of file
diff -r 89fb452b3688 -r 30fb8c8cceb9 test/runtime/8233197/libJvmtiAgent.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/runtime/8233197/libJvmtiAgent.c	Fri Jun 12 02:59:56 2020 +0100
@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) 2017, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include "jvmti.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifndef JNI_ENV_ARG
+
+#ifdef __cplusplus
+#define JNI_ENV_ARG(x, y) y
+#define JNI_ENV_PTR(x) x
+#else
+#define JNI_ENV_ARG(x,y) x, y
+#define JNI_ENV_PTR(x) (*x)
+#endif
+
+#endif
+
+#define TranslateError(err) "JVMTI error"
+
+static jvmtiEnv *jvmti = NULL;
+
+static jint Agent_Initialize(JavaVM *jvm, char *options, void *reserved);
+
+JNIEXPORT
+jint JNICALL Agent_OnLoad(JavaVM *jvm, char *options, void *reserved) {
+    return Agent_Initialize(jvm, options, reserved);
+}
+
+JNIEXPORT
+jint JNICALL Agent_OnAttach(JavaVM *jvm, char *options, void *reserved) {
+    return Agent_Initialize(jvm, options, reserved);
+}
+
+JNIEXPORT
+jint JNICALL JNI_OnLoad(JavaVM *jvm, void *reserved) {
+    return JNI_VERSION_1_8;
+}
+
+static void JNICALL
+Callback_VMStart(jvmtiEnv *jvmti_env, JNIEnv *env) {
+    printf("Localizing jdk.jfr.FlightRecorder\n");
+    // without fix for 8233197 the process will crash at the following line
+    jclass cls = (*env)->FindClass(env, "jdk/jfr/FlightRecorder");
+    jmethodID mid = (*env)->GetStaticMethodID(env, cls, "getFlightRecorder", "()Ljdk/jfr/FlightRecorder;");
+    if (mid == 0) {
+        printf("Unable to localize jdk.jfr.FlightRecorder#getFlightRecorder() method\n");
+        // crash the tested JVM to make the test fail
+        exit(-1);
+    }
+    printf("Going to initialize JFR subsystem ...\n");
+    jobject jfr = (*env)->CallStaticObjectMethod(env, cls, mid);
+
+    if (!(*env)->ExceptionCheck(env)) {
+        // crash the tested JVM to make the test fail
+        printf("JFR subsystem is wrongly initialized too early\n");
+        exit(-2);
+    }
+    // exit VM
+    exit(0);
+}
+
+static
+jint Agent_Initialize(JavaVM *jvm, char *options, void *reserved) {
+    jint res, size;
+    jvmtiCapabilities caps;
+    jvmtiEventCallbacks callbacks;
+    jvmtiError err;
+
+    res = JNI_ENV_PTR(jvm)->GetEnv(JNI_ENV_ARG(jvm, (void **) &jvmti),
+        JVMTI_VERSION_1_2);
+    if (res != JNI_OK || jvmti == NULL) {
+        printf("    Error: wrong result of a valid call to GetEnv!\n");
+        return JNI_ERR;
+    }
+
+    size = (jint)sizeof(callbacks);
+
+    memset(&callbacks, 0, sizeof(callbacks));
+    callbacks.VMStart = Callback_VMStart;
+
+    err = (*jvmti)->SetEventCallbacks(jvmti, &callbacks, size);
+    if (err != JVMTI_ERROR_NONE) {
+        printf("    Error in SetEventCallbacks: %s (%d)\n", TranslateError(err), err);
+        return JNI_ERR;
+    }
+
+    err = (*jvmti)->SetEventNotificationMode(jvmti, JVMTI_ENABLE, JVMTI_EVENT_VM_START, (jthread)NULL);
+    if (err != JVMTI_ERROR_NONE) {
+        printf("    Error in SetEventNotificationMode: %s (%d)\n", TranslateError(err), err);
+        return JNI_ERR;
+    }
+    return JNI_OK;
+}
+
+#ifdef __cplusplus
+}
+#endif