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
--- 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));
}
// -----------------------------------------------------------------------------
--- 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;
--- 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");
--- 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
--- 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.
//
--- 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.