8217309: ZGC: Fix ZNMethodTable corruption
authorpliden
Thu, 24 Jan 2019 12:23:01 +0100
changeset 53500 f618cfbaf35b
parent 53499 ca6f11c5acda
child 53501 6533b2b34593
8217309: ZGC: Fix ZNMethodTable corruption Reviewed-by: eosterlund, stefank
src/hotspot/share/code/nmethod.cpp
src/hotspot/share/gc/z/zNMethodTable.cpp
src/hotspot/share/gc/z/zNMethodTable.hpp
--- a/src/hotspot/share/code/nmethod.cpp	Wed Jan 23 23:24:10 2019 -0800
+++ b/src/hotspot/share/code/nmethod.cpp	Thu Jan 24 12:23:01 2019 +0100
@@ -1100,7 +1100,11 @@
          "must be at safepoint");
 
   // Unregister must be done before the state change
-  Universe::heap()->unregister_nmethod(this);
+  {
+    MutexLockerEx ml(SafepointSynchronize::is_at_safepoint() ? NULL : CodeCache_lock,
+                     Mutex::_no_safepoint_check_flag);
+    Universe::heap()->unregister_nmethod(this);
+  }
 
   // Log the unloading.
   log_state_change();
--- a/src/hotspot/share/gc/z/zNMethodTable.cpp	Wed Jan 23 23:24:10 2019 -0800
+++ b/src/hotspot/share/gc/z/zNMethodTable.cpp	Thu Jan 24 12:23:01 2019 +0100
@@ -264,21 +264,17 @@
   }
 }
 
-bool ZNMethodTable::unregister_entry(ZNMethodTableEntry* table, size_t size, nmethod* nm) {
+void ZNMethodTable::unregister_entry(ZNMethodTableEntry* table, size_t size, nmethod* nm) {
   if (size == 0) {
     // Table is empty
-    return false;
+    return;
   }
 
   size_t index = first_index(nm, size);
 
   for (;;) {
     const ZNMethodTableEntry table_entry = table[index];
-
-    if (!table_entry.registered() && !table_entry.unregistered()) {
-      // Entry not found
-      return false;
-    }
+    assert(table_entry.registered() || table_entry.unregistered(), "Entry not found");
 
     if (table_entry.registered() && table_entry.method() == nm) {
       // Remove entry
@@ -287,7 +283,7 @@
       // Destroy GC data
       ZNMethodData::destroy(gc_data(nm));
       set_gc_data(nm, NULL);
-      return true;
+      return;
     }
 
     index = next_index(index, size);
@@ -451,8 +447,6 @@
     return;
   }
 
-  assert(CodeCache_lock->owned_by_self(), "Lock must be held");
-
   while (_iter_table != NULL) {
     MutexUnlockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
     os::naked_short_sleep(1);
@@ -460,6 +454,7 @@
 }
 
 void ZNMethodTable::unregister_nmethod(nmethod* nm) {
+  assert(CodeCache_lock->owned_by_self(), "Lock must be held");
   ResourceMark rm;
 
   sweeper_wait_for_iteration();
@@ -467,14 +462,9 @@
   log_unregister(nm);
 
   // Remove entry
-  if (unregister_entry(_table, _size, nm)) {
-    // Entry was unregistered. When unregister_entry() instead returns
-    // false the nmethod was not in the table (because it didn't have
-    // any oops) so we do not want to decrease the number of registered
-    // entries in that case.
-    _nregistered--;
-    _nunregistered++;
-  }
+  unregister_entry(_table, _size, nm);
+  _nunregistered++;
+  _nregistered--;
 }
 
 void ZNMethodTable::disarm_nmethod(nmethod* nm) {
--- a/src/hotspot/share/gc/z/zNMethodTable.hpp	Wed Jan 23 23:24:10 2019 -0800
+++ b/src/hotspot/share/gc/z/zNMethodTable.hpp	Thu Jan 24 12:23:01 2019 +0100
@@ -57,7 +57,7 @@
   static void sweeper_wait_for_iteration();
 
   static bool register_entry(ZNMethodTableEntry* table, size_t size, ZNMethodTableEntry entry);
-  static bool unregister_entry(ZNMethodTableEntry* table, size_t size, nmethod* nm);
+  static void unregister_entry(ZNMethodTableEntry* table, size_t size, nmethod* nm);
 
   static void rebuild(size_t new_size);
   static void rebuild_if_needed();