8023037: Race between ciEnv::register_method and nmethod::make_not_entrant_or_zombie

Reviewed-by: kvn, iveresov

src/cpu/sparc/vm/sharedRuntime_sparc.cpp

@@ -1002,18 +1002,6 @@ void AdapterGenerator::gen_i2c_adapter(
   // and the vm will find there should this case occur.
   Address callee_target_addr(G2_thread, JavaThread::callee_target_offset());
   __ st_ptr(G5_method, callee_target_addr);
-
-  if (StressNonEntrant) {
-    // Open a big window for deopt failure
-    __ save_frame(0);
-    __ mov(G0, L0);
-    Label loop;
-    __ bind(loop);
-    __ sub(L0, 1, L0);
-    __ br_null_short(L0, Assembler::pt, loop);
-    __ restore();
-  }
-
   __ jmpl(G3, 0, G0);
   __ delayed()->nop();
 }

src/share/vm/ci/ciEnv.cpp

@@ -935,7 +935,9 @@ void ciEnv::register_method(ciMethod* target,
 
     // Prevent SystemDictionary::add_to_hierarchy from running
     // and invalidating our dependencies until we install this method.
+    // No safepoints are allowed. Otherwise, class redefinition can occur in between.
     MutexLocker ml(Compile_lock);
+    No_Safepoint_Verifier nsv;
 
     // Change in Jvmti state may invalidate compilation.
     if (!failing() &&
@@ -1001,16 +1003,6 @@ void ciEnv::register_method(ciMethod* target,
     // Free codeBlobs
     code_buffer->free_blob();
 
-    // stress test 6243940 by immediately making the method
-    // non-entrant behind the system's back. This has serious
-    // side effects on the code cache and is not meant for
-    // general stress testing
-    if (nm != NULL && StressNonEntrant) {
-      MutexLockerEx pl(Patching_lock, Mutex::_no_safepoint_check_flag);
-      NativeJump::patch_verified_entry(nm->entry_point(), nm->verified_entry_point(),
-                  SharedRuntime::get_handle_wrong_method_stub());
-    }
-
     if (nm == NULL) {
       // The CodeCache is full.  Print out warning and disable compilation.
       record_failure("code cache is full");
@@ -1036,11 +1028,11 @@ void ciEnv::register_method(ciMethod* target,
             char *method_name = method->name_and_sig_as_C_string();
             tty->print_cr("Replacing method %s", method_name);
           }
-          if (old != NULL ) {
+          if (old != NULL) {
             old->make_not_entrant();
           }
         }
-        if (TraceNMethodInstalls ) {
+        if (TraceNMethodInstalls) {
           ResourceMark rm;
           char *method_name = method->name_and_sig_as_C_string();
           ttyLocker ttyl;
@@ -1051,7 +1043,7 @@ void ciEnv::register_method(ciMethod* target,
         // Allow the code to be executed
         method->set_code(method, nm);
       } else {
-        if (TraceNMethodInstalls ) {
+        if (TraceNMethodInstalls) {
           ResourceMark rm;
           char *method_name = method->name_and_sig_as_C_string();
           ttyLocker ttyl;
@@ -1061,7 +1053,6 @@ void ciEnv::register_method(ciMethod* target,
                         entry_bci);
         }
         method->method_holder()->add_osr_nmethod(nm);
-
       }
     }
   }

src/share/vm/code/nmethod.cpp

@@ -618,21 +618,18 @@ nmethod* nmethod::new_nmethod(methodHandle method,
         // record this nmethod as dependent on this klass
         InstanceKlass::cast(klass)->add_dependent_nmethod(nm);
       }
-    }
-    NOT_PRODUCT(if (nm != NULL)  nmethod_stats.note_nmethod(nm));
-    if (PrintAssembly && nm != NULL) {
-      Disassembler::decode(nm);
+      NOT_PRODUCT(nmethod_stats.note_nmethod(nm));
+      if (PrintAssembly) {
+        Disassembler::decode(nm);
+      }
     }
   }
-
-  // verify nmethod
-  debug_only(if (nm) nm->verify();) // might block
-
+  // Do verification and logging outside CodeCache_lock.
   if (nm != NULL) {
+    // Safepoints in nmethod::verify aren't allowed because nm hasn't been installed yet.
+    DEBUG_ONLY(nm->verify();)
     nm->log_new_nmethod();
   }
-
-  // done
   return nm;
 }
 
@@ -2395,20 +2392,23 @@ void nmethod::verify() {
 
 
 void nmethod::verify_interrupt_point(address call_site) {
-  // This code does not work in release mode since
-  // owns_lock only is available in debug mode.
-  CompiledIC* ic = NULL;
-  Thread *cur = Thread::current();
-  if (CompiledIC_lock->owner() == cur ||
-      ((cur->is_VM_thread() || cur->is_ConcurrentGC_thread()) &&
-       SafepointSynchronize::is_at_safepoint())) {
-    ic = CompiledIC_at(this, call_site);
-    CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
-  } else {
-    MutexLocker ml_verify (CompiledIC_lock);
-    ic = CompiledIC_at(this, 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
+  if (is_installed) {
+    Thread *cur = Thread::current();
+    if (CompiledIC_lock->owner() == cur ||
+        ((cur->is_VM_thread() || cur->is_ConcurrentGC_thread()) &&
+         SafepointSynchronize::is_at_safepoint())) {
+      CompiledIC_at(this, call_site);
+      CHECK_UNHANDLED_OOPS_ONLY(Thread::current()->clear_unhandled_oops());
+    } else {
+      MutexLocker ml_verify (CompiledIC_lock);
+      CompiledIC_at(this, call_site);
+    }
   }
-  PcDesc* pd = pc_desc_at(ic->end_of_call());
+
+  PcDesc* pd = pc_desc_at(nativeCall_at(call_site)->return_address());
   assert(pd != NULL, "PcDesc must exist");
   for (ScopeDesc* sd = new ScopeDesc(this, pd->scope_decode_offset(),
                                      pd->obj_decode_offset(), pd->should_reexecute(),

src/share/vm/runtime/globals.hpp

@@ -3019,9 +3019,6 @@ class CommandLineFlags {
   notproduct(intx, ZombieALotInterval,     5,                               \
           "Number of exits until ZombieALot kicks in")                      \
                                                                             \
-  develop(bool, StressNonEntrant, false,                                    \
-          "Mark nmethods non-entrant at registration")                      \
-                                                                            \
   diagnostic(intx, MallocVerifyInterval,     0,                             \
           "If non-zero, verify C heap after every N calls to "              \
           "malloc/realloc/free")                                            \