8216541: CompiledICHolders of VM locked unloaded nmethods are released too late
Reviewed-by: kvn, thartmann
--- a/src/hotspot/share/code/compiledIC.cpp Mon Feb 04 17:53:26 2019 -0800
+++ b/src/hotspot/share/code/compiledIC.cpp Tue Feb 05 20:06:10 2019 +0100
@@ -51,7 +51,8 @@
CompiledICLocker::CompiledICLocker(CompiledMethod* method)
: _method(method),
_behaviour(CompiledICProtectionBehaviour::current()),
- _locked(_behaviour->lock(_method)){
+ _locked(_behaviour->lock(_method)),
+ _nsv(true, !SafepointSynchronize::is_at_safepoint()) {
}
CompiledICLocker::~CompiledICLocker() {
@@ -583,17 +584,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());
- }
-}
-
// ----------------------------------------------------------------------------
bool CompiledStaticCall::set_to_clean(bool in_use) {
--- a/src/hotspot/share/code/compiledIC.hpp Mon Feb 04 17:53:26 2019 -0800
+++ b/src/hotspot/share/code/compiledIC.hpp Tue Feb 05 20:06:10 2019 +0100
@@ -226,10 +226,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
--- a/src/hotspot/share/code/compiledMethod.cpp Mon Feb 04 17:53:26 2019 -0800
+++ b/src/hotspot/share/code/compiledMethod.cpp Tue Feb 05 20:06:10 2019 +0100
@@ -399,15 +399,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(CompiledICLocker::is_safe(this), "mt unsafe call");
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);
}
}
}
--- a/src/hotspot/share/code/compiledMethod.hpp Mon Feb 04 17:53:26 2019 -0800
+++ b/src/hotspot/share/code/compiledMethod.hpp Tue Feb 05 20:06:10 2019 +0100
@@ -359,7 +359,7 @@
void cleanup_inline_caches(bool clean_all);
virtual void clear_inline_caches();
- void clear_ic_stubs();
+ void clear_ic_callsites();
// Verify and count cached icholder relocations.
int verify_icholder_relocations();
--- a/src/hotspot/share/code/nmethod.cpp Mon Feb 04 17:53:26 2019 -0800
+++ b/src/hotspot/share/code/nmethod.cpp Tue Feb 05 20:06:10 2019 +0100
@@ -1099,6 +1099,12 @@
assert(SafepointSynchronize::is_at_safepoint() || Thread::current()->is_ConcurrentGC_thread(),
"must be at safepoint");
+ {
+ // Clear ICStubs and release any CompiledICHolders.
+ CompiledICLocker ml(this);
+ clear_ic_callsites();
+ }
+
// Unregister must be done before the state change
{
MutexLockerEx ml(SafepointSynchronize::is_at_safepoint() ? NULL : CodeCache_lock,
@@ -1291,10 +1297,11 @@
}
// Clear ICStubs to prevent back patching stubs of zombie or flushed
- // nmethods during the next safepoint (see ICStub::finalize).
+ // nmethods during the next safepoint (see ICStub::finalize), as well
+ // as to free up CompiledICHolder resources.
{
CompiledICLocker ml(this);
- clear_ic_stubs();
+ clear_ic_callsites();
}
// zombie only - if a JVMTI agent has enabled the CompiledMethodUnload
@@ -1326,6 +1333,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");
--- a/src/hotspot/share/runtime/sweeper.cpp Mon Feb 04 17:53:26 2019 -0800
+++ b/src/hotspot/share/runtime/sweeper.cpp Tue Feb 05 20:06:10 2019 +0100
@@ -663,27 +663,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;
- RelocIterator iter(nm);
- CompiledICLocker ml(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");
@@ -697,7 +676,7 @@
// Skip methods that are currently referenced by the VM
if (cm->is_locked_by_vm()) {
// But still remember to clean-up inline caches for alive nmethods
- if (cm->is_alive() && !cm->is_unloading()) {
+ if (cm->is_alive()) {
// Clean inline caches that point to zombie/non-entrant/unloaded nmethods
cm->cleanup_inline_caches(false);
SWEEP(cm);
@@ -709,7 +688,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()) {
@@ -728,7 +707,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 {
@@ -744,14 +723,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.
- cm->cleanup_inline_caches(false);
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 {
--- a/src/hotspot/share/runtime/sweeper.hpp Mon Feb 04 17:53:26 2019 -0800
+++ b/src/hotspot/share/runtime/sweeper.hpp Tue Feb 05 20:06:10 2019 +0100
@@ -89,7 +89,6 @@
static Tickspan _peak_sweep_fraction_time; // Peak time sweeping one fraction
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();