Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(259)

Unified Diff: base/metrics/persistent_histogram_allocator.cc

Issue 2665893002: Use atomic ops for managing g_allocator. (Closed)
Patch Set: Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/metrics/persistent_histogram_allocator.cc
diff --git a/base/metrics/persistent_histogram_allocator.cc b/base/metrics/persistent_histogram_allocator.cc
index b398cd84c5ce18a967768e175e5bdf3b12cd4cac..3849cbc36b4590a7c9fea2680a985da71852dfdb 100644
--- a/base/metrics/persistent_histogram_allocator.cc
+++ b/base/metrics/persistent_histogram_allocator.cc
@@ -6,6 +6,7 @@
#include <memory>
+#include "base/atomicops.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/files/important_file_writer.h"
@@ -44,8 +45,10 @@ enum : uint32_t {
// but that's best since PersistentMemoryAllocator objects (that underlie
// GlobalHistogramAllocator objects) are explicitly forbidden from doing
// anything essential at exit anyway due to the fact that they depend on data
-// managed elsewhere and which could be destructed first.
-GlobalHistogramAllocator* g_allocator = nullptr;
+// managed elsewhere and which could be destructed first. An AtomicWord is
+// used instead of std::atomic because the latter can create global ctors
+// and dtors.
+subtle::AtomicWord g_allocator = 0;
// Take an array of range boundaries and create a proper BucketRanges object
// which is returned to the caller. A return of nullptr indicates that the
@@ -502,7 +505,7 @@ PersistentHistogramAllocator::GetCreateHistogramResultHistogram() {
static bool initialized = false;
if (!initialized) {
initialized = true;
- if (g_allocator) {
+ if (GlobalHistogramAllocator::Get()) {
DVLOG(1) << "Creating the results-histogram inside persistent"
<< " memory can cause future allocations to crash if"
<< " that memory is ever released (for testing).";
@@ -650,7 +653,7 @@ PersistentHistogramAllocator::GetOrCreateStatisticsRecorderHistogram(
const HistogramBase* histogram) {
// This should never be called on the global histogram allocator as objects
// created there are already within the global statistics recorder.
- DCHECK_NE(g_allocator, this);
+ DCHECK_NE(GlobalHistogramAllocator::Get(), this);
DCHECK(histogram);
HistogramBase* existing =
@@ -819,8 +822,9 @@ void GlobalHistogramAllocator::Set(
// Releasing or changing an allocator is extremely dangerous because it
// likely has histograms stored within it. If the backing memory is also
// also released, future accesses to those histograms will seg-fault.
- CHECK(!g_allocator);
- g_allocator = allocator.release();
+ CHECK(!subtle::NoBarrier_Load(&g_allocator));
+ subtle::NoBarrier_Store(&g_allocator,
+ reinterpret_cast<uintptr_t>(allocator.release()));
size_t existing = StatisticsRecorder::GetHistogramCount();
DVLOG_IF(1, existing)
@@ -829,13 +833,14 @@ void GlobalHistogramAllocator::Set(
// static
GlobalHistogramAllocator* GlobalHistogramAllocator::Get() {
- return g_allocator;
+ return reinterpret_cast<GlobalHistogramAllocator*>(
+ subtle::NoBarrier_Load(&g_allocator));
}
// static
std::unique_ptr<GlobalHistogramAllocator>
GlobalHistogramAllocator::ReleaseForTesting() {
- GlobalHistogramAllocator* histogram_allocator = g_allocator;
+ GlobalHistogramAllocator* histogram_allocator = Get();
if (!histogram_allocator)
return nullptr;
PersistentMemoryAllocator* memory_allocator =
@@ -859,7 +864,7 @@ GlobalHistogramAllocator::ReleaseForTesting() {
DCHECK_NE(kResultHistogram, data->name);
}
- g_allocator = nullptr;
+ subtle::NoBarrier_Store(&g_allocator, 0);
return WrapUnique(histogram_allocator);
};
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698