8143571: [JVMCI] Double unregistering of nmethod during unloading
Reviewed-by: iveresov, twisti
--- a/hotspot/src/share/vm/code/nmethod.cpp Sat Dec 05 02:19:46 2015 +0000
+++ b/hotspot/src/share/vm/code/nmethod.cpp Fri Dec 04 15:18:46 2015 -1000
@@ -1348,6 +1348,9 @@
_state = unloaded;
+ // Log the unloading.
+ log_state_change();
+
#if INCLUDE_JVMCI
// The method can only be unloaded after the pointer to the installed code
// Java wrapper is no longer alive. Here we need to clear out this weak
@@ -1355,11 +1358,12 @@
// after the method is unregistered since the original value may be still
// tracked by the rset.
maybe_invalidate_installed_code();
+ // Clear these out after the nmethod has been unregistered and any
+ // updates to the InstalledCode instance have been performed.
+ _jvmci_installed_code = NULL;
+ _speculation_log = NULL;
#endif
- // Log the unloading.
- log_state_change();
-
// The Method* is gone at this point
assert(_method == NULL, "Tautology");
@@ -1470,6 +1474,9 @@
// Log the transition once
log_state_change();
+ // Invalidate while holding the patching lock
+ JVMCI_ONLY(maybe_invalidate_installed_code());
+
// Remove nmethod from method.
// We need to check if both the _code and _from_compiled_code_entry_point
// refer to this nmethod because there is a race in setting these two fields
@@ -1496,6 +1503,10 @@
MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
if (nmethod_needs_unregister) {
Universe::heap()->unregister_nmethod(this);
+#ifdef JVMCI
+ _jvmci_installed_code = NULL;
+ _speculation_log = NULL;
+#endif
}
flush_dependencies(NULL);
}
@@ -1519,8 +1530,6 @@
assert(state == not_entrant, "other cases may need to be handled differently");
}
- JVMCI_ONLY(maybe_invalidate_installed_code());
-
if (TraceCreateZombies) {
ResourceMark m;
tty->print_cr("nmethod <" INTPTR_FORMAT "> %s code made %s", p2i(this), this->method() ? this->method()->name_and_sig_as_C_string() : "null", (state == not_entrant) ? "not entrant" : "zombie");
@@ -3403,26 +3412,80 @@
#if INCLUDE_JVMCI
void nmethod::clear_jvmci_installed_code() {
- // This must be done carefully to maintain nmethod remembered sets properly
- BarrierSet* bs = Universe::heap()->barrier_set();
- bs->write_ref_nmethod_pre(&_jvmci_installed_code, this);
- _jvmci_installed_code = NULL;
- bs->write_ref_nmethod_post(&_jvmci_installed_code, this);
+ // write_ref_method_pre/post can only be safely called at a
+ // safepoint or while holding the CodeCache_lock
+ assert(CodeCache_lock->is_locked() ||
+ SafepointSynchronize::is_at_safepoint(), "should be performed under a lock for consistency");
+ if (_jvmci_installed_code != NULL) {
+ // This must be done carefully to maintain nmethod remembered sets properly
+ BarrierSet* bs = Universe::heap()->barrier_set();
+ bs->write_ref_nmethod_pre(&_jvmci_installed_code, this);
+ _jvmci_installed_code = NULL;
+ bs->write_ref_nmethod_post(&_jvmci_installed_code, this);
+ }
}
void nmethod::maybe_invalidate_installed_code() {
- if (_jvmci_installed_code != NULL) {
- if (!is_alive()) {
- // Break the link between nmethod and InstalledCode such that the nmethod
- // can subsequently be flushed safely. The link must be maintained while
- // the method could have live activations since invalidateInstalledCode
- // might want to invalidate all existing activations.
- InstalledCode::set_address(_jvmci_installed_code, 0);
- InstalledCode::set_entryPoint(_jvmci_installed_code, 0);
- clear_jvmci_installed_code();
- } else if (is_not_entrant()) {
- InstalledCode::set_entryPoint(_jvmci_installed_code, 0);
- }
+ assert(Patching_lock->is_locked() ||
+ SafepointSynchronize::is_at_safepoint(), "should be performed under a lock for consistency");
+ oop installed_code = jvmci_installed_code();
+ if (installed_code != NULL) {
+ nmethod* nm = (nmethod*)InstalledCode::address(installed_code);
+ if (nm == NULL || nm != this) {
+ // The link has been broken or the InstalledCode instance is
+ // associated with another nmethod so do nothing.
+ return;
+ }
+ if (!is_alive()) {
+ // Break the link between nmethod and InstalledCode such that the nmethod
+ // can subsequently be flushed safely. The link must be maintained while
+ // the method could have live activations since invalidateInstalledCode
+ // might want to invalidate all existing activations.
+ InstalledCode::set_address(installed_code, 0);
+ InstalledCode::set_entryPoint(installed_code, 0);
+ } else if (is_not_entrant()) {
+ // Remove the entry point so any invocation will fail but keep
+ // the address link around that so that existing activations can
+ // be invalidated.
+ InstalledCode::set_entryPoint(installed_code, 0);
+ }
+ }
+}
+
+void nmethod::invalidate_installed_code(Handle installedCode, TRAPS) {
+ if (installedCode() == NULL) {
+ THROW(vmSymbols::java_lang_NullPointerException());
+ }
+ jlong nativeMethod = InstalledCode::address(installedCode);
+ nmethod* nm = (nmethod*)nativeMethod;
+ if (nm == NULL) {
+ // Nothing to do
+ return;
+ }
+
+ nmethodLocker nml(nm);
+#ifdef ASSERT
+ {
+ MutexLockerEx pl(Patching_lock, Mutex::_no_safepoint_check_flag);
+ // This relationship can only be checked safely under a lock
+ assert(nm == NULL || !nm->is_alive() || nm->jvmci_installed_code() == installedCode(), "sanity check");
+ }
+#endif
+
+ if (nm->is_alive()) {
+ // The nmethod state machinery maintains the link between the
+ // HotSpotInstalledCode and nmethod* so as long as the nmethod appears to be
+ // alive assume there is work to do and deoptimize the nmethod.
+ nm->mark_for_deoptimization();
+ VM_Deoptimize op;
+ VMThread::execute(&op);
+ }
+
+ MutexLockerEx pl(Patching_lock, Mutex::_no_safepoint_check_flag);
+ // Check that it's still associated with the same nmethod and break
+ // the link if it is.
+ if (InstalledCode::address(installedCode) == nativeMethod) {
+ InstalledCode::set_address(installedCode, 0);
}
}
--- a/hotspot/src/share/vm/code/nmethod.hpp Sat Dec 05 02:19:46 2015 +0000
+++ b/hotspot/src/share/vm/code/nmethod.hpp Fri Dec 04 15:18:46 2015 -1000
@@ -607,10 +607,20 @@
#if INCLUDE_JVMCI
oop jvmci_installed_code() { return _jvmci_installed_code ; }
char* jvmci_installed_code_name(char* buf, size_t buflen);
- void clear_jvmci_installed_code();
+
+ // Update the state of any InstalledCode instance associated with
+ // this nmethod based on the current value of _state.
void maybe_invalidate_installed_code();
+
+ // Helper function to invalidate InstalledCode instances
+ static void invalidate_installed_code(Handle installed_code, TRAPS);
+
oop speculation_log() { return _speculation_log ; }
- void set_speculation_log(oop speculation_log) { _speculation_log = speculation_log; }
+
+ private:
+ void clear_jvmci_installed_code();
+
+ public:
#endif
// GC support
--- a/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp Sat Dec 05 02:19:46 2015 +0000
+++ b/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp Fri Dec 04 15:18:46 2015 -1000
@@ -84,24 +84,6 @@
return NULL;
}
-void CompilerToVM::invalidate_installed_code(Handle installedCode, TRAPS) {
- if (installedCode() == NULL) {
- THROW(vmSymbols::java_lang_NullPointerException());
- }
- jlong nativeMethod = InstalledCode::address(installedCode);
- nmethod* nm = (nmethod*)nativeMethod;
- assert(nm == NULL || nm->jvmci_installed_code() == installedCode(), "sanity check");
- if (nm != NULL && nm->is_alive()) {
- // The nmethod state machinery maintains the link between the
- // HotSpotInstalledCode and nmethod* so as long as the nmethod appears to be
- // alive assume there is work to do and deoptimize the nmethod.
- nm->mark_for_deoptimization();
- VM_Deoptimize op;
- VMThread::execute(&op);
- }
- InstalledCode::set_address(installedCode, 0);
-}
-
extern "C" {
extern VMStructEntry* gHotSpotVMStructs;
extern uint64_t gHotSpotVMStructEntryTypeNameOffset;
@@ -688,18 +670,22 @@
} else {
if (!installed_code_handle.is_null()) {
assert(installed_code_handle->is_a(InstalledCode::klass()), "wrong type");
- CompilerToVM::invalidate_installed_code(installed_code_handle, CHECK_0);
- InstalledCode::set_address(installed_code_handle, (jlong) cb);
- InstalledCode::set_version(installed_code_handle, InstalledCode::version(installed_code_handle) + 1);
- if (cb->is_nmethod()) {
- InstalledCode::set_entryPoint(installed_code_handle, (jlong) cb->as_nmethod_or_null()->verified_entry_point());
- } else {
- InstalledCode::set_entryPoint(installed_code_handle, (jlong) cb->code_begin());
- }
- if (installed_code_handle->is_a(HotSpotInstalledCode::klass())) {
- HotSpotInstalledCode::set_size(installed_code_handle, cb->size());
- HotSpotInstalledCode::set_codeStart(installed_code_handle, (jlong) cb->code_begin());
- HotSpotInstalledCode::set_codeSize(installed_code_handle, cb->code_size());
+ nmethod::invalidate_installed_code(installed_code_handle, CHECK_0);
+ {
+ // Ensure that all updates to the InstalledCode fields are consistent.
+ MutexLockerEx pl(Patching_lock, Mutex::_no_safepoint_check_flag);
+ InstalledCode::set_address(installed_code_handle, (jlong) cb);
+ InstalledCode::set_version(installed_code_handle, InstalledCode::version(installed_code_handle) + 1);
+ if (cb->is_nmethod()) {
+ InstalledCode::set_entryPoint(installed_code_handle, (jlong) cb->as_nmethod_or_null()->verified_entry_point());
+ } else {
+ InstalledCode::set_entryPoint(installed_code_handle, (jlong) cb->code_begin());
+ }
+ if (installed_code_handle->is_a(HotSpotInstalledCode::klass())) {
+ HotSpotInstalledCode::set_size(installed_code_handle, cb->size());
+ HotSpotInstalledCode::set_codeStart(installed_code_handle, (jlong) cb->code_begin());
+ HotSpotInstalledCode::set_codeSize(installed_code_handle, cb->code_size());
+ }
}
nmethod* nm = cb->as_nmethod_or_null();
if (nm != NULL && installed_code_handle->is_scavengable()) {
@@ -971,7 +957,7 @@
C2V_VMENTRY(void, invalidateInstalledCode, (JNIEnv*, jobject, jobject installed_code))
Handle installed_code_handle = JNIHandles::resolve(installed_code);
- CompilerToVM::invalidate_installed_code(installed_code_handle, CHECK);
+ nmethod::invalidate_installed_code(installed_code_handle, CHECK);
C2V_END
C2V_VMENTRY(jobject, readUncompressedOop, (JNIEnv*, jobject, jlong addr))
--- a/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.hpp Sat Dec 05 02:19:46 2015 +0000
+++ b/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.hpp Fri Dec 04 15:18:46 2015 -1000
@@ -97,8 +97,6 @@
static oop get_jvmci_method(const methodHandle& method, TRAPS);
static oop get_jvmci_type(KlassHandle klass, TRAPS);
-
- static void invalidate_installed_code(Handle installedCode, TRAPS);
};
class JavaArgumentUnboxer : public SignatureIterator {