# HG changeset patch # User vlivanov # Date 1383901991 28800 # Node ID e2509677809c20406d1f804e3c3a6c9550299717 # Parent be525e91f65b388864cb46caf9049a6682ba95c9 8023037: Race between ciEnv::register_method and nmethod::make_not_entrant_or_zombie Reviewed-by: kvn, iveresov diff -r be525e91f65b -r e2509677809c src/cpu/sparc/vm/sharedRuntime_sparc.cpp --- a/src/cpu/sparc/vm/sharedRuntime_sparc.cpp Wed Nov 06 06:51:24 2013 -0800 +++ b/src/cpu/sparc/vm/sharedRuntime_sparc.cpp Fri Nov 08 01:13:11 2013 -0800 @@ -1002,18 +1002,6 @@ // 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 -r be525e91f65b -r e2509677809c src/share/vm/ci/ciEnv.cpp --- a/src/share/vm/ci/ciEnv.cpp Wed Nov 06 06:51:24 2013 -0800 +++ b/src/share/vm/ci/ciEnv.cpp Fri Nov 08 01:13:11 2013 -0800 @@ -935,7 +935,9 @@ // 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 @@ // 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 @@ 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 @@ // 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 @@ entry_bci); } method->method_holder()->add_osr_nmethod(nm); - } } } diff -r be525e91f65b -r e2509677809c src/share/vm/code/nmethod.cpp --- a/src/share/vm/code/nmethod.cpp Wed Nov 06 06:51:24 2013 -0800 +++ b/src/share/vm/code/nmethod.cpp Fri Nov 08 01:13:11 2013 -0800 @@ -618,21 +618,18 @@ // 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_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 -r be525e91f65b -r e2509677809c src/share/vm/runtime/globals.hpp --- a/src/share/vm/runtime/globals.hpp Wed Nov 06 06:51:24 2013 -0800 +++ b/src/share/vm/runtime/globals.hpp Fri Nov 08 01:13:11 2013 -0800 @@ -3019,9 +3019,6 @@ 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") \