# HG changeset patch
# User dcubed
# Date 1441300959 25200
# Node ID b7b2407bc7e5bfa30fc9eb569c0cda0cb8776a1a
# Parent  73bec9f941d7728c025c1e2175566bf2c8425241
8049304: race between VM_Exit and _sync_FutileWakeups->inc()
Summary: Add PerfDataManager.has_PerfData() to indicate when PerfData objects should be safe to query. Update Java monitor PerfData usage to check the new flag. PerfDataManager::destroy() should only be called at a safepoint and when the StatSampler is not active.
Reviewed-by: kbarrett, dholmes, tbenson, bdelsart

diff -r 73bec9f941d7 -r b7b2407bc7e5 hotspot/src/share/vm/runtime/objectMonitor.cpp
--- a/hotspot/src/share/vm/runtime/objectMonitor.cpp	Wed Sep 02 17:49:46 2015 -0700
+++ b/hotspot/src/share/vm/runtime/objectMonitor.cpp	Thu Sep 03 10:22:39 2015 -0700
@@ -405,9 +405,7 @@
     event.commit();
   }
 
-  if (ObjectMonitor::_sync_ContendedLockAttempts != NULL) {
-    ObjectMonitor::_sync_ContendedLockAttempts->inc();
-  }
+  OM_PERFDATA_OP(ContendedLockAttempts, inc());
 }
 
 
@@ -574,9 +572,9 @@
     // That is by design - we trade "lossy" counters which are exposed to
     // races during updates for a lower probe effect.
     TEVENT(Inflated enter - Futile wakeup);
-    if (ObjectMonitor::_sync_FutileWakeups != NULL) {
-      ObjectMonitor::_sync_FutileWakeups->inc();
-    }
+    // This PerfData object can be used in parallel with a safepoint.
+    // See the work around in PerfDataManager::destroy().
+    OM_PERFDATA_OP(FutileWakeups, inc());
     ++nWakeups;
 
     // Assuming this is not a spurious wakeup we'll normally find _succ == Self.
@@ -748,9 +746,9 @@
     // *must* retry  _owner before parking.
     OrderAccess::fence();
 
-    if (ObjectMonitor::_sync_FutileWakeups != NULL) {
-      ObjectMonitor::_sync_FutileWakeups->inc();
-    }
+    // This PerfData object can be used in parallel with a safepoint.
+    // See the work around in PerfDataManager::destroy().
+    OM_PERFDATA_OP(FutileWakeups, inc());
   }
 
   // Self has acquired the lock -- Unlink Self from the cxq or EntryList .
@@ -1302,9 +1300,7 @@
   Trigger->unpark();
 
   // Maintain stats and report events to JVMTI
-  if (ObjectMonitor::_sync_Parks != NULL) {
-    ObjectMonitor::_sync_Parks->inc();
-  }
+  OM_PERFDATA_OP(Parks, inc());
 }
 
 
@@ -1765,9 +1761,7 @@
   }
   DTRACE_MONITOR_PROBE(notify, this, object(), THREAD);
   INotify(THREAD);
-  if (ObjectMonitor::_sync_Notifications != NULL) {
-    ObjectMonitor::_sync_Notifications->inc(1);
-  }
+  OM_PERFDATA_OP(Notifications, inc(1));
 }
 
 
@@ -1792,9 +1786,7 @@
     INotify(THREAD);
   }
 
-  if (tally != 0 && ObjectMonitor::_sync_Notifications != NULL) {
-    ObjectMonitor::_sync_Notifications->inc(tally);
-  }
+  OM_PERFDATA_OP(Notifications, inc(tally));
 }
 
 // -----------------------------------------------------------------------------
diff -r 73bec9f941d7 -r b7b2407bc7e5 hotspot/src/share/vm/runtime/objectMonitor.hpp
--- a/hotspot/src/share/vm/runtime/objectMonitor.hpp	Wed Sep 02 17:49:46 2015 -0700
+++ b/hotspot/src/share/vm/runtime/objectMonitor.hpp	Thu Sep 03 10:22:39 2015 -0700
@@ -177,6 +177,19 @@
 
  public:
   static void Initialize();
+
+  // Only perform a PerfData operation if the PerfData object has been
+  // allocated and if the PerfDataManager has not freed the PerfData
+  // objects which can happen at normal VM shutdown.
+  //
+  #define OM_PERFDATA_OP(f, op_str)              \
+    do {                                         \
+      if (ObjectMonitor::_sync_ ## f != NULL &&  \
+          PerfDataManager::has_PerfData()) {     \
+        ObjectMonitor::_sync_ ## f->op_str;      \
+      }                                          \
+    } while (0)
+
   static PerfCounter * _sync_ContendedLockAttempts;
   static PerfCounter * _sync_FutileWakeups;
   static PerfCounter * _sync_Parks;
diff -r 73bec9f941d7 -r b7b2407bc7e5 hotspot/src/share/vm/runtime/perfData.cpp
--- a/hotspot/src/share/vm/runtime/perfData.cpp	Wed Sep 02 17:49:46 2015 -0700
+++ b/hotspot/src/share/vm/runtime/perfData.cpp	Thu Sep 03 10:22:39 2015 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -39,6 +39,7 @@
 PerfDataList*   PerfDataManager::_all = NULL;
 PerfDataList*   PerfDataManager::_sampled = NULL;
 PerfDataList*   PerfDataManager::_constants = NULL;
+volatile bool   PerfDataManager::_has_PerfData = 0;
 
 /*
  * The jvmstat global and subsystem jvmstat counter name spaces. The top
@@ -272,16 +273,22 @@
 }
 
 
-
-
-
-
 void PerfDataManager::destroy() {
 
   if (_all == NULL)
     // destroy already called, or initialization never happened
     return;
 
+  // Clear the flag before we free the PerfData counters. Thus begins
+  // the race between this thread and another thread that has just
+  // queried PerfDataManager::has_PerfData() and gotten back 'true'.
+  // The hope is that the other thread will finish its PerfData
+  // manipulation before we free the memory. The two alternatives are
+  // 1) leak the PerfData memory or 2) do some form of synchronized
+  // access or check before every PerfData operation.
+  _has_PerfData = false;
+  os::naked_short_sleep(1);  // 1ms sleep to let other thread(s) run
+
   for (int index = 0; index < _all->length(); index++) {
     PerfData* p = _all->at(index);
     delete p;
@@ -302,6 +309,7 @@
 
   if (_all == NULL) {
     _all = new PerfDataList(100);
+    _has_PerfData = true;
   }
 
   assert(!_all->contains(p->name()), "duplicate name added");
diff -r 73bec9f941d7 -r b7b2407bc7e5 hotspot/src/share/vm/runtime/perfData.hpp
--- a/hotspot/src/share/vm/runtime/perfData.hpp	Wed Sep 02 17:49:46 2015 -0700
+++ b/hotspot/src/share/vm/runtime/perfData.hpp	Thu Sep 03 10:22:39 2015 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -668,6 +668,7 @@
     static PerfDataList* _sampled;
     static PerfDataList* _constants;
     static const char* _name_spaces[];
+    static volatile bool _has_PerfData;
 
     // add a PerfData item to the list(s) of know PerfData objects
     static void add_item(PerfData* p, bool sampled);
@@ -869,6 +870,7 @@
     }
 
     static void destroy();
+    static bool has_PerfData() { return _has_PerfData; }
 };
 
 // Useful macros to create the performance counters
diff -r 73bec9f941d7 -r b7b2407bc7e5 hotspot/src/share/vm/runtime/perfMemory.cpp
--- a/hotspot/src/share/vm/runtime/perfMemory.cpp	Wed Sep 02 17:49:46 2015 -0700
+++ b/hotspot/src/share/vm/runtime/perfMemory.cpp	Thu Sep 03 10:22:39 2015 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -32,6 +32,7 @@
 #include "runtime/os.hpp"
 #include "runtime/perfData.hpp"
 #include "runtime/perfMemory.hpp"
+#include "runtime/safepoint.hpp"
 #include "runtime/statSampler.hpp"
 #include "utilities/globalDefinitions.hpp"
 
@@ -64,16 +65,20 @@
   if (!UsePerfData) return;
   if (!PerfMemory::is_initialized()) return;
 
-  // if the StatSampler is active, then we don't want to remove
-  // resources it may be dependent on. Typically, the StatSampler
-  // is disengaged from the watcher thread when this method is called,
-  // but it is not disengaged if this method is invoked during a
-  // VM abort.
+  // Only destroy PerfData objects if we're at a safepoint and the
+  // StatSampler is not active. Otherwise, we risk removing PerfData
+  // objects that are currently being used by running JavaThreads
+  // or the StatSampler. This method is invoked while we are not at
+  // a safepoint during a VM abort so leaving the PerfData objects
+  // around may also help diagnose the failure. In rare cases,
+  // PerfData objects are used in parallel with a safepoint. See
+  // the work around in PerfDataManager::destroy().
   //
-  if (!StatSampler::is_active())
+  if (SafepointSynchronize::is_at_safepoint() && !StatSampler::is_active()) {
     PerfDataManager::destroy();
+  }
 
-  // remove the persistent external resources, if any. this method
+  // Remove the persistent external resources, if any. This method
   // does not unmap or invalidate any virtual memory allocated during
   // initialization.
   //
diff -r 73bec9f941d7 -r b7b2407bc7e5 hotspot/src/share/vm/runtime/synchronizer.cpp
--- a/hotspot/src/share/vm/runtime/synchronizer.cpp	Wed Sep 02 17:49:46 2015 -0700
+++ b/hotspot/src/share/vm/runtime/synchronizer.cpp	Thu Sep 03 10:22:39 2015 -0700
@@ -189,9 +189,7 @@
         mon->INotify(self);
         ++tally;
       } while (mon->first_waiter() != NULL && all);
-      if (ObjectMonitor::_sync_Notifications != NULL) {
-        ObjectMonitor::_sync_Notifications->inc(tally);
-      }
+      OM_PERFDATA_OP(Notifications, inc(tally));
     }
     return true;
   }
@@ -1413,7 +1411,7 @@
 
       // Hopefully the performance counters are allocated on distinct cache lines
       // to avoid false sharing on MP systems ...
-      if (ObjectMonitor::_sync_Inflations != NULL) ObjectMonitor::_sync_Inflations->inc();
+      OM_PERFDATA_OP(Inflations, inc());
       TEVENT(Inflate: overwrite stacklock);
       if (TraceMonitorInflation) {
         if (object->is_instance()) {
@@ -1461,7 +1459,7 @@
 
     // Hopefully the performance counters are allocated on distinct
     // cache lines to avoid false sharing on MP systems ...
-    if (ObjectMonitor::_sync_Inflations != NULL) ObjectMonitor::_sync_Inflations->inc();
+    OM_PERFDATA_OP(Inflations, inc());
     TEVENT(Inflate: overwrite neutral);
     if (TraceMonitorInflation) {
       if (object->is_instance()) {
@@ -1678,8 +1676,8 @@
   }
   Thread::muxRelease(&gListLock);
 
-  if (ObjectMonitor::_sync_Deflations != NULL) ObjectMonitor::_sync_Deflations->inc(nScavenged);
-  if (ObjectMonitor::_sync_MonExtant  != NULL) ObjectMonitor::_sync_MonExtant ->set_value(nInCirculation);
+  OM_PERFDATA_OP(Deflations, inc(nScavenged));
+  OM_PERFDATA_OP(MonExtant, set_value(nInCirculation));
 
   // TODO: Add objectMonitor leak detection.
   // Audit/inventory the objectMonitors -- make sure they're all accounted for.