From 71f85a0741baa55e449864e0cce28b6cc406b2d0 Mon Sep 17 00:00:00 2001 From: Julian Seward Date: Fri, 3 Jun 2016 12:13:53 +0200 Subject: [PATCH] Bug 1258183 - TSan: data race toolkit/components/telemetry/Telemetry.cpp in CanRecordBase (part 2, derace). r=chutten. --HG-- extra : rebase_source : 322f4fb1826a1182d2b4f0450e013eaef5858398 --- ipc/chromium/src/base/histogram.cc | 82 +- ipc/chromium/src/base/histogram.h | 66 +- toolkit/components/telemetry/Telemetry.cpp | 10 + toolkit/components/telemetry/Telemetry.h | 7 + .../telemetry/TelemetryHistogram.cpp | 894 ++++++++++++------ .../components/telemetry/TelemetryHistogram.h | 3 + toolkit/xre/nsAppRunner.cpp | 11 +- toolkit/xre/nsEmbedFunctions.cpp | 7 +- 8 files changed, 646 insertions(+), 434 deletions(-) diff --git a/ipc/chromium/src/base/histogram.cc b/ipc/chromium/src/base/histogram.cc index 1f8b7b5d9b26..49561bf6080f 100644 --- a/ipc/chromium/src/base/histogram.cc +++ b/ipc/chromium/src/base/histogram.cc @@ -29,8 +29,6 @@ namespace base { typedef ::Lock Lock; typedef ::AutoLock AutoLock; -using mozilla::OffTheBooksMutexAutoLock; - // Static table of checksums for all possible 8 bit bytes. const uint32_t Histogram::kCrcTable[256] = {0x0, 0x77073096L, 0xee0e612cL, 0x990951baL, 0x76dc419L, 0x706af48fL, 0xe963a535L, 0x9e6495a3L, 0xedb8832L, @@ -177,24 +175,20 @@ void Histogram::WriteAscii(bool graph_it, const std::string& newline, SampleSet snapshot; SnapshotSample(&snapshot); - // For the rest of the routine, we hold |snapshot|'s lock so as to - // be able to examine it atomically. - OffTheBooksMutexAutoLock locker(snapshot.mutex()); + Count sample_count = snapshot.TotalCount(); - Count sample_count = snapshot.TotalCount(locker); - - WriteAsciiHeader(snapshot, locker, sample_count, output); + WriteAsciiHeader(snapshot, sample_count, output); output->append(newline); // Prepare to normalize graphical rendering of bucket contents. double max_size = 0; if (graph_it) - max_size = GetPeakBucketSize(snapshot, locker); + max_size = GetPeakBucketSize(snapshot); // Calculate space needed to print bucket range numbers. Leave room to print // nearly the largest bucket range without sliding over the histogram. size_t largest_non_empty_bucket = bucket_count() - 1; - while (0 == snapshot.counts(locker, largest_non_empty_bucket)) { + while (0 == snapshot.counts(largest_non_empty_bucket)) { if (0 == largest_non_empty_bucket) break; // All buckets are empty. --largest_non_empty_bucket; @@ -203,7 +197,7 @@ void Histogram::WriteAscii(bool graph_it, const std::string& newline, // Calculate largest print width needed for any of our bucket range displays. size_t print_width = 1; for (size_t i = 0; i < bucket_count(); ++i) { - if (snapshot.counts(locker, i)) { + if (snapshot.counts(i)) { size_t width = GetAsciiBucketRange(i).size() + 1; if (width > print_width) print_width = width; @@ -214,7 +208,7 @@ void Histogram::WriteAscii(bool graph_it, const std::string& newline, int64_t past = 0; // Output the actual histogram graph. for (size_t i = 0; i < bucket_count(); ++i) { - Count current = snapshot.counts(locker, i); + Count current = snapshot.counts(i); if (!current && !PrintEmptyBucket(i)) continue; remaining -= current; @@ -223,8 +217,8 @@ void Histogram::WriteAscii(bool graph_it, const std::string& newline, for (size_t j = 0; range.size() + j < print_width + 1; ++j) output->push_back(' '); if (0 == current && - i < bucket_count() - 1 && 0 == snapshot.counts(locker, i + 1)) { - while (i < bucket_count() - 1 && 0 == snapshot.counts(locker, i + 1)) + i < bucket_count() - 1 && 0 == snapshot.counts(i + 1)) { + while (i < bucket_count() - 1 && 0 == snapshot.counts(i + 1)) ++i; output->append("... "); output->append(newline); @@ -244,14 +238,14 @@ void Histogram::WriteAscii(bool graph_it, const std::string& newline, // Methods for the validating a sample and a related histogram. //------------------------------------------------------------------------------ -Histogram::Inconsistencies Histogram::FindCorruption( - const SampleSet& snapshot, - const OffTheBooksMutexAutoLock& snapshotLockEvidence) const { +Histogram::Inconsistencies +Histogram::FindCorruption(const SampleSet& snapshot) const +{ int inconsistencies = NO_INCONSISTENCIES; Sample previous_range = -1; // Bottom range is always 0. int64_t count = 0; for (size_t index = 0; index < bucket_count(); ++index) { - count += snapshot.counts(snapshotLockEvidence, index); + count += snapshot.counts(index); int new_range = ranges(index); if (previous_range >= new_range) inconsistencies |= BUCKET_ORDER_ERROR; @@ -261,7 +255,7 @@ Histogram::Inconsistencies Histogram::FindCorruption( if (!HasValidRangeChecksum()) inconsistencies |= RANGE_CHECKSUM_ERROR; - int64_t delta64 = snapshot.redundant_count(snapshotLockEvidence) - count; + int64_t delta64 = snapshot.redundant_count() - count; if (delta64 != 0) { int delta = static_cast(delta64); if (delta != delta64) @@ -302,7 +296,6 @@ size_t Histogram::bucket_count() const { } void Histogram::SnapshotSample(SampleSet* sample) const { - OffTheBooksMutexAutoLock locker(sample_.mutex()); *sample = sample_; } @@ -336,9 +329,9 @@ size_t Histogram::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) return n; } -size_t Histogram::SampleSet::SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) +size_t +Histogram::SampleSet::SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) { - OffTheBooksMutexAutoLock locker(mutex_); // We're not allowed to do deep dives into STL data structures. This // is as close as we can get to measuring this array. return aMallocSizeOf(&counts_[0]); @@ -556,13 +549,11 @@ uint32_t Histogram::Crc32(uint32_t sum, Histogram::Sample range) { //------------------------------------------------------------------------------ // Private methods -double Histogram::GetPeakBucketSize(const SampleSet& snapshot, - const OffTheBooksMutexAutoLock& - snapshotLockEvidence) const { +double Histogram::GetPeakBucketSize(const SampleSet& snapshot) const { double max = 0; for (size_t i = 0; i < bucket_count() ; ++i) { double current_size - = GetBucketSize(snapshot.counts(snapshotLockEvidence, i), i); + = GetBucketSize(snapshot.counts(i), i); if (current_size > max) max = current_size; } @@ -570,15 +561,13 @@ double Histogram::GetPeakBucketSize(const SampleSet& snapshot, } void Histogram::WriteAsciiHeader(const SampleSet& snapshot, - const OffTheBooksMutexAutoLock& - snapshotLockEvidence, Count sample_count, std::string* output) const { StringAppendF(output, "Histogram: %s recorded %d samples", histogram_name().c_str(), sample_count); - int64_t snapshot_sum = snapshot.sum(snapshotLockEvidence); + int64_t snapshot_sum = snapshot.sum(); if (0 == sample_count) { DCHECK_EQ(snapshot_sum, 0); } else { @@ -629,20 +618,17 @@ void Histogram::WriteAsciiBucketGraph(double current_size, double max_size, Histogram::SampleSet::SampleSet() : counts_(), sum_(0), - redundant_count_(0), - mutex_("Histogram::SampleSet::SampleSet") { + redundant_count_(0) { } Histogram::SampleSet::~SampleSet() { } void Histogram::SampleSet::Resize(const Histogram& histogram) { - OffTheBooksMutexAutoLock locker(mutex_); counts_.resize(histogram.bucket_count(), 0); } -void Histogram::SampleSet::Accumulate(const OffTheBooksMutexAutoLock& ev, - Sample value, Count count, +void Histogram::SampleSet::Accumulate(Sample value, Count count, size_t index) { DCHECK(count == 1 || count == -1); counts_[index] += count; @@ -653,15 +639,7 @@ void Histogram::SampleSet::Accumulate(const OffTheBooksMutexAutoLock& ev, DCHECK_GE(redundant_count_, 0); } -void Histogram::SampleSet::Accumulate(Sample value, - Count count, - size_t index) { - OffTheBooksMutexAutoLock locker(mutex_); - Accumulate(locker, value, count, index); -} - -Count Histogram::SampleSet::TotalCount(const OffTheBooksMutexAutoLock& ev) - const { +Count Histogram::SampleSet::TotalCount() const { Count total = 0; for (Counts::const_iterator it = counts_.begin(); it != counts_.end(); @@ -672,7 +650,6 @@ Count Histogram::SampleSet::TotalCount(const OffTheBooksMutexAutoLock& ev) } void Histogram::SampleSet::Add(const SampleSet& other) { - OffTheBooksMutexAutoLock locker(mutex_); DCHECK_EQ(counts_.size(), other.counts_.size()); sum_ += other.sum_; redundant_count_ += other.redundant_count_; @@ -874,8 +851,7 @@ FlagHistogram::Accumulate(Sample value, Count count, size_t index) void FlagHistogram::AddSampleSet(const SampleSet& sample) { - OffTheBooksMutexAutoLock locker(sample.mutex()); - DCHECK_EQ(bucket_count(), sample.size(locker)); + DCHECK_EQ(bucket_count(), sample.size()); // We can't be sure the SampleSet provided came from another FlagHistogram, // so we take the following steps: // - If our flag has already been set do nothing. @@ -889,12 +865,12 @@ FlagHistogram::AddSampleSet(const SampleSet& sample) { return; } - if (sample.sum(locker) != 1) { + if (sample.sum() != 1) { return; } size_t one_index = BucketIndex(1); - if (sample.counts(locker, one_index) == 1) { + if (sample.counts(one_index) == 1) { Accumulate(1, 1, one_index); } } @@ -946,20 +922,18 @@ CountHistogram::Accumulate(Sample value, Count count, size_t index) void CountHistogram::AddSampleSet(const SampleSet& sample) { - OffTheBooksMutexAutoLock locker(sample.mutex()); - DCHECK_EQ(bucket_count(), sample.size(locker)); + DCHECK_EQ(bucket_count(), sample.size()); // We can't be sure the SampleSet provided came from another CountHistogram, // so we at least check that the unused buckets are empty. const size_t indices[] = { BucketIndex(0), BucketIndex(1), BucketIndex(2) }; - if (sample.counts(locker, indices[1]) != 0 || - sample.counts(locker, indices[2]) != 0) { + if (sample.counts(indices[1]) != 0 || sample.counts(indices[2]) != 0) { return; } - if (sample.counts(locker, indices[0]) != 0) { - Accumulate(1, sample.counts(locker, indices[0]), indices[0]); + if (sample.counts(indices[0]) != 0) { + Accumulate(1, sample.counts(indices[0]), indices[0]); } } diff --git a/ipc/chromium/src/base/histogram.h b/ipc/chromium/src/base/histogram.h index 132898ae784e..20868e9dc14d 100644 --- a/ipc/chromium/src/base/histogram.h +++ b/ipc/chromium/src/base/histogram.h @@ -45,7 +45,6 @@ #include "mozilla/Atomics.h" #include "mozilla/MemoryReporting.h" -#include "mozilla/Mutex.h" #include #include @@ -56,9 +55,6 @@ namespace base { -using mozilla::OffTheBooksMutex; -using mozilla::OffTheBooksMutexAutoLock; - //------------------------------------------------------------------------------ // Provide easy general purpose histogram in a macro, just like stats counters. // The first four macros use 50 buckets. @@ -327,20 +323,8 @@ class Histogram { explicit SampleSet(); ~SampleSet(); - // This class contains a mozilla::OffTheBooksMutex, |mutex_|. - // Most of the methods are thread-safe: they acquire and release - // the mutex themselves. A few are not thread-safe, and require - // the caller to provide evidence that the object is locked, by - // supplying a const OffTheBooksMutexAutoLock& parameter. The - // parameter is ignored but must be present. |mutex_| must be an - // OffTheBooks variant because some of the containing SampleSet - // objects are leaked until shutdown, so a standard Mutex can't be - // used, since that does leak checking, and causes test failures. - - //---------------- THREAD SAFE METHODS ----------------// - // - // The caller must not already hold |this.mutex_|, otherwise we - // will end up deadlocking. + // None of the methods in this class are thread-safe. Callers + // must deal with locking themselves. // Adjust size of counts_ for use with given histogram. void Resize(const Histogram& histogram); @@ -353,38 +337,20 @@ class Histogram { size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf); - //---------------- THREAD UNSAFE METHODS ----------------// - // - // The caller must hold |this.mutex_|, and must supply evidence by passing - // a const reference to the relevant OffTheBooksMutexAutoLock used. - - Count counts(const OffTheBooksMutexAutoLock& ev, size_t i) const { + Count counts(size_t i) const { return counts_[i]; } - Count TotalCount(const OffTheBooksMutexAutoLock& ev) const; - int64_t sum(const OffTheBooksMutexAutoLock& ev) const { + Count TotalCount() const; + int64_t sum() const { return sum_; } - int64_t redundant_count(const OffTheBooksMutexAutoLock& ev) const { + int64_t redundant_count() const { return redundant_count_; } - size_t size(const OffTheBooksMutexAutoLock& ev) const { + size_t size() const { return counts_.size(); } - // An assignment operator. The presence of mozilla::OffTheBooksMutex - // in this class causes the default assignment operator to be deleted. - const SampleSet& operator=(const SampleSet& other) { - counts_ = other.counts_; - sum_ = other.sum_; - redundant_count_ = other.redundant_count_; - return *this; - } - - private: - void Accumulate(const OffTheBooksMutexAutoLock& ev, - Sample value, Count count, size_t index); - protected: // Actual histogram data is stored in buckets, showing the count of values // that fit into each bucket. @@ -402,13 +368,6 @@ class Histogram { // and also the snapshotting code may asynchronously get a mismatch (though // generally either race based mismatch cause is VERY rare). int64_t redundant_count_; - - private: - // Protects all data fields. - mutable OffTheBooksMutex mutex_; - - public: - OffTheBooksMutex& mutex() const { return mutex_; } }; //---------------------------------------------------------------------------- @@ -466,9 +425,7 @@ class Histogram { // produce a false-alarm if a race occurred in the reading of the data during // a SnapShot process, but should otherwise be false at all times (unless we // have memory over-writes, or DRAM failures). - virtual Inconsistencies FindCorruption(const SampleSet& snapshot, - const OffTheBooksMutexAutoLock& - snapshotLockEvidence) const; + virtual Inconsistencies FindCorruption(const SampleSet& snapshot) const; //---------------------------------------------------------------------------- // Accessors for factory constuction, serialization and testing. @@ -483,7 +440,7 @@ class Histogram { // Do a safe atomic snapshot of sample data. The caller is assumed to // have exclusive access to the destination, |*sample|, and no locking - // of it is done here. This routine does lock the source sample though. + // of it is done here. virtual void SnapshotSample(SampleSet* sample) const; virtual bool HasConstructorArguments(Sample minimum, Sample maximum, @@ -559,13 +516,10 @@ class Histogram { // Helpers for emitting Ascii graphic. Each method appends data to output. // Find out how large the (graphically) the largest bucket will appear to be. - double GetPeakBucketSize(const SampleSet& snapshot, - const OffTheBooksMutexAutoLock& - snapshotLockEvidence) const; + double GetPeakBucketSize(const SampleSet& snapshot) const; // Write a common header message describing this histogram. void WriteAsciiHeader(const SampleSet& snapshot, - const OffTheBooksMutexAutoLock& snapshotLockEvidence, Count sample_count, std::string* output) const; // Write information about previous, current, and next buckets. diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index 0dee568abafa..e315f5eaa5c4 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -2906,5 +2906,15 @@ SetProfileDir(nsIFile* aProfD) sTelemetryIOObserver->AddPath(profDirPath, NS_LITERAL_STRING("{profile}")); } +void CreateStatisticsRecorder() +{ + TelemetryHistogram::CreateStatisticsRecorder(); +} + +void DestroyStatisticsRecorder() +{ + TelemetryHistogram::DestroyStatisticsRecorder(); +} + } // namespace Telemetry } // namespace mozilla diff --git a/toolkit/components/telemetry/Telemetry.h b/toolkit/components/telemetry/Telemetry.h index c1133c05d1df..e0832d91759a 100644 --- a/toolkit/components/telemetry/Telemetry.h +++ b/toolkit/components/telemetry/Telemetry.h @@ -37,6 +37,13 @@ enum TimerResolution { Microsecond }; +/** + * Create and destroy the underlying base::StatisticsRecorder singleton. + * Creation has to be done very early in the startup sequence. + */ +void CreateStatisticsRecorder(); +void DestroyStatisticsRecorder(); + /** * Initialize the Telemetry service on the main thread at startup. */ diff --git a/toolkit/components/telemetry/TelemetryHistogram.cpp b/toolkit/components/telemetry/TelemetryHistogram.cpp index ce11db32f4b5..55f2b28f388c 100644 --- a/toolkit/components/telemetry/TelemetryHistogram.cpp +++ b/toolkit/components/telemetry/TelemetryHistogram.cpp @@ -17,6 +17,7 @@ #include "mozilla/dom/ToJSValue.h" #include "mozilla/StartupTimeline.h" +#include "mozilla/StaticMutex.h" #include "TelemetryCommon.h" #include "TelemetryHistogram.h" @@ -29,6 +30,58 @@ using base::BooleanHistogram; using base::CountHistogram; using base::FlagHistogram; using base::LinearHistogram; +using mozilla::StaticMutex; +using mozilla::StaticMutexAutoLock; + + +//////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////// +// +// Naming: there are two kinds of functions in this file: +// +// * Functions named internal_*: these can only be reached via an +// interface function (TelemetryHistogram::*). They mostly expect +// the interface function to have acquired +// |gTelemetryHistogramMutex|, so they do not have to be +// thread-safe. However, those internal_* functions that are +// reachable from internal_WrapAndReturnHistogram and +// internal_WrapAndReturnKeyedHistogram can sometimes be called +// without |gTelemetryHistogramMutex|, and so might be racey. +// +// * Functions named TelemetryHistogram::*. This is the external interface. +// Entries and exits to these functions are serialised using +// |gTelemetryHistogramMutex|. +// +// Avoiding races and deadlocks: +// +// All functions in the external interface (TelemetryHistogram::*) are +// serialised using the mutex |gTelemetryHistogramMutex|. This means +// that the external interface is thread-safe, and many of the +// internal_* functions can ignore thread safety. But it also brings +// a danger of deadlock if any function in the external interface can +// get back to that interface. That is, we will deadlock on any call +// chain like this +// +// TelemetryHistogram::* -> .. any functions .. -> TelemetryHistogram::* +// +// To reduce the danger of that happening, observe the following rules: +// +// * No function in TelemetryHistogram::* may directly call, nor take the +// address of, any other function in TelemetryHistogram::*. +// +// * No internal function internal_* may call, nor take the address +// of, any function in TelemetryHistogram::*. +// +// internal_WrapAndReturnHistogram and +// internal_WrapAndReturnKeyedHistogram are not protected by +// |gTelemetryHistogramMutex| because they make calls to the JS +// engine, but that can in turn call back to Telemetry and hence back +// to a TelemetryHistogram:: function, in order to report GC and other +// statistics. This would lead to deadlock due to attempted double +// acquisition of |gTelemetryHistogramMutex|, if the internal_* functions +// were required to be protected by |gTelemetryHistogramMutex|. To +// break that cycle, we relax that requirement. Unfortunately this +// means that this file is not guaranteed race-free. //////////////////////////////////////////////////////////////////////// @@ -120,6 +173,9 @@ bool gCorruptHistograms[mozilla::Telemetry::HistogramCount]; AddonMapType gAddonMap; +// The singleton StatisticsRecorder object for this process. +base::StatisticsRecorder* gStatisticsRecorder = nullptr; + } // namespace @@ -128,6 +184,8 @@ AddonMapType gAddonMap; // // PRIVATE CONSTANTS +namespace { + // List of histogram IDs which should have recording disabled initially. const mozilla::Telemetry::ID kRecordingInitiallyDisabledIDs[] = { mozilla::Telemetry::FX_REFRESH_DRIVER_SYNC_SCROLL_FRAME_DELAY_MS, @@ -137,6 +195,8 @@ const mozilla::Telemetry::ID kRecordingInitiallyDisabledIDs[] = { mozilla::Telemetry::TELEMETRY_TEST_KEYED_COUNT_INIT_NO_RECORD }; +} // namespace + //////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////// @@ -146,14 +206,24 @@ const mozilla::Telemetry::ID kRecordingInitiallyDisabledIDs[] = { namespace { bool -IsHistogramEnumId(mozilla::Telemetry::ID aID) +internal_CanRecordBase() { + return gCanRecordBase; +} + +bool +internal_CanRecordExtended() { + return gCanRecordExtended; +} + +bool +internal_IsHistogramEnumId(mozilla::Telemetry::ID aID) { static_assert(((mozilla::Telemetry::ID)-1 > 0), "ID should be unsigned."); return aID < mozilla::Telemetry::HistogramCount; } bool -IsExpired(const char *expiration) +internal_IsExpired(const char *expiration) { static mozilla::Version current_version = mozilla::Version(MOZ_APP_VERSION); MOZ_ASSERT(expiration); @@ -162,7 +232,7 @@ IsExpired(const char *expiration) } bool -IsInDataset(uint32_t dataset, uint32_t containingDataset) +internal_IsInDataset(uint32_t dataset, uint32_t containingDataset) { if (dataset == containingDataset) { return true; @@ -179,18 +249,19 @@ IsInDataset(uint32_t dataset, uint32_t containingDataset) } bool -CanRecordDataset(uint32_t dataset) +internal_CanRecordDataset(uint32_t dataset) { // If we are extended telemetry is enabled, we are allowed to record // regardless of the dataset. - if (TelemetryHistogram::CanRecordExtended()) { + if (internal_CanRecordExtended()) { return true; } // If base telemetry data is enabled and we're trying to record base // telemetry, allow it. - if (TelemetryHistogram::CanRecordBase() && - IsInDataset(dataset, nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT)) { + if (internal_CanRecordBase() && + internal_IsInDataset(dataset, + nsITelemetry::DATASET_RELEASE_CHANNEL_OPTOUT)) { return true; } @@ -200,38 +271,36 @@ CanRecordDataset(uint32_t dataset) } bool -IsValidHistogramName(const nsACString& name) +internal_IsValidHistogramName(const nsACString& name) { return !FindInReadable(NS_LITERAL_CSTRING(KEYED_HISTOGRAM_NAME_SEPARATOR), name); } // Note: this is completely unrelated to mozilla::IsEmpty. bool -IsEmpty(const Histogram *h) +internal_IsEmpty(const Histogram *h) { Histogram::SampleSet ss; h->SnapshotSample(&ss); - - mozilla::OffTheBooksMutexAutoLock locker(ss.mutex()); - return ss.counts(locker, 0) == 0 && ss.sum(locker) == 0; + return ss.counts(0) == 0 && ss.sum() == 0; } bool -IsExpired(const Histogram *histogram) +internal_IsExpired(const Histogram *histogram) { return histogram->histogram_name() == EXPIRED_ID; } nsresult -GetRegisteredHistogramIds(bool keyed, uint32_t dataset, uint32_t *aCount, - char*** aHistograms) +internal_GetRegisteredHistogramIds(bool keyed, uint32_t dataset, + uint32_t *aCount, char*** aHistograms) { nsTArray collection; for (size_t i = 0; i < mozilla::ArrayLength(gHistograms); ++i) { const HistogramInfo& h = gHistograms[i]; - if (IsExpired(h.expiration()) || h.keyed != keyed || - !IsInDataset(h.dataset, dataset)) { + if (internal_IsExpired(h.expiration()) || h.keyed != keyed || + !internal_IsInDataset(h.dataset, dataset)) { continue; } @@ -272,8 +341,9 @@ HistogramInfo::expiration() const namespace { nsresult -CheckHistogramArguments(uint32_t histogramType, uint32_t min, uint32_t max, - uint32_t bucketCount, bool haveOptArgs) +internal_CheckHistogramArguments(uint32_t histogramType, + uint32_t min, uint32_t max, + uint32_t bucketCount, bool haveOptArgs) { if (histogramType != nsITelemetry::HISTOGRAM_BOOLEAN && histogramType != nsITelemetry::HISTOGRAM_FLAG @@ -301,16 +371,18 @@ CheckHistogramArguments(uint32_t histogramType, uint32_t min, uint32_t max, * haveOptArgs has to be set if the caller provides them. */ nsresult -HistogramGet(const char *name, const char *expiration, uint32_t histogramType, - uint32_t min, uint32_t max, uint32_t bucketCount, bool haveOptArgs, - Histogram **result) +internal_HistogramGet(const char *name, const char *expiration, + uint32_t histogramType, uint32_t min, uint32_t max, + uint32_t bucketCount, bool haveOptArgs, + Histogram **result) { - nsresult rv = CheckHistogramArguments(histogramType, min, max, bucketCount, haveOptArgs); + nsresult rv = internal_CheckHistogramArguments(histogramType, min, max, + bucketCount, haveOptArgs); if (NS_FAILED(rv)) { return rv; } - if (IsExpired(expiration)) { + if (internal_IsExpired(expiration)) { name = EXPIRED_ID; min = 1; max = 2; @@ -342,7 +414,7 @@ HistogramGet(const char *name, const char *expiration, uint32_t histogramType, } nsresult -GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id) +internal_GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id) { if (!gInitDone) { return NS_ERROR_FAILURE; @@ -358,7 +430,7 @@ GetHistogramEnumId(const char *name, mozilla::Telemetry::ID *id) // O(1) histogram lookup by numeric id nsresult -GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret) +internal_GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret) { static Histogram* knownHistograms[mozilla::Telemetry::HistogramCount] = {0}; Histogram *h = knownHistograms[id]; @@ -372,15 +444,15 @@ GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret) return NS_ERROR_FAILURE; } - nsresult rv = HistogramGet(p.id(), p.expiration(), p.histogramType, - p.min, p.max, p.bucketCount, true, &h); + nsresult rv = internal_HistogramGet(p.id(), p.expiration(), p.histogramType, + p.min, p.max, p.bucketCount, true, &h); if (NS_FAILED(rv)) return rv; #ifdef DEBUG // Check that the C++ Histogram code computes the same ranges as the // Python histogram code. - if (!IsExpired(p.expiration())) { + if (!internal_IsExpired(p.expiration())) { const struct bounds &b = gBucketLowerBoundIndex[id]; if (b.length != 0) { MOZ_ASSERT(size_t(b.length) == h->bucket_count(), @@ -398,15 +470,16 @@ GetHistogramByEnumId(mozilla::Telemetry::ID id, Histogram **ret) } nsresult -GetHistogramByName(const nsACString &name, Histogram **ret) +internal_GetHistogramByName(const nsACString &name, Histogram **ret) { mozilla::Telemetry::ID id; - nsresult rv = GetHistogramEnumId(PromiseFlatCString(name).get(), &id); + nsresult rv + = internal_GetHistogramEnumId(PromiseFlatCString(name).get(), &id); if (NS_FAILED(rv)) { return rv; } - rv = GetHistogramByEnumId(id, ret); + rv = internal_GetHistogramByEnumId(id, ret); if (NS_FAILED(rv)) return rv; @@ -419,17 +492,19 @@ GetHistogramByName(const nsACString &name, Histogram **ret) * For simplicity this is limited to registered histograms. */ Histogram* -CloneHistogram(const nsACString& newName, mozilla::Telemetry::ID existingId, - Histogram& existing) +internal_CloneHistogram(const nsACString& newName, + mozilla::Telemetry::ID existingId, + Histogram& existing) { const HistogramInfo &info = gHistograms[existingId]; Histogram *clone = nullptr; nsresult rv; - rv = HistogramGet(PromiseFlatCString(newName).get(), info.expiration(), - info.histogramType, existing.declared_min(), - existing.declared_max(), existing.bucket_count(), - true, &clone); + rv = internal_HistogramGet(PromiseFlatCString(newName).get(), + info.expiration(), + info.histogramType, existing.declared_min(), + existing.declared_max(), existing.bucket_count(), + true, &clone); if (NS_FAILED(rv)) { return nullptr; } @@ -447,23 +522,25 @@ CloneHistogram(const nsACString& newName, mozilla::Telemetry::ID existingId, * For simplicity this is limited to registered histograms. */ Histogram* -CloneHistogram(const nsACString& newName, mozilla::Telemetry::ID existingId) +internal_CloneHistogram(const nsACString& newName, + mozilla::Telemetry::ID existingId) { Histogram *existing = nullptr; - nsresult rv = GetHistogramByEnumId(existingId, &existing); + nsresult rv = internal_GetHistogramByEnumId(existingId, &existing); if (NS_FAILED(rv)) { return nullptr; } - return CloneHistogram(newName, existingId, *existing); + return internal_CloneHistogram(newName, existingId, *existing); } #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) Histogram* -GetSubsessionHistogram(Histogram& existing) +internal_GetSubsessionHistogram(Histogram& existing) { mozilla::Telemetry::ID id; - nsresult rv = GetHistogramEnumId(existing.histogram_name().c_str(), &id); + nsresult rv + = internal_GetHistogramEnumId(existing.histogram_name().c_str(), &id); if (NS_FAILED(rv) || gHistograms[id].keyed) { return nullptr; } @@ -482,21 +559,21 @@ GetSubsessionHistogram(Histogram& existing) nsCString subsessionName(prefix); subsessionName.Append(existingName); - subsession[id] = CloneHistogram(subsessionName, id, existing); + subsession[id] = internal_CloneHistogram(subsessionName, id, existing); return subsession[id]; } #endif nsresult -HistogramAdd(Histogram& histogram, int32_t value, uint32_t dataset) +internal_HistogramAdd(Histogram& histogram, int32_t value, uint32_t dataset) { // Check if we are allowed to record the data. - if (!CanRecordDataset(dataset) || !histogram.IsRecordingEnabled()) { + if (!internal_CanRecordDataset(dataset) || !histogram.IsRecordingEnabled()) { return NS_OK; } #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) - if (Histogram* subsession = GetSubsessionHistogram(histogram)) { + if (Histogram* subsession = internal_GetSubsessionHistogram(histogram)) { subsession->Add(value); } #endif @@ -509,14 +586,15 @@ HistogramAdd(Histogram& histogram, int32_t value, uint32_t dataset) } nsresult -HistogramAdd(Histogram& histogram, int32_t value) +internal_HistogramAdd(Histogram& histogram, int32_t value) { uint32_t dataset = nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN; // We only really care about the dataset of the histogram if we are not recording // extended telemetry. Otherwise, we always record histogram data. - if (!TelemetryHistogram::CanRecordExtended()) { + if (!internal_CanRecordExtended()) { mozilla::Telemetry::ID id; - nsresult rv = GetHistogramEnumId(histogram.histogram_name().c_str(), &id); + nsresult rv + = internal_GetHistogramEnumId(histogram.histogram_name().c_str(), &id); if (NS_FAILED(rv)) { // If we can't look up the dataset, it might be because the histogram was added // at runtime. Since we're not recording extended telemetry, bail out. @@ -525,18 +603,18 @@ HistogramAdd(Histogram& histogram, int32_t value) dataset = gHistograms[id].dataset; } - return HistogramAdd(histogram, value, dataset); + return internal_HistogramAdd(histogram, value, dataset); } void -HistogramClear(Histogram& aHistogram, bool onlySubsession) +internal_HistogramClear(Histogram& aHistogram, bool onlySubsession) { if (!onlySubsession) { aHistogram.Clear(); } #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) - if (Histogram* subsession = GetSubsessionHistogram(aHistogram)) { + if (Histogram* subsession = internal_GetSubsessionHistogram(aHistogram)) { subsession->Clear(); } #endif @@ -552,14 +630,16 @@ HistogramClear(Histogram& aHistogram, bool onlySubsession) namespace { +void internal_Accumulate(mozilla::Telemetry::ID aHistogram, uint32_t aSample); + void -IdentifyCorruptHistograms(StatisticsRecorder::Histograms &hs) +internal_IdentifyCorruptHistograms(StatisticsRecorder::Histograms &hs) { for (HistogramIterator it = hs.begin(); it != hs.end(); ++it) { Histogram *h = *it; mozilla::Telemetry::ID id; - nsresult rv = ::GetHistogramEnumId(h->histogram_name().c_str(), &id); + nsresult rv = internal_GetHistogramEnumId(h->histogram_name().c_str(), &id); // This histogram isn't a static histogram, just ignore it. if (NS_FAILED(rv)) { continue; @@ -572,12 +652,7 @@ IdentifyCorruptHistograms(StatisticsRecorder::Histograms &hs) Histogram::SampleSet ss; h->SnapshotSample(&ss); - Histogram::Inconsistencies check; - { - mozilla::OffTheBooksMutexAutoLock locker(ss.mutex()); - check = h->FindCorruption(ss, locker); - } - + Histogram::Inconsistencies check = h->FindCorruption(ss); bool corrupt = (check != Histogram::NO_INCONSISTENCIES); if (corrupt) { @@ -591,7 +666,7 @@ IdentifyCorruptHistograms(StatisticsRecorder::Histograms &hs) } else if (check & Histogram::COUNT_LOW_ERROR) { corruptID = mozilla::Telemetry::TOTAL_COUNT_LOW_ERRORS; } - TelemetryHistogram::Accumulate(corruptID, 1); + internal_Accumulate(corruptID, 1); } gCorruptHistograms[id] = corrupt; @@ -609,7 +684,7 @@ IdentifyCorruptHistograms(StatisticsRecorder::Histograms &hs) namespace { bool -FillRanges(JSContext *cx, JS::Handle array, Histogram *h) +internal_FillRanges(JSContext *cx, JS::Handle array, Histogram *h) { JS::Rooted range(cx); for (size_t i = 0; i < h->bucket_count(); i++) { @@ -621,13 +696,12 @@ FillRanges(JSContext *cx, JS::Handle array, Histogram *h) } enum reflectStatus -ReflectHistogramAndSamples(JSContext *cx, JS::Handle obj, Histogram *h, - const Histogram::SampleSet &ss) +internal_ReflectHistogramAndSamples(JSContext *cx, + JS::Handle obj, Histogram *h, + const Histogram::SampleSet &ss) { - mozilla::OffTheBooksMutexAutoLock locker(ss.mutex()); - // We don't want to reflect corrupt histograms. - if (h->FindCorruption(ss, locker) != Histogram::NO_INCONSISTENCIES) { + if (h->FindCorruption(ss) != Histogram::NO_INCONSISTENCIES) { return REFLECT_CORRUPT; } @@ -638,7 +712,7 @@ ReflectHistogramAndSamples(JSContext *cx, JS::Handle obj, Histogram * && JS_DefineProperty(cx, obj, "histogram_type", h->histogram_type(), JSPROP_ENUMERATE) && JS_DefineProperty(cx, obj, "sum", - double(ss.sum(locker)), JSPROP_ENUMERATE))) { + double(ss.sum()), JSPROP_ENUMERATE))) { return REFLECT_FAILURE; } @@ -647,7 +721,7 @@ ReflectHistogramAndSamples(JSContext *cx, JS::Handle obj, Histogram * if (!rarray) { return REFLECT_FAILURE; } - if (!(FillRanges(cx, rarray, h) + if (!(internal_FillRanges(cx, rarray, h) && JS_DefineProperty(cx, obj, "ranges", rarray, JSPROP_ENUMERATE))) { return REFLECT_FAILURE; } @@ -661,7 +735,7 @@ ReflectHistogramAndSamples(JSContext *cx, JS::Handle obj, Histogram * } for (size_t i = 0; i < count; i++) { if (!JS_DefineElement(cx, counts_array, i, - ss.counts(locker, i), JSPROP_ENUMERATE)) { + ss.counts(i), JSPROP_ENUMERATE)) { return REFLECT_FAILURE; } } @@ -670,19 +744,20 @@ ReflectHistogramAndSamples(JSContext *cx, JS::Handle obj, Histogram * } enum reflectStatus -ReflectHistogramSnapshot(JSContext *cx, JS::Handle obj, Histogram *h) +internal_ReflectHistogramSnapshot(JSContext *cx, + JS::Handle obj, Histogram *h) { Histogram::SampleSet ss; h->SnapshotSample(&ss); - return ReflectHistogramAndSamples(cx, obj, h, ss); + return internal_ReflectHistogramAndSamples(cx, obj, h, ss); } bool -ShouldReflectHistogram(Histogram *h) +internal_ShouldReflectHistogram(Histogram *h) { const char *name = h->histogram_name().c_str(); mozilla::Telemetry::ID id; - nsresult rv = ::GetHistogramEnumId(name, &id); + nsresult rv = internal_GetHistogramEnumId(name, &id); if (NS_FAILED(rv)) { // GetHistogramEnumId generally should not fail. But a lookup // failure shouldn't prevent us from reflecting histograms into JS. @@ -700,7 +775,7 @@ ShouldReflectHistogram(Histogram *h) } } -} +} // namespace //////////////////////////////////////////////////////////////////////// @@ -797,9 +872,9 @@ KeyedHistogram::GetHistogram(const nsCString& key, Histogram** histogram, histogramName.Append(key); Histogram* h; - nsresult rv = HistogramGet(histogramName.get(), mExpiration.get(), - mHistogramType, mMin, mMax, mBucketCount, - true, &h); + nsresult rv = internal_HistogramGet(histogramName.get(), mExpiration.get(), + mHistogramType, mMin, mMax, mBucketCount, + true, &h); if (NS_FAILED(rv)) { return rv; } @@ -837,7 +912,7 @@ KeyedHistogram::GetDataset(uint32_t* dataset) const nsresult KeyedHistogram::Add(const nsCString& key, uint32_t sample) { - if (!CanRecordDataset(mDataset)) { + if (!internal_CanRecordDataset(mDataset)) { return NS_OK; } @@ -919,7 +994,8 @@ KeyedHistogram::ReflectKeyedHistogram(KeyedHistogramEntry* entry, return false; } - if (ReflectHistogramSnapshot(cx, histogramSnapshot, entry->mData) != REFLECT_OK) { + if (internal_ReflectHistogramSnapshot(cx, histogramSnapshot, + entry->mData) != REFLECT_OK) { return false; } @@ -965,7 +1041,7 @@ KeyedHistogram::GetJSSnapshot(JSContext* cx, JS::Handle obj, namespace { KeyedHistogram* -GetKeyedHistogramById(const nsACString &name) +internal_GetKeyedHistogramById(const nsACString &name) { if (!gInitDone) { return nullptr; @@ -984,10 +1060,26 @@ GetKeyedHistogramById(const nsACString &name) // // PRIVATE: JSHistogram_* functions +// NOTE: the functions in this section: +// +// internal_JSHistogram_Add +// internal_JSHistogram_Snapshot +// internal_JSHistogram_Clear +// internal_JSHistogram_Dataset +// internal_WrapAndReturnHistogram +// +// all run without protection from |gTelemetryHistogramMutex|. If they +// held |gTelemetryHistogramMutex|, there would be the possibility of +// deadlock because the JS_ calls that they make may call back into the +// TelemetryHistogram interface, hence trying to re-acquire the mutex. +// +// This means that these functions potentially race against threads, but +// that seems preferable to risking deadlock. + namespace { bool -JSHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) { JSObject *obj = JS_THIS_OBJECT(cx, vp); if (!obj) { @@ -1019,15 +1111,15 @@ JSHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) } } - if (TelemetryHistogram::CanRecordBase()) { - HistogramAdd(*h, value); + if (internal_CanRecordBase()) { + internal_HistogramAdd(*h, value); } return true; } bool -JSHistogram_Snapshot(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSHistogram_Snapshot(JSContext *cx, unsigned argc, JS::Value *vp) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); JSObject *obj = JS_THIS_OBJECT(cx, vp); @@ -1040,7 +1132,7 @@ JSHistogram_Snapshot(JSContext *cx, unsigned argc, JS::Value *vp) if (!snapshot) return false; - switch (ReflectHistogramSnapshot(cx, snapshot, h)) { + switch (internal_ReflectHistogramSnapshot(cx, snapshot, h)) { case REFLECT_FAILURE: return false; case REFLECT_CORRUPT: @@ -1055,7 +1147,7 @@ JSHistogram_Snapshot(JSContext *cx, unsigned argc, JS::Value *vp) } bool -JSHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) { JSObject *obj = JS_THIS_OBJECT(cx, vp); if (!obj) { @@ -1079,14 +1171,14 @@ JSHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) Histogram *h = static_cast(JS_GetPrivate(obj)); MOZ_ASSERT(h); if (h) { - HistogramClear(*h, onlySubsession); + internal_HistogramClear(*h, onlySubsession); } return true; } bool -JSHistogram_Dataset(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSHistogram_Dataset(JSContext *cx, unsigned argc, JS::Value *vp) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); JSObject *obj = JS_THIS_OBJECT(cx, vp); @@ -1096,7 +1188,7 @@ JSHistogram_Dataset(JSContext *cx, unsigned argc, JS::Value *vp) Histogram *h = static_cast(JS_GetPrivate(obj)); mozilla::Telemetry::ID id; - nsresult rv = ::GetHistogramEnumId(h->histogram_name().c_str(), &id); + nsresult rv = internal_GetHistogramEnumId(h->histogram_name().c_str(), &id); if (NS_SUCCEEDED(rv)) { args.rval().setNumber(gHistograms[id].dataset); return true; @@ -1105,8 +1197,11 @@ JSHistogram_Dataset(JSContext *cx, unsigned argc, JS::Value *vp) return false; } +// NOTE: Runs without protection from |gTelemetryHistogramMutex|. +// See comment at the top of this section. nsresult -WrapAndReturnHistogram(Histogram *h, JSContext *cx, JS::MutableHandle ret) +internal_WrapAndReturnHistogram(Histogram *h, JSContext *cx, + JS::MutableHandle ret) { static const JSClass JSHistogram_class = { "JSHistogram", /* name */ @@ -1116,10 +1211,14 @@ WrapAndReturnHistogram(Histogram *h, JSContext *cx, JS::MutableHandle JS::Rooted obj(cx, JS_NewObject(cx, &JSHistogram_class)); if (!obj) return NS_ERROR_FAILURE; - if (!(JS_DefineFunction(cx, obj, "add", JSHistogram_Add, 1, 0) - && JS_DefineFunction(cx, obj, "snapshot", JSHistogram_Snapshot, 0, 0) - && JS_DefineFunction(cx, obj, "clear", JSHistogram_Clear, 0, 0) - && JS_DefineFunction(cx, obj, "dataset", JSHistogram_Dataset, 0, 0))) { + // The 4 functions that are wrapped up here are eventually called + // by the same thread that runs this function. + if (!(JS_DefineFunction(cx, obj, "add", internal_JSHistogram_Add, 1, 0) + && JS_DefineFunction(cx, obj, "snapshot", + internal_JSHistogram_Snapshot, 0, 0) + && JS_DefineFunction(cx, obj, "clear", internal_JSHistogram_Clear, 0, 0) + && JS_DefineFunction(cx, obj, "dataset", + internal_JSHistogram_Dataset, 0, 0))) { return NS_ERROR_FAILURE; } JS_SetPrivate(obj, h); @@ -1135,11 +1234,27 @@ WrapAndReturnHistogram(Histogram *h, JSContext *cx, JS::MutableHandle // // PRIVATE: JSKeyedHistogram_* functions +// NOTE: the functions in this section: +// +// internal_KeyedHistogram_SnapshotImpl +// internal_JSKeyedHistogram_Add +// internal_JSKeyedHistogram_Keys +// internal_JSKeyedHistogram_Snapshot +// internal_JSKeyedHistogram_SubsessionSnapshot +// internal_JSKeyedHistogram_SnapshotSubsessionAndClear +// internal_JSKeyedHistogram_Clear +// internal_JSKeyedHistogram_Dataset +// internal_WrapAndReturnKeyedHistogram +// +// Same comments as above, at the JSHistogram_* section, regarding +// deadlock avoidance, apply. + namespace { bool -KeyedHistogram_SnapshotImpl(JSContext *cx, unsigned argc, JS::Value *vp, - bool subsession, bool clearSubsession) +internal_KeyedHistogram_SnapshotImpl(JSContext *cx, unsigned argc, + JS::Value *vp, + bool subsession, bool clearSubsession) { JSObject *obj = JS_THIS_OBJECT(cx, vp); if (!obj) { @@ -1187,7 +1302,7 @@ KeyedHistogram_SnapshotImpl(JSContext *cx, unsigned argc, JS::Value *vp, return false; } - switch (ReflectHistogramSnapshot(cx, snapshot, h)) { + switch (internal_ReflectHistogramSnapshot(cx, snapshot, h)) { case REFLECT_FAILURE: return false; case REFLECT_CORRUPT: @@ -1202,7 +1317,7 @@ KeyedHistogram_SnapshotImpl(JSContext *cx, unsigned argc, JS::Value *vp, } bool -JSKeyedHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSKeyedHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) { JSObject *obj = JS_THIS_OBJECT(cx, vp); if (!obj) { @@ -1252,7 +1367,7 @@ JSKeyedHistogram_Add(JSContext *cx, unsigned argc, JS::Value *vp) } bool -JSKeyedHistogram_Keys(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSKeyedHistogram_Keys(JSContext *cx, unsigned argc, JS::Value *vp) { JSObject *obj = JS_THIS_OBJECT(cx, vp); if (!obj) { @@ -1269,34 +1384,37 @@ JSKeyedHistogram_Keys(JSContext *cx, unsigned argc, JS::Value *vp) } bool -JSKeyedHistogram_Snapshot(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSKeyedHistogram_Snapshot(JSContext *cx, unsigned argc, JS::Value *vp) { - return KeyedHistogram_SnapshotImpl(cx, argc, vp, false, false); + return internal_KeyedHistogram_SnapshotImpl(cx, argc, vp, false, false); } #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) bool -JSKeyedHistogram_SubsessionSnapshot(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSKeyedHistogram_SubsessionSnapshot(JSContext *cx, + unsigned argc, JS::Value *vp) { - return KeyedHistogram_SnapshotImpl(cx, argc, vp, true, false); + return internal_KeyedHistogram_SnapshotImpl(cx, argc, vp, true, false); } #endif #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) bool -JSKeyedHistogram_SnapshotSubsessionAndClear(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSKeyedHistogram_SnapshotSubsessionAndClear(JSContext *cx, + unsigned argc, + JS::Value *vp) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); if (args.length() != 0) { JS_ReportError(cx, "No key arguments supported for snapshotSubsessionAndClear"); } - return KeyedHistogram_SnapshotImpl(cx, argc, vp, true, true); + return internal_KeyedHistogram_SnapshotImpl(cx, argc, vp, true, true); } #endif bool -JSKeyedHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSKeyedHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) { JSObject *obj = JS_THIS_OBJECT(cx, vp); if (!obj) { @@ -1329,7 +1447,7 @@ JSKeyedHistogram_Clear(JSContext *cx, unsigned argc, JS::Value *vp) } bool -JSKeyedHistogram_Dataset(JSContext *cx, unsigned argc, JS::Value *vp) +internal_JSKeyedHistogram_Dataset(JSContext *cx, unsigned argc, JS::Value *vp) { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); JSObject *obj = JS_THIS_OBJECT(cx, vp); @@ -1352,8 +1470,11 @@ JSKeyedHistogram_Dataset(JSContext *cx, unsigned argc, JS::Value *vp) return true; } +// NOTE: Runs without protection from |gTelemetryHistogramMutex|. +// See comment at the top of this section. nsresult -WrapAndReturnKeyedHistogram(KeyedHistogram *h, JSContext *cx, JS::MutableHandle ret) +internal_WrapAndReturnKeyedHistogram(KeyedHistogram *h, JSContext *cx, + JS::MutableHandle ret) { static const JSClass JSHistogram_class = { "JSKeyedHistogram", /* name */ @@ -1363,15 +1484,23 @@ WrapAndReturnKeyedHistogram(KeyedHistogram *h, JSContext *cx, JS::MutableHandle< JS::Rooted obj(cx, JS_NewObject(cx, &JSHistogram_class)); if (!obj) return NS_ERROR_FAILURE; - if (!(JS_DefineFunction(cx, obj, "add", JSKeyedHistogram_Add, 2, 0) - && JS_DefineFunction(cx, obj, "snapshot", JSKeyedHistogram_Snapshot, 1, 0) + // The 7 functions that are wrapped up here are eventually called + // by the same thread that runs this function. + if (!(JS_DefineFunction(cx, obj, "add", internal_JSKeyedHistogram_Add, 2, 0) + && JS_DefineFunction(cx, obj, "snapshot", + internal_JSKeyedHistogram_Snapshot, 1, 0) #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) - && JS_DefineFunction(cx, obj, "subsessionSnapshot", JSKeyedHistogram_SubsessionSnapshot, 1, 0) - && JS_DefineFunction(cx, obj, "snapshotSubsessionAndClear", JSKeyedHistogram_SnapshotSubsessionAndClear, 0, 0) + && JS_DefineFunction(cx, obj, "subsessionSnapshot", + internal_JSKeyedHistogram_SubsessionSnapshot, 1, 0) + && JS_DefineFunction(cx, obj, "snapshotSubsessionAndClear", + internal_JSKeyedHistogram_SnapshotSubsessionAndClear, 0, 0) #endif - && JS_DefineFunction(cx, obj, "keys", JSKeyedHistogram_Keys, 0, 0) - && JS_DefineFunction(cx, obj, "clear", JSKeyedHistogram_Clear, 0, 0) - && JS_DefineFunction(cx, obj, "dataset", JSKeyedHistogram_Dataset, 0, 0))) { + && JS_DefineFunction(cx, obj, "keys", + internal_JSKeyedHistogram_Keys, 0, 0) + && JS_DefineFunction(cx, obj, "clear", + internal_JSKeyedHistogram_Clear, 0, 0) + && JS_DefineFunction(cx, obj, "dataset", + internal_JSKeyedHistogram_Dataset, 0, 0))) { return NS_ERROR_FAILURE; } @@ -1395,8 +1524,8 @@ namespace { // might conflict with other histograms in other addons or even with our // own. void -AddonHistogramName(const nsACString &id, const nsACString &name, - nsACString &ret) +internal_AddonHistogramName(const nsACString &id, const nsACString &name, + nsACString &ret) { ret.Append(id); ret.Append(':'); @@ -1404,12 +1533,13 @@ AddonHistogramName(const nsACString &id, const nsACString &name, } bool -CreateHistogramForAddon(const nsACString &name, AddonHistogramInfo &info) +internal_CreateHistogramForAddon(const nsACString &name, + AddonHistogramInfo &info) { Histogram *h; - nsresult rv = HistogramGet(PromiseFlatCString(name).get(), "never", - info.histogramType, info.min, info.max, - info.bucketCount, true, &h); + nsresult rv = internal_HistogramGet(PromiseFlatCString(name).get(), "never", + info.histogramType, info.min, info.max, + info.bucketCount, true, &h); if (NS_FAILED(rv)) { return false; } @@ -1422,8 +1552,8 @@ CreateHistogramForAddon(const nsACString &name, AddonHistogramInfo &info) } bool -AddonHistogramReflector(AddonHistogramEntryType *entry, - JSContext *cx, JS::Handle obj) +internal_AddonHistogramReflector(AddonHistogramEntryType *entry, + JSContext *cx, JS::Handle obj) { AddonHistogramInfo &info = entry->mData; @@ -1433,12 +1563,12 @@ AddonHistogramReflector(AddonHistogramEntryType *entry, if (info.histogramType != nsITelemetry::HISTOGRAM_FLAG) return true; - if (!CreateHistogramForAddon(entry->GetKey(), info)) { + if (!internal_CreateHistogramForAddon(entry->GetKey(), info)) { return false; } } - if (IsEmpty(info.h)) { + if (internal_IsEmpty(info.h)) { return true; } @@ -1447,7 +1577,7 @@ AddonHistogramReflector(AddonHistogramEntryType *entry, // Just consider this to be skippable. return true; } - switch (ReflectHistogramSnapshot(cx, snapshot, info.h)) { + switch (internal_ReflectHistogramSnapshot(cx, snapshot, info.h)) { case REFLECT_FAILURE: case REFLECT_CORRUPT: return false; @@ -1463,7 +1593,8 @@ AddonHistogramReflector(AddonHistogramEntryType *entry, } bool -AddonReflector(AddonEntryType *entry, JSContext *cx, JS::Handle obj) +internal_AddonReflector(AddonEntryType *entry, JSContext *cx, + JS::Handle obj) { const nsACString &addonId = entry->GetKey(); JS::Rooted subobj(cx, JS_NewPlainObject(cx)); @@ -1472,7 +1603,7 @@ AddonReflector(AddonEntryType *entry, JSContext *cx, JS::Handle obj) } AddonHistogramMapType *map = entry->mData; - if (!(map->ReflectIntoJS(AddonHistogramReflector, cx, subobj) + if (!(map->ReflectIntoJS(internal_AddonHistogramReflector, cx, subobj) && JS_DefineProperty(cx, obj, PromiseFlatCString(addonId).get(), subobj, JSPROP_ENUMERATE))) { return false; @@ -1486,12 +1617,109 @@ AddonReflector(AddonEntryType *entry, JSContext *cx, JS::Handle obj) //////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////// // -// EXTERNALLY VISIBLE FUNCTIONS +// PRIVATE: thread-unsafe helpers for the external interface -namespace TelemetryHistogram { +namespace { -void InitializeGlobalState(bool canRecordBase, bool canRecordExtended) +void +internal_SetHistogramRecordingEnabled(mozilla::Telemetry::ID aID, bool aEnabled) { + if (!internal_IsHistogramEnumId(aID)) { + MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) must be used with an enum id"); + return; + } + + if (gHistograms[aID].keyed) { + const nsDependentCString id(gHistograms[aID].id()); + KeyedHistogram* keyed = internal_GetKeyedHistogramById(id); + if (keyed) { + keyed->SetRecordingEnabled(aEnabled); + return; + } + } else { + Histogram *h; + nsresult rv = internal_GetHistogramByEnumId(aID, &h); + if (NS_SUCCEEDED(rv)) { + h->SetRecordingEnabled(aEnabled); + return; + } + } + + MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) id not found"); +} + +void internal_Accumulate(mozilla::Telemetry::ID aHistogram, uint32_t aSample) +{ + if (!internal_CanRecordBase()) { + return; + } + Histogram *h; + nsresult rv = internal_GetHistogramByEnumId(aHistogram, &h); + if (NS_SUCCEEDED(rv)) { + internal_HistogramAdd(*h, aSample, gHistograms[aHistogram].dataset); + } +} + +void +internal_Accumulate(mozilla::Telemetry::ID aID, + const nsCString& aKey, uint32_t aSample) +{ + if (!gInitDone || !internal_CanRecordBase()) { + return; + } + const HistogramInfo& th = gHistograms[aID]; + KeyedHistogram* keyed + = internal_GetKeyedHistogramById(nsDependentCString(th.id())); + MOZ_ASSERT(keyed); + keyed->Add(aKey, aSample); +} + +} // namespace + + +//////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////// +// +// EXTERNALLY VISIBLE FUNCTIONS in namespace TelemetryHistogram:: + +// This is a StaticMutex rather than a plain Mutex (1) so that +// it gets initialised in a thread-safe manner the first time +// it is used, and (2) because it is never de-initialised, and +// a normal Mutex would show up as a leak in BloatView. StaticMutex +// also has the "OffTheBooks" property, so it won't show as a leak +// in BloatView. +static StaticMutex gTelemetryHistogramMutex; + +// All of these functions are actually in namespace TelemetryHistogram::, +// but the ::TelemetryHistogram prefix is given explicitly. This is +// because it is critical to see which calls from these functions are +// to another function in this interface. Mis-identifying "inwards +// calls" from "calls to another function in this interface" will lead +// to deadlocking and/or races. See comments at the top of the file +// for further (important!) details. + +// Create and destroy the singleton StatisticsRecorder object. +void TelemetryHistogram::CreateStatisticsRecorder() +{ + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + MOZ_ASSERT(!gStatisticsRecorder); + gStatisticsRecorder = new base::StatisticsRecorder(); +} + +void TelemetryHistogram::DestroyStatisticsRecorder() +{ + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + MOZ_ASSERT(gStatisticsRecorder); + if (gStatisticsRecorder) { + delete gStatisticsRecorder; + gStatisticsRecorder = nullptr; + } +} + +void TelemetryHistogram::InitializeGlobalState(bool canRecordBase, + bool canRecordExtended) +{ + StaticMutexAutoLock locker(gTelemetryHistogramMutex); MOZ_ASSERT(!gInitDone, "TelemetryHistogram::InitializeGlobalState " "may only be called once"); @@ -1547,8 +1775,9 @@ void InitializeGlobalState(bool canRecordBase, bool canRecordExtended) gInitDone = true; } -void DeInitializeGlobalState() +void TelemetryHistogram::DeInitializeGlobalState() { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); gCanRecordBase = false; gCanRecordExtended = false; gHistogramMap.Clear(); @@ -1558,80 +1787,70 @@ void DeInitializeGlobalState() } #ifdef DEBUG -bool GlobalStateHasBeenInitialized() { +bool TelemetryHistogram::GlobalStateHasBeenInitialized() { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); return gInitDone; } #endif bool -CanRecordBase() { - return gCanRecordBase; +TelemetryHistogram::CanRecordBase() { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + return internal_CanRecordBase(); } void -SetCanRecordBase(bool b) { +TelemetryHistogram::SetCanRecordBase(bool b) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); gCanRecordBase = b; } bool -CanRecordExtended() { - return gCanRecordExtended; +TelemetryHistogram::CanRecordExtended() { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + return internal_CanRecordExtended(); } void -SetCanRecordExtended(bool b) { +TelemetryHistogram::SetCanRecordExtended(bool b) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); gCanRecordExtended = b; } void -InitHistogramRecordingEnabled() +TelemetryHistogram::InitHistogramRecordingEnabled() { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); const size_t length = mozilla::ArrayLength(kRecordingInitiallyDisabledIDs); for (size_t i = 0; i < length; i++) { - SetHistogramRecordingEnabled(kRecordingInitiallyDisabledIDs[i], false); + internal_SetHistogramRecordingEnabled(kRecordingInitiallyDisabledIDs[i], + false); } } void -SetHistogramRecordingEnabled(mozilla::Telemetry::ID aID, bool aEnabled) +TelemetryHistogram::SetHistogramRecordingEnabled(mozilla::Telemetry::ID aID, + bool aEnabled) { - if (!IsHistogramEnumId(aID)) { - MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) must be used with an enum id"); - return; - } - - if (gHistograms[aID].keyed) { - const nsDependentCString id(gHistograms[aID].id()); - KeyedHistogram* keyed = ::GetKeyedHistogramById(id); - if (keyed) { - keyed->SetRecordingEnabled(aEnabled); - return; - } - } else { - Histogram *h; - nsresult rv = GetHistogramByEnumId(aID, &h); - if (NS_SUCCEEDED(rv)) { - h->SetRecordingEnabled(aEnabled); - return; - } - } - - MOZ_ASSERT(false, "Telemetry::SetHistogramRecordingEnabled(...) id not found"); + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + internal_SetHistogramRecordingEnabled(aID, aEnabled); } nsresult -SetHistogramRecordingEnabled(const nsACString &id, bool aEnabled) +TelemetryHistogram::SetHistogramRecordingEnabled(const nsACString &id, + bool aEnabled) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); Histogram *h; - nsresult rv = GetHistogramByName(id, &h); + nsresult rv = internal_GetHistogramByName(id, &h); if (NS_SUCCEEDED(rv)) { h->SetRecordingEnabled(aEnabled); return NS_OK; } - KeyedHistogram* keyed = ::GetKeyedHistogramById(id); + KeyedHistogram* keyed = internal_GetKeyedHistogramById(id); if (keyed) { keyed->SetRecordingEnabled(aEnabled); return NS_OK; @@ -1642,183 +1861,209 @@ SetHistogramRecordingEnabled(const nsACString &id, bool aEnabled) void -Accumulate(mozilla::Telemetry::ID aHistogram, uint32_t aSample) +TelemetryHistogram::Accumulate(mozilla::Telemetry::ID aHistogram, + uint32_t aSample) { - if (!CanRecordBase()) { - return; - } - Histogram *h; - nsresult rv = GetHistogramByEnumId(aHistogram, &h); - if (NS_SUCCEEDED(rv)) { - HistogramAdd(*h, aSample, gHistograms[aHistogram].dataset); - } + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + internal_Accumulate(aHistogram, aSample); } void -Accumulate(mozilla::Telemetry::ID aID, const nsCString& aKey, uint32_t aSample) +TelemetryHistogram::Accumulate(mozilla::Telemetry::ID aID, + const nsCString& aKey, uint32_t aSample) { - if (!gInitDone || !CanRecordBase()) { - return; - } - const HistogramInfo& th = gHistograms[aID]; - KeyedHistogram* keyed - = ::GetKeyedHistogramById(nsDependentCString(th.id())); - MOZ_ASSERT(keyed); - keyed->Add(aKey, aSample); - + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + internal_Accumulate(aID, aKey, aSample); } void -Accumulate(const char* name, uint32_t sample) +TelemetryHistogram::Accumulate(const char* name, uint32_t sample) { - if (!CanRecordBase()) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + if (!internal_CanRecordBase()) { return; } mozilla::Telemetry::ID id; - nsresult rv = ::GetHistogramEnumId(name, &id); + nsresult rv = internal_GetHistogramEnumId(name, &id); if (NS_FAILED(rv)) { return; } Histogram *h; - rv = GetHistogramByEnumId(id, &h); + rv = internal_GetHistogramByEnumId(id, &h); if (NS_SUCCEEDED(rv)) { - HistogramAdd(*h, sample, gHistograms[id].dataset); + internal_HistogramAdd(*h, sample, gHistograms[id].dataset); } } void -Accumulate(const char* name, const nsCString& key, uint32_t sample) +TelemetryHistogram::Accumulate(const char* name, + const nsCString& key, uint32_t sample) { - if (!CanRecordBase()) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + if (!internal_CanRecordBase()) { return; } mozilla::Telemetry::ID id; - nsresult rv = ::GetHistogramEnumId(name, &id); + nsresult rv = internal_GetHistogramEnumId(name, &id); if (NS_SUCCEEDED(rv)) { - Accumulate(id, key, sample); + internal_Accumulate(id, key, sample); } } void -ClearHistogram(mozilla::Telemetry::ID aId) +TelemetryHistogram::ClearHistogram(mozilla::Telemetry::ID aId) { - if (!TelemetryHistogram::CanRecordBase()) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + if (!internal_CanRecordBase()) { return; } Histogram *h; - nsresult rv = ::GetHistogramByEnumId(aId, &h); + nsresult rv = internal_GetHistogramByEnumId(aId, &h); if (NS_SUCCEEDED(rv) && h) { - ::HistogramClear(*h, false); + internal_HistogramClear(*h, false); } } nsresult -GetHistogramById(const nsACString &name, JSContext *cx, - JS::MutableHandle ret) +TelemetryHistogram::GetHistogramById(const nsACString &name, JSContext *cx, + JS::MutableHandle ret) { - Histogram *h; - nsresult rv = GetHistogramByName(name, &h); - if (NS_FAILED(rv)) - return rv; - - return WrapAndReturnHistogram(h, cx, ret); + Histogram *h = nullptr; + { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + nsresult rv = internal_GetHistogramByName(name, &h); + if (NS_FAILED(rv)) + return rv; + } + // Runs without protection from |gTelemetryHistogramMutex| + return internal_WrapAndReturnHistogram(h, cx, ret); } nsresult -GetKeyedHistogramById(const nsACString &name, JSContext *cx, - JS::MutableHandle ret) +TelemetryHistogram::GetKeyedHistogramById(const nsACString &name, + JSContext *cx, + JS::MutableHandle ret) { KeyedHistogram* keyed = nullptr; - if (!gKeyedHistograms.Get(name, &keyed)) { - return NS_ERROR_FAILURE; + { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + if (!gKeyedHistograms.Get(name, &keyed)) { + return NS_ERROR_FAILURE; + } } - - return WrapAndReturnKeyedHistogram(keyed, cx, ret); - + // Runs without protection from |gTelemetryHistogramMutex| + return internal_WrapAndReturnKeyedHistogram(keyed, cx, ret); } const char* -GetHistogramName(mozilla::Telemetry::ID id) +TelemetryHistogram::GetHistogramName(mozilla::Telemetry::ID id) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); const HistogramInfo& h = gHistograms[id]; return h.id(); } nsresult -NewHistogram(const nsACString &name, const nsACString &expiration, - uint32_t histogramType, uint32_t min, uint32_t max, - uint32_t bucketCount, JSContext *cx, - uint8_t optArgCount, JS::MutableHandle ret) +TelemetryHistogram::NewHistogram(const nsACString &name, + const nsACString &expiration, + uint32_t histogramType, + uint32_t min, uint32_t max, + uint32_t bucketCount, JSContext *cx, + uint8_t optArgCount, + JS::MutableHandle ret) { - if (!IsValidHistogramName(name)) { - return NS_ERROR_INVALID_ARG; + Histogram *h = nullptr; + { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + if (!internal_IsValidHistogramName(name)) { + return NS_ERROR_INVALID_ARG; + } + + nsresult rv = internal_HistogramGet(PromiseFlatCString(name).get(), + PromiseFlatCString(expiration).get(), + histogramType, min, max, bucketCount, + optArgCount == 3, &h); + if (NS_FAILED(rv)) + return rv; + h->ClearFlags(Histogram::kUmaTargetedHistogramFlag); } - Histogram *h; - nsresult rv = HistogramGet(PromiseFlatCString(name).get(), - PromiseFlatCString(expiration).get(), - histogramType, min, max, bucketCount, - optArgCount == 3, &h); - if (NS_FAILED(rv)) - return rv; - h->ClearFlags(Histogram::kUmaTargetedHistogramFlag); - return WrapAndReturnHistogram(h, cx, ret); - + // Runs without protection from |gTelemetryHistogramMutex| + return internal_WrapAndReturnHistogram(h, cx, ret); } nsresult -NewKeyedHistogram(const nsACString &name, const nsACString &expiration, - uint32_t histogramType, uint32_t min, uint32_t max, - uint32_t bucketCount, JSContext *cx, - uint8_t optArgCount, JS::MutableHandle ret) +TelemetryHistogram::NewKeyedHistogram(const nsACString &name, + const nsACString &expiration, + uint32_t histogramType, + uint32_t min, uint32_t max, + uint32_t bucketCount, JSContext *cx, + uint8_t optArgCount, + JS::MutableHandle ret) { - if (!IsValidHistogramName(name)) { - return NS_ERROR_INVALID_ARG; + KeyedHistogram* keyed = nullptr; + { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + if (!internal_IsValidHistogramName(name)) { + return NS_ERROR_INVALID_ARG; + } + + nsresult rv + = internal_CheckHistogramArguments(histogramType, min, max, + bucketCount, optArgCount == 3); + if (NS_FAILED(rv)) { + return rv; + } + + keyed = new KeyedHistogram(name, expiration, histogramType, + min, max, bucketCount, + nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN); + if (MOZ_UNLIKELY(!gKeyedHistograms.Put(name, keyed, mozilla::fallible))) { + delete keyed; + return NS_ERROR_OUT_OF_MEMORY; + } } - nsresult rv = CheckHistogramArguments(histogramType, min, max, bucketCount, optArgCount == 3); - if (NS_FAILED(rv)) { - return rv; - } - - KeyedHistogram* keyed = new KeyedHistogram(name, expiration, histogramType, - min, max, bucketCount, - nsITelemetry::DATASET_RELEASE_CHANNEL_OPTIN); - if (MOZ_UNLIKELY(!gKeyedHistograms.Put(name, keyed, mozilla::fallible))) { - delete keyed; - return NS_ERROR_OUT_OF_MEMORY; - } - - return WrapAndReturnKeyedHistogram(keyed, cx, ret); - + // Runs without protection from |gTelemetryHistogramMutex| + return internal_WrapAndReturnKeyedHistogram(keyed, cx, ret); } nsresult -HistogramFrom(const nsACString &name, const nsACString &existing_name, - JSContext *cx, JS::MutableHandle ret) +TelemetryHistogram::HistogramFrom(const nsACString &name, + const nsACString &existing_name, + JSContext *cx, + JS::MutableHandle ret) { - mozilla::Telemetry::ID id; - nsresult rv = ::GetHistogramEnumId(PromiseFlatCString(existing_name).get(), &id); - if (NS_FAILED(rv)) { - return rv; + Histogram* clone = nullptr; + { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + mozilla::Telemetry::ID id; + nsresult rv + = internal_GetHistogramEnumId(PromiseFlatCString(existing_name).get(), + &id); + if (NS_FAILED(rv)) { + return rv; + } + + clone = internal_CloneHistogram(name, id); + if (!clone) { + return NS_ERROR_FAILURE; + } } - Histogram* clone = CloneHistogram(name, id); - if (!clone) { - return NS_ERROR_FAILURE; - } - - return WrapAndReturnHistogram(clone, cx, ret); + // Runs without protection from |gTelemetryHistogramMutex| + return internal_WrapAndReturnHistogram(clone, cx, ret); } nsresult -CreateHistogramSnapshots(JSContext *cx, - JS::MutableHandle ret, - bool subsession, - bool clearSubsession) +TelemetryHistogram::CreateHistogramSnapshots(JSContext *cx, + JS::MutableHandle ret, + bool subsession, + bool clearSubsession) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); JS::Rooted root_obj(cx, JS_NewPlainObject(cx)); if (!root_obj) return NS_ERROR_FAILURE; @@ -1835,7 +2080,7 @@ CreateHistogramSnapshots(JSContext *cx, type == nsITelemetry::HISTOGRAM_COUNT) { Histogram *h; mozilla::DebugOnly rv - = GetHistogramByEnumId(mozilla::Telemetry::ID(i), &h); + = internal_GetHistogramByEnumId(mozilla::Telemetry::ID(i), &h); MOZ_ASSERT(NS_SUCCEEDED(rv)); } } @@ -1849,20 +2094,21 @@ CreateHistogramSnapshots(JSContext *cx, // // Of course, we hope that all of these corruption-statistics // histograms are not themselves corrupt... - IdentifyCorruptHistograms(hs); + internal_IdentifyCorruptHistograms(hs); // OK, now we can actually reflect things. JS::Rooted hobj(cx); for (HistogramIterator it = hs.begin(); it != hs.end(); ++it) { Histogram *h = *it; - if (!ShouldReflectHistogram(h) || IsEmpty(h) || IsExpired(h)) { + if (!internal_ShouldReflectHistogram(h) || internal_IsEmpty(h) || + internal_IsExpired(h)) { continue; } Histogram* original = h; #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) if (subsession) { - h = GetSubsessionHistogram(*h); + h = internal_GetSubsessionHistogram(*h); if (!h) { continue; } @@ -1873,7 +2119,7 @@ CreateHistogramSnapshots(JSContext *cx, if (!hobj) { return NS_ERROR_FAILURE; } - switch (ReflectHistogramSnapshot(cx, hobj, h)) { + switch (internal_ReflectHistogramSnapshot(cx, hobj, h)) { case REFLECT_CORRUPT: // We can still hit this case even if ShouldReflectHistograms // returns true. The histogram lies outside of our control @@ -1898,22 +2144,29 @@ CreateHistogramSnapshots(JSContext *cx, } nsresult -RegisteredHistograms(uint32_t aDataset, uint32_t *aCount, - char*** aHistograms) +TelemetryHistogram::RegisteredHistograms(uint32_t aDataset, uint32_t *aCount, + char*** aHistograms) { - return GetRegisteredHistogramIds(false, aDataset, aCount, aHistograms); + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + return internal_GetRegisteredHistogramIds(false, + aDataset, aCount, aHistograms); } nsresult -RegisteredKeyedHistograms(uint32_t aDataset, uint32_t *aCount, - char*** aHistograms) +TelemetryHistogram::RegisteredKeyedHistograms(uint32_t aDataset, + uint32_t *aCount, + char*** aHistograms) { - return GetRegisteredHistogramIds(true, aDataset, aCount, aHistograms); + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + return internal_GetRegisteredHistogramIds(true, + aDataset, aCount, aHistograms); } nsresult -GetKeyedHistogramSnapshots(JSContext *cx, JS::MutableHandle ret) +TelemetryHistogram::GetKeyedHistogramSnapshots(JSContext *cx, + JS::MutableHandle ret) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); JS::Rooted obj(cx, JS_NewPlainObject(cx)); if (!obj) { return NS_ERROR_FAILURE; @@ -1940,10 +2193,14 @@ GetKeyedHistogramSnapshots(JSContext *cx, JS::MutableHandle ret) } nsresult -RegisterAddonHistogram(const nsACString &id, const nsACString &name, - uint32_t histogramType, uint32_t min, uint32_t max, - uint32_t bucketCount, uint8_t optArgCount) +TelemetryHistogram::RegisterAddonHistogram(const nsACString &id, + const nsACString &name, + uint32_t histogramType, + uint32_t min, uint32_t max, + uint32_t bucketCount, + uint8_t optArgCount) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); if (histogramType == nsITelemetry::HISTOGRAM_EXPONENTIAL || histogramType == nsITelemetry::HISTOGRAM_LINEAR) { if (optArgCount != 3) { @@ -1996,36 +2253,45 @@ RegisterAddonHistogram(const nsACString &id, const nsACString &name, } nsresult -GetAddonHistogram(const nsACString &id, const nsACString &name, - JSContext *cx, JS::MutableHandle ret) +TelemetryHistogram::GetAddonHistogram(const nsACString &id, + const nsACString &name, + JSContext *cx, + JS::MutableHandle ret) { - AddonEntryType *addonEntry = gAddonMap.GetEntry(id); - // The given id has not been registered. - if (!addonEntry) { - return NS_ERROR_INVALID_ARG; - } + AddonHistogramInfo* info = nullptr; + { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); + AddonEntryType *addonEntry = gAddonMap.GetEntry(id); + // The given id has not been registered. + if (!addonEntry) { + return NS_ERROR_INVALID_ARG; + } - AddonHistogramMapType *histogramMap = addonEntry->mData; - AddonHistogramEntryType *histogramEntry = histogramMap->GetEntry(name); - // The given histogram name has not been registered. - if (!histogramEntry) { - return NS_ERROR_INVALID_ARG; - } + AddonHistogramMapType *histogramMap = addonEntry->mData; + AddonHistogramEntryType *histogramEntry = histogramMap->GetEntry(name); + // The given histogram name has not been registered. + if (!histogramEntry) { + return NS_ERROR_INVALID_ARG; + } - AddonHistogramInfo &info = histogramEntry->mData; - if (!info.h) { - nsAutoCString actualName; - AddonHistogramName(id, name, actualName); - if (!::CreateHistogramForAddon(actualName, info)) { - return NS_ERROR_FAILURE; + info = &histogramEntry->mData; + if (!info->h) { + nsAutoCString actualName; + internal_AddonHistogramName(id, name, actualName); + if (!internal_CreateHistogramForAddon(actualName, *info)) { + return NS_ERROR_FAILURE; + } } } - return WrapAndReturnHistogram(info.h, cx, ret); + + // Runs without protection from |gTelemetryHistogramMutex| + return internal_WrapAndReturnHistogram(info->h, cx, ret); } nsresult -UnregisterAddonHistograms(const nsACString &id) +TelemetryHistogram::UnregisterAddonHistograms(const nsACString &id) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); AddonEntryType *addonEntry = gAddonMap.GetEntry(id); if (addonEntry) { // Histogram's destructor is private, so this is the best we can do. @@ -2040,14 +2306,16 @@ UnregisterAddonHistograms(const nsACString &id) } nsresult -GetAddonHistogramSnapshots(JSContext *cx, JS::MutableHandle ret) +TelemetryHistogram::GetAddonHistogramSnapshots(JSContext *cx, + JS::MutableHandle ret) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); JS::Rooted obj(cx, JS_NewPlainObject(cx)); if (!obj) { return NS_ERROR_FAILURE; } - if (!gAddonMap.ReflectIntoJS(AddonReflector, cx, obj)) { + if (!gAddonMap.ReflectIntoJS(internal_AddonReflector, cx, obj)) { return NS_ERROR_FAILURE; } ret.setObject(*obj); @@ -2055,15 +2323,19 @@ GetAddonHistogramSnapshots(JSContext *cx, JS::MutableHandle ret) } size_t -GetMapShallowSizesOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) +TelemetryHistogram::GetMapShallowSizesOfExcludingThis(mozilla::MallocSizeOf + aMallocSizeOf) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); return gAddonMap.ShallowSizeOfExcludingThis(aMallocSizeOf) + gHistogramMap.ShallowSizeOfExcludingThis(aMallocSizeOf); } size_t -GetHistogramSizesofIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) +TelemetryHistogram::GetHistogramSizesofIncludingThis(mozilla::MallocSizeOf + aMallocSizeOf) { + StaticMutexAutoLock locker(gTelemetryHistogramMutex); StatisticsRecorder::Histograms hs; StatisticsRecorder::GetHistograms(&hs); size_t n = 0; @@ -2073,5 +2345,3 @@ GetHistogramSizesofIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) } return n; } - -} // namespace TelemetryHistogram diff --git a/toolkit/components/telemetry/TelemetryHistogram.h b/toolkit/components/telemetry/TelemetryHistogram.h index fdaee5b8a02d..e855c4ffadc3 100644 --- a/toolkit/components/telemetry/TelemetryHistogram.h +++ b/toolkit/components/telemetry/TelemetryHistogram.h @@ -15,6 +15,9 @@ namespace TelemetryHistogram { +void CreateStatisticsRecorder(); +void DestroyStatisticsRecorder(); + void InitializeGlobalState(bool canRecordBase, bool canRecordExtended); void DeInitializeGlobalState(); #ifdef DEBUG diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index 9e6492626d80..dfb484d233d3 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -93,8 +93,6 @@ #include "mozilla/scache/StartupCache.h" #include "gfxPrefs.h" -#include "base/histogram.h" - #include "mozilla/unused.h" #ifdef XP_WIN @@ -4569,12 +4567,9 @@ XRE_StopLateWriteChecks(void) { void XRE_CreateStatsObject() { - // A initializer to initialize histogram collection, a chromium - // thing used by Telemetry (and effectively a global; it's all static). - // Note: purposely leaked - base::StatisticsRecorder* statistics_recorder = new base::StatisticsRecorder(); - MOZ_LSAN_INTENTIONALLY_LEAK_OBJECT(statistics_recorder); - Unused << statistics_recorder; + // Initialize global variables used by histogram collection + // machinery that is used by by Telemetry. Note: is never de-initialised. + Telemetry::CreateStatisticsRecorder(); } int diff --git a/toolkit/xre/nsEmbedFunctions.cpp b/toolkit/xre/nsEmbedFunctions.cpp index 7ffeeeb5da95..ccab17c854e0 100644 --- a/toolkit/xre/nsEmbedFunctions.cpp +++ b/toolkit/xre/nsEmbedFunctions.cpp @@ -76,7 +76,7 @@ #include "GeckoProfiler.h" - #include "base/histogram.h" +#include "mozilla/Telemetry.h" #if defined(MOZ_SANDBOX) && defined(XP_WIN) #include "mozilla/sandboxTarget.h" @@ -318,8 +318,7 @@ XRE_InitChildProcess(int aArgc, #endif // This is needed by Telemetry to initialize histogram collection. - UniquePtr statisticsRecorder = - MakeUnique(); + Telemetry::CreateStatisticsRecorder(); #if !defined(MOZ_WIDGET_ANDROID) && !defined(MOZ_WIDGET_GONK) // On non-Fennec Gecko, the GMPLoader code resides in plugin-container, @@ -668,7 +667,7 @@ XRE_InitChildProcess(int aArgc, } } - statisticsRecorder = nullptr; + Telemetry::DestroyStatisticsRecorder(); profiler_shutdown(); NS_LogTerm(); return XRE_DeinitCommandLine();