8049304: race between VM_Exit and _sync_FutileWakeups->inc()

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
This commit is contained in:
Daniel D. Daugherty 2015-09-03 10:22:39 -07:00
parent c091c7348a
commit 9b12926ed8
6 changed files with 57 additions and 39 deletions

@ -405,9 +405,7 @@ void NOINLINE ObjectMonitor::enter(TRAPS) {
event.commit();
}
if (ObjectMonitor::_sync_ContendedLockAttempts != NULL) {
ObjectMonitor::_sync_ContendedLockAttempts->inc();
}
OM_PERFDATA_OP(ContendedLockAttempts, inc());
}
@ -574,9 +572,9 @@ void NOINLINE ObjectMonitor::EnterI(TRAPS) {
// 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 @@ void NOINLINE ObjectMonitor::ReenterI(Thread * Self, ObjectWaiter * SelfNode) {
// *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 @@ void ObjectMonitor::ExitEpilog(Thread * Self, ObjectWaiter * Wakee) {
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 @@ void ObjectMonitor::notify(TRAPS) {
}
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 @@ void ObjectMonitor::notifyAll(TRAPS) {
INotify(THREAD);
}
if (tally != 0 && ObjectMonitor::_sync_Notifications != NULL) {
ObjectMonitor::_sync_Notifications->inc(tally);
}
OM_PERFDATA_OP(Notifications, inc(tally));
}
// -----------------------------------------------------------------------------

@ -177,6 +177,19 @@ class ObjectMonitor {
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;

@ -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 @@ PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC
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 @@ PerfStringConstant::PerfStringConstant(CounterNS ns, const char* namep,
}
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 @@ void PerfDataManager::add_item(PerfData* p, bool sampled) {
if (_all == NULL) {
_all = new PerfDataList(100);
_has_PerfData = true;
}
assert(!_all->contains(p->name()), "duplicate name added");

@ -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 @@ class PerfDataManager : AllStatic {
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 @@ class PerfDataManager : AllStatic {
}
static void destroy();
static bool has_PerfData() { return _has_PerfData; }
};
// Useful macros to create the performance counters

@ -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 @@ void perfMemory_exit() {
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.
//

@ -189,9 +189,7 @@ bool ObjectSynchronizer::quick_notify(oopDesc * obj, Thread * self, bool all) {
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 @@ ObjectMonitor * NOINLINE ObjectSynchronizer::inflate(Thread * Self,
// 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 @@ ObjectMonitor * NOINLINE ObjectSynchronizer::inflate(Thread * Self,
// 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 @@ void ObjectSynchronizer::deflate_idle_monitors() {
}
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.