From 30f451441b9414ac04a990293d9f03b8d94db0fa Mon Sep 17 00:00:00 2001 From: vlivanov Date: Fri, 8 Nov 2013 01:13:11 -0800 Subject: [PATCH] 8023037: Race between ciEnv::register_method and nmethod::make_not_entrant_or_zombie Reviewed-by: kvn, iveresov --- src/cpu/sparc/vm/sharedRuntime_sparc.cpp | 12 ------- src/share/vm/ci/ciEnv.cpp | 19 +++------- src/share/vm/code/nmethod.cpp | 46 ++++++++++++------------ src/share/vm/runtime/globals.hpp | 3 -- 4 files changed, 28 insertions(+), 52 deletions(-) diff --git a/src/cpu/sparc/vm/sharedRuntime_sparc.cpp b/src/cpu/sparc/vm/sharedRuntime_sparc.cpp index 62ff0f23b..4b6b1c7c0 100644 --- a/src/cpu/sparc/vm/sharedRuntime_sparc.cpp +++ b/src/cpu/sparc/vm/sharedRuntime_sparc.cpp @@ -1002,18 +1002,6 @@ void AdapterGenerator::gen_i2c_adapter( // and the vm will find there should this case occur. Address callee_target_addr(G2_thread, JavaThread::callee_target_offset()); __ st_ptr(G5_method, callee_target_addr); - - if (StressNonEntrant) { - // Open a big window for deopt failure - __ save_frame(0); - __ mov(G0, L0); - Label loop; - __ bind(loop); - __ sub(L0, 1, L0); - __ br_null_short(L0, Assembler::pt, loop); - __ restore(); - } - __ jmpl(G3, 0, G0); __ delayed()->nop(); } diff --git a/src/share/vm/ci/ciEnv.cpp b/src/share/vm/ci/ciEnv.cpp index 7e61a849b..f6c894948 100644 --- a/src/share/vm/ci/ciEnv.cpp +++ b/src/share/vm/ci/ciEnv.cpp @@ -935,7 +935,9 @@ void ciEnv::register_method(ciMethod* target, // Prevent SystemDictionary::add_to_hierarchy from running // and invalidating our dependencies until we install this method. + // No safepoints are allowed. Otherwise, class redefinition can occur in between. MutexLocker ml(Compile_lock); + No_Safepoint_Verifier nsv; // Change in Jvmti state may invalidate compilation. if (!failing() && @@ -1001,16 +1003,6 @@ void ciEnv::register_method(ciMethod* target, // Free codeBlobs code_buffer->free_blob(); - // stress test 6243940 by immediately making the method - // non-entrant behind the system's back. This has serious - // side effects on the code cache and is not meant for - // general stress testing - if (nm != NULL && StressNonEntrant) { - MutexLockerEx pl(Patching_lock, Mutex::_no_safepoint_check_flag); - NativeJump::patch_verified_entry(nm->entry_point(), nm->verified_entry_point(), - SharedRuntime::get_handle_wrong_method_stub()); - } - if (nm == NULL) { // The CodeCache is full. Print out warning and disable compilation. record_failure("code cache is full"); @@ -1036,11 +1028,11 @@ void ciEnv::register_method(ciMethod* target, char *method_name = method->name_and_sig_as_C_string(); tty->print_cr("Replacing method %s", method_name); } - if (old != NULL ) { + if (old != NULL) { old->make_not_entrant(); } } - if (TraceNMethodInstalls ) { + if (TraceNMethodInstalls) { ResourceMark rm; char *method_name = method->name_and_sig_as_C_string(); ttyLocker ttyl; @@ -1051,7 +1043,7 @@ void ciEnv::register_method(ciMethod* target, // Allow the code to be executed method->set_code(method, nm); } else { - if (TraceNMethodInstalls ) { + if (TraceNMethodInstalls) { ResourceMark rm; char *method_name = method->name_and_sig_as_C_string(); ttyLocker ttyl; @@ -1061,7 +1053,6 @@ void ciEnv::register_method(ciMethod* target, entry_bci); } method->method_holder()->add_osr_nmethod(nm); - } } } diff --git a/src/share/vm/code/nmethod.cpp b/src/share/vm/code/nmethod.cpp index 5d74db0d3..eca6314fe 100644 --- a/src/share/vm/code/nmethod.cpp +++ b/src/share/vm/code/nmethod.cpp @@ -618,21 +618,18 @@ nmethod* nmethod::new_nmethod(methodHandle method, // record this nmethod as dependent on this klass InstanceKlass::cast(klass)->add_dependent_nmethod(nm); } - } - NOT_PRODUCT(if (nm != NULL) nmethod_stats.note_nmethod(nm)); - if (PrintAssembly && nm != NULL) { - Disassembler::decode(nm); + NOT_PRODUCT(nmethod_stats.note_nmethod(nm)); + if (PrintAssembly) { + Disassembler::decode(nm); + } } } - - // verify nmethod - debug_only(if (nm) nm->verify();) // might block - + // Do verification and logging outside CodeCache_lock. if (nm != NULL) { + // Safepoints in nmethod::verify aren't allowed because nm hasn't been installed yet. + DEBUG_ONLY(nm->verify();) nm->log_new_nmethod(); } - - // done return nm; } @@ -2395,20 +2392,23 @@ void nmethod::verify() { void nmethod::verify_interrupt_point(address call_site) { - // This code does not work in release mode since - // owns_lock only is available in debug mode. - CompiledIC* ic = NULL; - Thread *cur = Thread::current(); - if (CompiledIC_lock->owner() == cur || - ((cur->is_VM_thread() || cur->is_ConcurrentGC_thread()) && - SafepointSynchronize::is_at_safepoint())) { - ic = CompiledIC_at(this, call_site); - CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops()); - } else { - MutexLocker ml_verify (CompiledIC_lock); - ic = CompiledIC_at(this, call_site); + // Verify IC only when nmethod installation is finished. + bool is_installed = (method()->code() == this) // nmethod is in state 'alive' and installed + || !this->is_in_use(); // nmethod is installed, but not in 'alive' state + if (is_installed) { + Thread *cur = Thread::current(); + if (CompiledIC_lock->owner() == cur || + ((cur->is_VM_thread() || cur->is_ConcurrentGC_thread()) && + SafepointSynchronize::is_at_safepoint())) { + CompiledIC_at(this, call_site); + CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops()); + } else { + MutexLocker ml_verify (CompiledIC_lock); + CompiledIC_at(this, call_site); + } } - PcDesc* pd = pc_desc_at(ic->end_of_call()); + + PcDesc* pd = pc_desc_at(nativeCall_at(call_site)->return_address()); assert(pd != NULL, "PcDesc must exist"); for (ScopeDesc* sd = new ScopeDesc(this, pd->scope_decode_offset(), pd->obj_decode_offset(), pd->should_reexecute(), diff --git a/src/share/vm/runtime/globals.hpp b/src/share/vm/runtime/globals.hpp index 2fe5420be..c3d98153c 100644 --- a/src/share/vm/runtime/globals.hpp +++ b/src/share/vm/runtime/globals.hpp @@ -3019,9 +3019,6 @@ class CommandLineFlags { notproduct(intx, ZombieALotInterval, 5, \ "Number of exits until ZombieALot kicks in") \ \ - develop(bool, StressNonEntrant, false, \ - "Mark nmethods non-entrant at registration") \ - \ diagnostic(intx, MallocVerifyInterval, 0, \ "If non-zero, verify C heap after every N calls to " \ "malloc/realloc/free") \ -- GitLab