jdk-sandbox: comparison src/hotspot/share/code/nmethod.cpp

equal deleted inserted replaced

-:388c36110e88
+:7826a2a06f87
 if (mdo == NULL)  return;
 // There is a benign race here.  See comments in methodData.hpp.
 mdo->inc_decompile_count();
 }
+bool nmethod::try_transition(int new_state_int) {
+signed char new_state = new_state_int;
+for (;;) {
+signed char old_state = Atomic::load(&_state);
+if (old_state >= new_state) {
+// Ensure monotonicity of transitions.
+return false;
+}
+if (Atomic::cmpxchg(new_state, &_state, old_state) == old_state) {
+return true;
+}
+}
+}
 void nmethod::make_unloaded() {
 post_compiled_method_unload();
 // This nmethod is being unloaded, make sure that dependencies
 // recorded in instanceKlasses get flushed.
 p2i(this), p2i(_method));
 ls.cr();
 }
 // Unlink the osr method, so we do not look this up again
 if (is_osr_method()) {
-// Invalidate the osr nmethod only once
+// Invalidate the osr nmethod only once. Note that with concurrent
+// code cache unloading, OSR nmethods are invalidated before they
+// are made unloaded. Therefore, this becomes a no-op then.
 if (is_in_use()) {
 invalidate_osr_method();
 }
 #ifdef ASSERT
 if (method() != NULL) {
 assert(_method == NULL, "Tautology");
 set_osr_link(NULL);
 NMethodSweeper::report_state_change(this);
-// The release is only needed for compile-time ordering, as accesses
+bool transition_success = try_transition(unloaded);
-// into the nmethod after the store are not safe due to the sweeper
-// being allowed to free it when the store is observed, during
+// It is an important invariant that there exists no race between
-// concurrent nmethod unloading. Therefore, there is no need for
+// the sweeper and GC thread competing for making the same nmethod
-// acquire on the loader side.
+// zombie and unloaded respectively. This is ensured by
-OrderAccess::release_store(&_state, (signed char)unloaded);
+// can_convert_to_zombie() returning false for any is_unloading()
+// nmethod, informing the sweeper not to step on any GC toes.
+assert(transition_success, "Invalid nmethod transition to unloaded");
 #if INCLUDE_JVMCI
 // Clear the link between this nmethod and a HotSpotNmethod mirror
 JVMCINMethodData* nmethod_data = jvmci_nmethod_data();
 if (nmethod_data != NULL) {
 */
 bool nmethod::make_not_entrant_or_zombie(int state) {
 assert(state == zombie || state == not_entrant, "must be zombie or not_entrant");
 assert(!is_zombie(), "should not already be a zombie");
-if (_state == state) {
+if (Atomic::load(&_state) >= state) {
 // Avoid taking the lock if already in required state.
 // This is safe from races because the state is an end-state,
 // which the nmethod cannot back out of once entered.
 // No need for fencing either.
 return false;
 }
 // Enter critical section.  Does not block for safepoint.
 MutexLocker pl(Patching_lock, Mutex::_no_safepoint_check_flag);
-if (_state == state) {
+if (Atomic::load(&_state) >= state) {
 // another thread already performed this transition so nothing
 // to do, but return false to indicate this.
 return false;
 }
 mark_as_seen_on_stack();
 OrderAccess::storestore(); // _stack_traversal_mark and _state
 }
 // Change state
-_state = state;
+if (!try_transition(state)) {
+// If the transition fails, it is due to another thread making the nmethod more
+// dead. In particular, one thread might be making the nmethod unloaded concurrently.
+// If so, having patched in the jump in the verified entry unnecessarily is fine.
+// The nmethod is no longer possible to call by Java threads.
+// Incrementing the decompile count is also fine as the caller of make_not_entrant()
+// had a valid reason to deoptimize the nmethod.
+// Marking the nmethod as seen on stack also has no effect, as the nmethod is now
+// !is_alive(), and the seen on stack value is only used to convert not_entrant
+// nmethods to zombie in can_convert_to_zombie().
+return false;
+}
 // Log the transition once
 log_state_change();
 // Remove nmethod from method.

changeset 57490	7826a2a06f87
parent 55562	2f464d628942
child 57603	f9d9bed12d1a