From dd6330ceb2ba29238019c2d0fea4efb9bc7a225d Mon Sep 17 00:00:00 2001 From: kvn Date: Fri, 6 Dec 2013 12:11:51 -0800 Subject: [PATCH] 8028107: Kitchensink crashed with EAV Summary: check the state of caller and callee nmethods and skip call site patching if any of them is not alive Reviewed-by: jrose, twisti --- src/share/vm/code/compiledIC.cpp | 4 ++-- src/share/vm/code/nmethod.cpp | 14 ++++++------- src/share/vm/code/nmethod.hpp | 15 +++++++------- src/share/vm/runtime/sharedRuntime.cpp | 28 +++++++++++++++++++++----- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/share/vm/code/compiledIC.cpp b/src/share/vm/code/compiledIC.cpp index 489e649fa..251a4c79c 100644 --- a/src/share/vm/code/compiledIC.cpp +++ b/src/share/vm/code/compiledIC.cpp @@ -418,7 +418,7 @@ void CompiledIC::compute_monomorphic_entry(methodHandle method, TRAPS) { nmethod* method_code = method->code(); address entry = NULL; - if (method_code != NULL) { + if (method_code != NULL && method_code->is_in_use()) { // Call to compiled code if (static_bound || is_optimized) { entry = method_code->verified_entry_point(); @@ -545,7 +545,7 @@ void CompiledStaticCall::set(const StaticCallInfo& info) { void CompiledStaticCall::compute_entry(methodHandle m, StaticCallInfo& info) { nmethod* m_code = m->code(); info._callee = m; - if (m_code != NULL) { + if (m_code != NULL && m_code->is_in_use()) { info._to_interpreter = false; info._entry = m_code->verified_entry_point(); } else { diff --git a/src/share/vm/code/nmethod.cpp b/src/share/vm/code/nmethod.cpp index 68718499a..b9dd99034 100644 --- a/src/share/vm/code/nmethod.cpp +++ b/src/share/vm/code/nmethod.cpp @@ -459,7 +459,7 @@ const char* nmethod::compile_kind() const { // Fill in default values for various flag fields void nmethod::init_defaults() { - _state = alive; + _state = in_use; _marked_for_reclamation = 0; _has_flushed_dependencies = 0; _has_unsafe_access = 0; @@ -1660,8 +1660,8 @@ void nmethod::do_unloading(BoolObjectClosure* is_alive, bool unloading_occurred) CompiledICHolder* cichk_oop = ic->cached_icholder(); if (cichk_oop->holder_method()->method_holder()->is_loader_alive(is_alive) && cichk_oop->holder_klass()->is_loader_alive(is_alive)) { - continue; - } + continue; + } } else { Metadata* ic_oop = ic->cached_metadata(); if (ic_oop != NULL) { @@ -1677,8 +1677,8 @@ void nmethod::do_unloading(BoolObjectClosure* is_alive, bool unloading_occurred) ShouldNotReachHere(); } } - } - ic->set_to_clean(); + } + ic->set_to_clean(); } } } @@ -2393,8 +2393,8 @@ void nmethod::verify() { void nmethod::verify_interrupt_point(address 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 + bool is_installed = (method()->code() == this) // nmethod is in state 'in_use' and installed + || !this->is_in_use(); // nmethod is installed, but not in 'in_use' state if (is_installed) { Thread *cur = Thread::current(); if (CompiledIC_lock->owner() == cur || diff --git a/src/share/vm/code/nmethod.hpp b/src/share/vm/code/nmethod.hpp index 538bcf778..4e1afcc04 100644 --- a/src/share/vm/code/nmethod.hpp +++ b/src/share/vm/code/nmethod.hpp @@ -184,11 +184,12 @@ class nmethod : public CodeBlob { bool _oops_are_stale; // indicates that it's no longer safe to access oops section #endif - enum { alive = 0, - not_entrant = 1, // uncommon trap has happened but activations may still exist - zombie = 2, - unloaded = 3 }; - + enum { in_use = 0, // executable nmethod + not_entrant = 1, // marked for deoptimization but activations may still exist, + // will be transformed to zombie when all activations are gone + zombie = 2, // no activations exist, nmethod is ready for purge + unloaded = 3 }; // there should be no activations, should not be called, + // will be transformed to zombie immediately jbyte _scavenge_root_state; @@ -407,8 +408,8 @@ class nmethod : public CodeBlob { address verified_entry_point() const { return _verified_entry_point; } // if klass is correct // flag accessing and manipulation - bool is_in_use() const { return _state == alive; } - bool is_alive() const { return _state == alive || _state == not_entrant; } + bool is_in_use() const { return _state == in_use; } + bool is_alive() const { return _state == in_use || _state == not_entrant; } bool is_not_entrant() const { return _state == not_entrant; } bool is_zombie() const { return _state == zombie; } bool is_unloaded() const { return _state == unloaded; } diff --git a/src/share/vm/runtime/sharedRuntime.cpp b/src/share/vm/runtime/sharedRuntime.cpp index d963c3520..a993c4d38 100644 --- a/src/share/vm/runtime/sharedRuntime.cpp +++ b/src/share/vm/runtime/sharedRuntime.cpp @@ -1178,12 +1178,12 @@ methodHandle SharedRuntime::resolve_sub_helper(JavaThread *thread, CodeBlob* caller_cb = caller_frame.cb(); guarantee(caller_cb != NULL && caller_cb->is_nmethod(), "must be called from nmethod"); nmethod* caller_nm = caller_cb->as_nmethod_or_null(); + // make sure caller is not getting deoptimized // and removed before we are done with it. // CLEANUP - with lazy deopt shouldn't need this lock nmethodLocker caller_lock(caller_nm); - // determine call info & receiver // note: a) receiver is NULL for static calls // b) an exception is thrown if receiver is NULL for non-static calls @@ -1198,6 +1198,11 @@ methodHandle SharedRuntime::resolve_sub_helper(JavaThread *thread, (!is_virtual && invoke_code == Bytecodes::_invokedynamic) || ( is_virtual && invoke_code != Bytecodes::_invokestatic ), "inconsistent bytecode"); + // We do not patch the call site if the caller nmethod has been made non-entrant. + if (!caller_nm->is_in_use()) { + return callee_method; + } + #ifndef PRODUCT // tracing/debugging/statistics int *addr = (is_optimized) ? (&_resolve_opt_virtual_ctr) : @@ -1237,6 +1242,10 @@ methodHandle SharedRuntime::resolve_sub_helper(JavaThread *thread, // Make sure the callee nmethod does not get deoptimized and removed before // we are done patching the code. nmethod* callee_nm = callee_method->code(); + if (callee_nm != NULL && !callee_nm->is_in_use()) { + // Patch call site to C2I adapter if callee nmethod is deoptimized or unloaded. + callee_nm = NULL; + } nmethodLocker nl_callee(callee_nm); #ifdef ASSERT address dest_entry_point = callee_nm == NULL ? 0 : callee_nm->entry_point(); // used below @@ -1258,15 +1267,24 @@ methodHandle SharedRuntime::resolve_sub_helper(JavaThread *thread, { MutexLocker ml_patch(CompiledIC_lock); + // Lock blocks for safepoint during which both nmethods can change state. + // Now that we are ready to patch if the Method* was redefined then // don't update call site and let the caller retry. - - if (!callee_method->is_old()) { + // Don't update call site if caller nmethod has been made non-entrant + // as it is a waste of time. + // Don't update call site if callee nmethod was unloaded or deoptimized. + // Don't update call site if callee nmethod was replaced by an other nmethod + // which may happen when multiply alive nmethod (tiered compilation) + // will be supported. + if (!callee_method->is_old() && caller_nm->is_in_use() && + (callee_nm == NULL || callee_nm->is_in_use() && (callee_method->code() == callee_nm))) { #ifdef ASSERT // We must not try to patch to jump to an already unloaded method. if (dest_entry_point != 0) { - assert(CodeCache::find_blob(dest_entry_point) != NULL, - "should not unload nmethod while locked"); + CodeBlob* cb = CodeCache::find_blob(dest_entry_point); + assert((cb != NULL) && cb->is_nmethod() && (((nmethod*)cb) == callee_nm), + "should not call unloaded nmethod"); } #endif if (is_virtual) { -- GitLab