[GR-20027] Be more careful when getting nmethods from InstalledCode i…

…nstances.

PullRequest: labsjdk-ce-11/42

src/hotspot/share/code/nmethod.cpp

@@ -1252,7 +1252,7 @@ bool nmethod::make_not_entrant_or_zombie(int state) {
  }
 
  // Must happen before state change. Otherwise we have a race condition in
- // nmethod::can_not_entrant_be_converted(). I.e., a method can immediately
+ // nmethod::can_convert_to_zombie(). I.e., a method can immediately
  // transition its state from 'not_entrant' to 'zombie' without having to wait
  // for stack scanning.
  if (state == not_entrant) {

src/hotspot/share/code/nmethod.hpp

@@ -627,9 +627,9 @@ class nmethodLocker : public StackObj {
  lock(_nm);
  }
 
- static void lock(CompiledMethod* method) {
+ static void lock(CompiledMethod* method, bool zombie_ok = false) {
  if (method == NULL) return;
- lock_nmethod(method);
+ lock_nmethod(method, zombie_ok);
  }
 
  static void unlock(CompiledMethod* method) {
@@ -643,10 +643,10 @@ class nmethodLocker : public StackObj {
  }
 
  CompiledMethod* code() { return _nm; }
- void set_code(CompiledMethod* new_nm) {
+ void set_code(CompiledMethod* new_nm, bool zombie_ok = false) {
  unlock(_nm); // note: This works even if _nm==new_nm.
  _nm = new_nm;
- lock(_nm);
+ lock(_nm, zombie_ok);
  }
 };
 

src/hotspot/share/jvmci/jniAccessMark.inline.hpp

@@ -25,6 +25,7 @@
 #ifndef SHARE_JVMCI_JNIACCESSMARK_INLINE_HPP
 #define SHARE_JVMCI_JNIACCESSMARK_INLINE_HPP
 
+#include "code/nmethod.hpp"
 #include "jvmci/jvmciEnv.hpp"
 #include "runtime/interfaceSupport.inline.hpp"
 

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -973,7 +973,8 @@ C2V_VMENTRY_NULL(jobject, disassembleCodeBlob, (JNIEnv* env, jobject, jobject in
  }
 
  JVMCIObject installedCodeObject = JVMCIENV->wrap(installedCode);
- CodeBlob* cb = JVMCIENV->asCodeBlob(installedCodeObject);
+ nmethodLocker locker;
+ CodeBlob* cb = JVMCIENV->get_code_blob(installedCodeObject, locker);
  if (cb == NULL) {
  return NULL;
  }
@@ -1018,7 +1019,8 @@ C2V_VMENTRY_NULL(jobject, executeHotSpotNmethod, (JNIEnv* env, jobject, jobject
  HandleMark hm;
 
  JVMCIObject nmethod_mirror = JVMCIENV->wrap(hs_nmethod);
- nmethod* nm = JVMCIENV->asNmethod(nmethod_mirror);
+ nmethodLocker locker;
+ nmethod* nm = JVMCIENV->get_nmethod(nmethod_mirror, locker);
  if (nm == NULL) {
  JVMCI_THROW_NULL(InvalidInstalledCodeException);
  }
@@ -2439,7 +2441,8 @@ C2V_VMENTRY_0(jlong, translate, (JNIEnv* env, jobject, jobject obj_handle))
  Handle constant = thisEnv->asConstant(obj, JVMCI_CHECK_0);
  result = peerEnv->get_object_constant(constant());
  } else if (thisEnv->isa_HotSpotNmethod(obj)) {
- nmethod* nm = thisEnv->asNmethod(obj);
+ nmethodLocker locker;
+ nmethod* nm = JVMCIENV->get_nmethod(obj, locker);
  if (nm != NULL) {
  JVMCINMethodData* data = nm->jvmci_nmethod_data();
  if (data != NULL) {
@@ -2504,12 +2507,14 @@ C2V_VMENTRY_NULL(jobject, unhand, (JNIEnv* env, jobject, jlong obj_handle))
 C2V_VMENTRY(void, updateHotSpotNmethod, (JNIEnv* env, jobject, jobject code_handle))
  JVMCIObject code = JVMCIENV->wrap(code_handle);
  // Execute this operation for the side effect of updating the InstalledCode state
- JVMCIENV->asNmethod(code);
+ nmethodLocker locker;
+ JVMCIENV->get_nmethod(code, locker);
 }
 
 C2V_VMENTRY_NULL(jbyteArray, getCode, (JNIEnv* env, jobject, jobject code_handle))
  JVMCIObject code = JVMCIENV->wrap(code_handle);
- CodeBlob* cb = JVMCIENV->asCodeBlob(code);
+ nmethodLocker locker;
+ CodeBlob* cb = JVMCIENV->get_code_blob(code, locker);
  if (cb == NULL) {
  return NULL;
  }

src/hotspot/share/jvmci/jvmciEnv.cpp

@@ -1469,8 +1469,8 @@ void JVMCIEnv::invalidate_nmethod_mirror(JVMCIObject mirror, JVMCI_TRAPS) {
  JVMCI_THROW(NullPointerException);
  }
 
- jlong nativeMethod = get_InstalledCode_address(mirror);
- nmethod* nm = JVMCIENV->asNmethod(mirror);
+ nmethodLocker locker;
+ nmethod* nm = JVMCIENV->get_nmethod(mirror, locker);
  if (nm == NULL) {
  // Nothing to do
  return;
@@ -1512,45 +1512,81 @@ ConstantPool* JVMCIEnv::asConstantPool(JVMCIObject obj) {
  return *metadataHandle;
 }
 
-
-CodeBlob* JVMCIEnv::asCodeBlob(JVMCIObject obj) {
+CodeBlob* JVMCIEnv::get_code_blob(JVMCIObject obj, nmethodLocker& locker) {
  address code = (address) get_InstalledCode_address(obj);
  if (code == NULL) {
  return NULL;
  }
  if (isa_HotSpotNmethod(obj)) {
- jlong compile_id_snapshot = get_HotSpotNmethod_compileIdSnapshot(obj);
- if (compile_id_snapshot != 0L) {
- // A HotSpotNMethod not in an nmethod's oops table so look up
- // the nmethod and then update the fields based on its state.
+ nmethod* nm = NULL;
+ {
+ // Lookup the CodeBlob while holding the CodeCache_lock to ensure the nmethod can't be freed
+ // by nmethod::flush while we're interrogating it.
+ MutexLockerEx cm_lock(CodeCache_lock, Mutex::_no_safepoint_check_flag);
  CodeBlob* cb = CodeCache::find_blob_unsafe(code);
  if (cb == (CodeBlob*) code) {
- // Found a live CodeBlob with the same address, make sure it's the same nmethod
- nmethod* nm = cb->as_nmethod_or_null();
- if (nm != NULL && nm->compile_id() == compile_id_snapshot) {
- if (!nm->is_alive()) {
- // Break the links from the mirror to the nmethod
- set_InstalledCode_address(obj, 0);
- set_InstalledCode_entryPoint(obj, 0);
- } else if (nm->is_not_entrant()) {
- // Zero the entry point so that the nmethod
- // cannot be invoked by the mirror but can
- // still be deoptimized.
- set_InstalledCode_entryPoint(obj, 0);
- }
- return cb;
+ nmethod* the_nm = cb->as_nmethod_or_null();
+ if (the_nm != NULL && the_nm->is_alive()) {
+ // Lock the nmethod to stop any further transitions by the sweeper. It's still possible
+ // for this code to execute in the middle of the sweeping of the nmethod but that will be
+ // handled below.
+ locker.set_code(nm, true);
+ nm = the_nm;
  }
  }
- // Clear the InstalledCode fields of this HotSpotNmethod
- // that no longer refers to an nmethod in the code cache.
+ }
+
+ if (nm != NULL) {
+ // We found the nmethod but it could be in the process of being freed. Check the state of the
+ // nmethod while holding the Patching_lock. This ensures that any transitions by other
+ // threads have seen the is_locked_by_vm() update above.
+ MutexLockerEx cm_lock(Patching_lock, Mutex::_no_safepoint_check_flag);
+ if (!nm->is_alive()) {
+ // It was alive when we looked it up but it's no longer alive so release it.
+ locker.set_code(NULL);
+ nm = NULL;
+ }
+ }
+
+ jlong compile_id_snapshot = get_HotSpotNmethod_compileIdSnapshot(obj);
+ if (compile_id_snapshot != 0L) {
+ // Found a live nmethod with the same address, make sure it's the same nmethod
+ if (nm == (nmethod*) code && nm->compile_id() == compile_id_snapshot && nm->is_alive()) {
+ if (nm->is_not_entrant()) {
+ // Zero the entry point so that the nmethod
+ // cannot be invoked by the mirror but can
+ // still be deoptimized.
+ set_InstalledCode_entryPoint(obj, 0);
+ }
+ return nm;
+ }
+ // The HotSpotNmethod no longer refers to a valid nmethod so clear the state
+ locker.set_code(NULL);
+ nm = NULL;
+ }
+
+ if (nm == NULL) {
+ // The HotSpotNmethod was pointing at some nmethod but the nmethod is no longer valid, so
+ // clear the InstalledCode fields of this HotSpotNmethod so that it no longer refers to a
+ // nmethod in the code cache.
  set_InstalledCode_address(obj, 0);
  set_InstalledCode_entryPoint(obj, 0);
- return NULL;
  }
+ return nm;
  }
- return (CodeBlob*) code;
+
+ CodeBlob* cb = (CodeBlob*) code;
+ assert(!cb->is_nmethod(), "unexpected nmethod");
+ return cb;
 }
 
+nmethod* JVMCIEnv::get_nmethod(JVMCIObject obj, nmethodLocker& locker) {
+ CodeBlob* cb = get_code_blob(obj, locker);
+ if (cb != NULL) {
+ return cb->as_nmethod_or_null();
+ }
+ return NULL;
+}
 
 // Generate implementations for the initialize, new, isa, get and set methods for all the types and
 // fields declared in the JVMCI_CLASSES_DO macro.

src/hotspot/share/jvmci/jvmciEnv.hpp

@@ -324,18 +324,13 @@ class JVMCIEnv : public ResourceObj {
 
  void fthrow_error(const char* file, int line, const char* format, ...) ATTRIBUTE_PRINTF(4, 5);
 
- // Given an instance of HotSpotInstalledCode return the corresponding CodeBlob*
- CodeBlob* asCodeBlob(JVMCIObject code);
-
- nmethod* asNmethod(JVMCIObject code) {
- CodeBlob* cb = asCodeBlob(code);
- if (cb == NULL) {
- return NULL;
- }
- nmethod* nm = cb->as_nmethod_or_null();
- guarantee(nm != NULL, "not an nmethod");
- return nm;
- }
+ // Given an instance of HotSpotInstalledCode return the corresponding CodeBlob*. The
+ // nmethodLocker is required to keep the CodeBlob alive in the case where it's an nmethod.
+ CodeBlob* get_code_blob(JVMCIObject code, nmethodLocker& locker);
+
+ // Given an instance of HotSpotInstalledCode return the corresponding nmethod. The
+ // nmethodLocker is required to keep the nmethod alive.
+ nmethod* get_nmethod(JVMCIObject code, nmethodLocker& locker);
 
  MethodData* asMethodData(jlong metaspaceMethodData) {
  return (MethodData*) (address) metaspaceMethodData;