8233197: Invert JvmtiExport::post_vm_initialized() and Jfr:on_vm_start() start-up order for correct option parsing

Reviewed-by: sspitsyn, egahlin
This commit is contained in:
Markus Grönlund 2019-11-25 18:38:01 +01:00
parent 1b3fd96d2a
commit 096bacb9ea
13 changed files with 138 additions and 94 deletions

View File

@ -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 void check_jvmti_error(jvmtiEnv* jvmti, jvmtiError errnum, const char* st
}
}
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 @@ static jclass* create_classes_array(jint classes_count, TRAPS) {
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 void check_exception_and_log(JNIEnv* env, TRAPS) {
}
}
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 @@ void JfrJvmtiAgent::retransform_classes(JNIEnv* env, jobjectArray classes_array,
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 @@ static jvmtiError register_callbacks(JavaThread* jt) {
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 @@ static jvmtiError register_capabilities(JavaThread* jt) {
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 @@ static jint create_jvmti_env(JavaThread* jt) {
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() {}
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 @@ static bool initialize() {
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 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 @@ void JfrJvmtiAgent::destroy() {
agent = NULL;
}
}

View File

@ -45,15 +45,21 @@ bool Jfr::is_recording() {
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");
}
}

View File

@ -43,8 +43,9 @@ class Jfr : AllStatic {
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);

View File

@ -540,6 +540,10 @@ void JfrJavaSupport::throw_class_format_error(const char* message, TRAPS) {
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));

View File

@ -84,6 +84,7 @@ class JfrJavaSupport : public AllStatic {
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 bool is_jdk_jfr_module_available();
static bool is_jdk_jfr_module_available(outputStream* stream, TRAPS);

View File

@ -192,7 +192,9 @@ JVM_ENTRY_NO_ENV(jboolean, jfr_create_jfr(JNIEnv* env, jobject jvm, jboolean sim
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;

View File

@ -46,6 +46,9 @@
#include "runtime/handles.inline.hpp"
#include "runtime/globals_extension.hpp"
#include "utilities/growableArray.hpp"
#ifdef ASSERT
#include "prims/jvmtiEnvBase.hpp"
#endif
bool JfrRecorder::is_disabled() {
// True if -XX:-FlightRecorder has been explicitly set on the
@ -57,7 +60,9 @@ static bool _enabled = false;
static bool enable() {
assert(!_enabled, "invariant");
FLAG_SET_MGMT(FlightRecorder, true);
if (!FlightRecorder) {
FLAG_SET_MGMT(FlightRecorder, true);
}
_enabled = FlightRecorder;
assert(_enabled, "invariant");
return _enabled;
@ -67,7 +72,7 @@ bool JfrRecorder::is_enabled() {
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 release_recordings() {
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 parse_recording_options(const char* options, JfrStartFlightRecording
}
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 @@ static bool launch_recording(JfrStartFlightRecordingDCmd* dcmd_recording, TRAPS)
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();
@ -168,7 +173,7 @@ static void log_jdk_jfr_module_resolution_error(TRAPS) {
static bool is_cds_dump_requested() {
// we will not be able to launch recordings if a cds dump is being requested
if (Arguments::is_dumping_archive() && (JfrOptionSet::startup_recording_options() != NULL)) {
if (Arguments::is_dumping_archive() && (JfrOptionSet::start_flight_recording_options() != NULL)) {
warning("JFR will be disabled during CDS dumping");
teardown_startup_support();
return true;
@ -176,7 +181,7 @@ static bool is_cds_dump_requested() {
return false;
}
bool JfrRecorder::on_vm_start() {
bool JfrRecorder::on_create_vm_2() {
if (is_cds_dump_requested()) {
return true;
}
@ -187,9 +192,7 @@ bool JfrRecorder::on_vm_start() {
if (!register_jfr_dcmds()) {
return false;
}
const bool in_graph = JfrJavaSupport::is_jdk_jfr_module_available();
if (in_graph) {
if (!validate_recording_options(thread)) {
return false;
@ -198,17 +201,19 @@ bool JfrRecorder::on_vm_start() {
return false;
}
}
if (!is_enabled()) {
return true;
}
if (!in_graph) {
log_jdk_jfr_module_resolution_error(thread);
return false;
}
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;
@ -277,7 +282,6 @@ bool JfrRecorder::create_components() {
}
// subsystems
static JfrJvmtiAgent* _jvmti_agent = NULL;
static JfrPostBox* _post_box = NULL;
static JfrStorage* _storage = NULL;
static JfrCheckpointManager* _checkpoint_manager = NULL;

View File

@ -38,6 +38,9 @@ class JfrRecorder : public JfrCHeapObj {
friend class Jfr;
friend void recorderthread_entry(JavaThread*, Thread*);
private:
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();
@ -52,8 +55,6 @@ class JfrRecorder : public JfrCHeapObj {
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();

View File

@ -677,7 +677,7 @@ bool JfrOptionSet::parse_flight_recorder_option(const JavaVMOption** option, cha
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");
@ -700,28 +700,28 @@ bool JfrOptionSet::parse_start_flight_recording_option(const JavaVMOption** opti
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));
FREE_C_HEAP_ARRAY(char, start_flight_recording_options_array->at(i));
}
delete startup_recording_options_array;
startup_recording_options_array = NULL;
delete start_flight_recording_options_array;
start_flight_recording_options_array = NULL;
}
}

View File

@ -80,8 +80,8 @@ class JfrOptionSet : public AllStatic {
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_JFR_RECORDER_SERVICE_JFROPTIONSET_HPP

View File

@ -3887,7 +3887,7 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
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();
@ -4026,6 +4026,8 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
// loaded until phase 2 completes
call_initPhase2(CHECK_JNI_ERR);
JFR_ONLY(Jfr::on_create_vm_2();)
// 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();
@ -4061,7 +4063,7 @@ jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
// 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 INCLUDE_MANAGEMENT
Management::initialize(THREAD);

View File

@ -203,6 +203,8 @@ public final class JVM {
* Call to invoke event tagging and retransformation of the passed classes
*
* @param classes
*
* @throws IllegalStateException if wrong JVMTI phase.
*/
public native synchronized void retransformClasses(Class<?>[] classes);

View File

@ -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
@ -123,6 +123,8 @@ public final class JDKEvents {
list.add(java.lang.Error.class);
Logger.log(LogTag.JFR_SYSTEM, LogLevel.INFO, "Retransformed JDK classes");
jvm.retransformClasses(list.toArray(new Class<?>[list.size()]));
} catch (IllegalStateException ise) {
throw ise;
} catch (Exception e) {
Logger.log(LogTag.JFR_SYSTEM, LogLevel.WARN, "Could not add instrumentation for JDK events. " + e.getMessage());
}