8023037: Race between ciEnv::register_method and nmethod::make_not_entrant_or_zombie
Reviewed-by: kvn, iveresov
--- a/hotspot/src/cpu/sparc/vm/sharedRuntime_sparc.cpp Wed Nov 06 06:51:24 2013 -0800
+++ b/hotspot/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();
}
--- a/hotspot/src/share/vm/ci/ciEnv.cpp Wed Nov 06 06:51:24 2013 -0800
+++ b/hotspot/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);
-
}
}
}
--- a/hotspot/src/share/vm/code/nmethod.cpp Wed Nov 06 06:51:24 2013 -0800
+++ b/hotspot/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(),
--- a/hotspot/src/share/vm/runtime/globals.hpp Wed Nov 06 06:51:24 2013 -0800
+++ b/hotspot/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") \