8216541: CompiledICHolders of VM locked unloaded nmethods are released too late
Reviewed-by: kvn, thartmann
diff --git a/src/hotspot/share/code/compiledIC.cpp b/src/hotspot/share/code/compiledIC.cpp
index 1f0b441..3a39b6e 100644
--- a/src/hotspot/share/code/compiledIC.cpp
+++ b/src/hotspot/share/code/compiledIC.cpp
@@ -539,17 +539,6 @@
return is_icholder_entry(dest);
}
-// Release the CompiledICHolder* associated with this call site is there is one.
-void CompiledIC::cleanup_call_site(virtual_call_Relocation* call_site, const CompiledMethod* cm) {
- assert(cm->is_nmethod(), "must be nmethod");
- // This call site might have become stale so inspect it carefully.
- NativeCall* call = nativeCall_at(call_site->addr());
- if (is_icholder_entry(call->destination())) {
- NativeMovConstReg* value = nativeMovConstReg_at(call_site->cached_value());
- InlineCacheBuffer::queue_for_release((CompiledICHolder*)value->data());
- }
-}
-
// ----------------------------------------------------------------------------
void CompiledStaticCall::set_to_clean(bool in_use) {
diff --git a/src/hotspot/share/code/compiledIC.hpp b/src/hotspot/share/code/compiledIC.hpp
index 09790ef..8cab521 100644
--- a/src/hotspot/share/code/compiledIC.hpp
+++ b/src/hotspot/share/code/compiledIC.hpp
@@ -210,10 +210,6 @@
friend CompiledIC* CompiledIC_at(Relocation* call_site);
friend CompiledIC* CompiledIC_at(RelocIterator* reloc_iter);
- // This is used to release CompiledICHolder*s from nmethods that
- // are about to be freed. The callsite might contain other stale
- // values of other kinds so it must be careful.
- static void cleanup_call_site(virtual_call_Relocation* call_site, const CompiledMethod* cm);
static bool is_icholder_call_site(virtual_call_Relocation* call_site, const CompiledMethod* cm);
// Return the cached_metadata/destination associated with this inline cache. If the cache currently points
diff --git a/src/hotspot/share/code/compiledMethod.cpp b/src/hotspot/share/code/compiledMethod.cpp
index 1d189a7..7838d24 100644
--- a/src/hotspot/share/code/compiledMethod.cpp
+++ b/src/hotspot/share/code/compiledMethod.cpp
@@ -322,15 +322,16 @@
}
}
-// Clear ICStubs of all compiled ICs
-void CompiledMethod::clear_ic_stubs() {
+// Clear IC callsites, releasing ICStubs of all compiled ICs
+// as well as any associated CompiledICHolders.
+void CompiledMethod::clear_ic_callsites() {
assert_locked_or_safepoint(CompiledIC_lock);
ResourceMark rm;
RelocIterator iter(this);
while(iter.next()) {
if (iter.type() == relocInfo::virtual_call_type) {
CompiledIC* ic = CompiledIC_at(&iter);
- ic->clear_ic_stub();
+ ic->set_to_clean(false);
}
}
}
diff --git a/src/hotspot/share/code/compiledMethod.hpp b/src/hotspot/share/code/compiledMethod.hpp
index e6c2815..1dd185d 100644
--- a/src/hotspot/share/code/compiledMethod.hpp
+++ b/src/hotspot/share/code/compiledMethod.hpp
@@ -347,7 +347,7 @@
}
virtual void clear_inline_caches();
- void clear_ic_stubs();
+ void clear_ic_callsites();
// Verify and count cached icholder relocations.
int verify_icholder_relocations();
diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp
index 5dcf67d..3cb1138 100644
--- a/src/hotspot/share/code/nmethod.cpp
+++ b/src/hotspot/share/code/nmethod.cpp
@@ -1076,6 +1076,9 @@
CodeCache::set_needs_cache_clean(true);
}
+ // Clear ICStubs and release any CompiledICHolders.
+ clear_ic_callsites();
+
// Unregister must be done before the state change
Universe::heap()->unregister_nmethod(this);
@@ -1286,6 +1289,7 @@
}
void nmethod::flush() {
+ MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
// Note that there are no valid oops in the nmethod anymore.
assert(!is_osr_method() || is_unloaded() || is_zombie(),
"osr nmethod must be unloaded or zombie before flushing");
diff --git a/src/hotspot/share/runtime/sweeper.cpp b/src/hotspot/share/runtime/sweeper.cpp
index 0f9acbe..21404db 100644
--- a/src/hotspot/share/runtime/sweeper.cpp
+++ b/src/hotspot/share/runtime/sweeper.cpp
@@ -591,27 +591,6 @@
}
};
-void NMethodSweeper::release_compiled_method(CompiledMethod* nm) {
- // Make sure the released nmethod is no longer referenced by the sweeper thread
- CodeCacheSweeperThread* thread = (CodeCacheSweeperThread*)JavaThread::current();
- thread->set_scanned_compiled_method(NULL);
-
- // Clean up any CompiledICHolders
- {
- ResourceMark rm;
- MutexLocker ml_patch(CompiledIC_lock);
- RelocIterator iter(nm);
- while (iter.next()) {
- if (iter.type() == relocInfo::virtual_call_type) {
- CompiledIC::cleanup_call_site(iter.virtual_call_reloc(), nm);
- }
- }
- }
-
- MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
- nm->flush();
-}
-
NMethodSweeper::MethodStateChange NMethodSweeper::process_compiled_method(CompiledMethod* cm) {
assert(cm != NULL, "sanity");
assert(!CodeCache_lock->owned_by_self(), "just checking");
@@ -638,7 +617,7 @@
// All inline caches that referred to this nmethod were cleaned in the
// previous sweeper cycle. Now flush the nmethod from the code cache.
assert(!cm->is_locked_by_vm(), "must not flush locked Compiled Methods");
- release_compiled_method(cm);
+ cm->flush();
assert(result == None, "sanity");
result = Flushed;
} else if (cm->is_not_entrant()) {
@@ -650,7 +629,7 @@
// nmethods during the next safepoint (see ICStub::finalize).
{
MutexLocker cl(CompiledIC_lock);
- cm->clear_ic_stubs();
+ cm->clear_ic_callsites();
}
// Code cache state change is tracked in make_zombie()
cm->make_zombie();
@@ -663,7 +642,7 @@
// Make sure that we unregistered the nmethod with the heap and flushed all
// dependencies before removing the nmethod (done in make_zombie()).
assert(cm->is_zombie(), "nmethod must be unregistered");
- release_compiled_method(cm);
+ cm->flush();
assert(result == None, "sanity");
result = Flushed;
} else {
@@ -680,16 +659,10 @@
} else if (cm->is_unloaded()) {
// Code is unloaded, so there are no activations on the stack.
// Convert the nmethod to zombie or flush it directly in the OSR case.
- {
- // Clean ICs of unloaded nmethods as well because they may reference other
- // unloaded nmethods that may be flushed earlier in the sweeper cycle.
- MutexLocker cl(CompiledIC_lock);
- cm->cleanup_inline_caches();
- }
if (cm->is_osr_method()) {
SWEEP(cm);
// No inline caches will ever point to osr methods, so we can just remove it
- release_compiled_method(cm);
+ cm->flush();
assert(result == None, "sanity");
result = Flushed;
} else {
diff --git a/src/hotspot/share/runtime/sweeper.hpp b/src/hotspot/share/runtime/sweeper.hpp
index 855a050..5383b05 100644
--- a/src/hotspot/share/runtime/sweeper.hpp
+++ b/src/hotspot/share/runtime/sweeper.hpp
@@ -91,7 +91,6 @@
static Monitor* _stat_lock;
static MethodStateChange process_compiled_method(CompiledMethod *nm);
- static void release_compiled_method(CompiledMethod* nm);
static void init_sweeper_log() NOT_DEBUG_RETURN;
static bool wait_for_stack_scanning();