Fix memory corruption in PerformanceTimer when Stats are active

PerformanceTimer was neither thread safe nor re-entrant.  Because it was being used increasingly on render and worker threads it has increased the likelihood of heap corruption.
I was able to identify this by enabling full page heap verification using the gflags app.

https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/enable-page-heap

interface/src/ui/Stats.cpp

@@ -431,7 +431,7 @@ void Stats::updateStats(bool force) {
         // a new Map sorted by average time...
         bool onlyDisplayTopTen = Menu::getInstance()->isOptionChecked(MenuOption::OnlyDisplayTopTen);
         QMap<float, QString> sortedRecords;
-        const QMap<QString, PerformanceTimerRecord>& allRecords = PerformanceTimer::getAllTimerRecords();
+        auto allRecords = PerformanceTimer::getAllTimerRecords();
         QMapIterator<QString, PerformanceTimerRecord> i(allRecords);
 
         while (i.hasNext()) {
@@ -479,7 +479,7 @@ void Stats::updateStats(bool force) {
             bool operator<(const SortableStat& other) const { return priority < other.priority; }
         };
 
-        const QMap<QString, PerformanceTimerRecord>& allRecords = PerformanceTimer::getAllTimerRecords();
+        auto allRecords = PerformanceTimer::getAllTimerRecords();
         std::priority_queue<SortableStat> idleUpdateStats;
         auto itr = allRecords.find("/idle/update");
         if (itr != allRecords.end()) {
@@ -496,7 +496,7 @@ void Stats::updateStats(bool force) {
             };
             for (int32_t j = 0; j < categories.size(); ++j) {
                 QString recordKey = "/idle/update/" + categories[j];
-                auto record = PerformanceTimer::getTimerRecord(recordKey);
+                auto& record = allRecords[recordKey];
                 if (record.getCount()) {
                     float dt = (float) record.getMovingAverage() / (float)USECS_PER_MSEC;
                     QString message = QString("\n    %1 = %2").arg(categories[j]).arg(dt);

libraries/shared/src/PerfStat.cpp

@@ -80,16 +80,19 @@ void PerformanceTimerRecord::tallyResult(const quint64& now) {
 // ----------------------------------------------------------------------------
 
 std::atomic<bool> PerformanceTimer::_isActive(false);
+std::mutex PerformanceTimer::_mutex;
 QHash<QThread*, QString> PerformanceTimer::_fullNames;
 QMap<QString, PerformanceTimerRecord> PerformanceTimer::_records;
 
-
 PerformanceTimer::PerformanceTimer(const QString& name) {
     if (_isActive) {
         _name = name;
-        QString& fullName = _fullNames[QThread::currentThread()];
-        fullName.append("/");
-        fullName.append(_name);
+        {
+            std::lock_guard<std::mutex> guard(_mutex);
+            QString& fullName = _fullNames[QThread::currentThread()];
+            fullName.append("/");
+            fullName.append(_name);
+        }
         _start = usecTimestampNow();
     }
 }
@@ -97,6 +100,7 @@ PerformanceTimer::PerformanceTimer(const QString& name) {
 PerformanceTimer::~PerformanceTimer() {
     if (_isActive && _start != 0) {
         quint64 elapsedUsec = (usecTimestampNow() - _start);
+        std::lock_guard<std::mutex> guard(_mutex);
         QString& fullName = _fullNames[QThread::currentThread()];
         PerformanceTimerRecord& namedRecord = _records[fullName];
         namedRecord.accumulateResult(elapsedUsec);
@@ -111,11 +115,13 @@ bool PerformanceTimer::isActive() {
 
 // static
 QString PerformanceTimer::getContextName() {
+    std::lock_guard<std::mutex> guard(_mutex);
     return _fullNames[QThread::currentThread()];
 }
 
 // static
 void PerformanceTimer::addTimerRecord(const QString& fullName, quint64 elapsedUsec) {
+    std::lock_guard<std::mutex> guard(_mutex);
     PerformanceTimerRecord& namedRecord = _records[fullName];
     namedRecord.accumulateResult(elapsedUsec);
 }
@@ -125,6 +131,7 @@ void PerformanceTimer::setActive(bool active) {
     if (active != _isActive) {
         _isActive.store(active);
         if (!active) {
+            std::lock_guard<std::mutex> guard(_mutex);
             _fullNames.clear();
             _records.clear();
         }
@@ -133,8 +140,15 @@ void PerformanceTimer::setActive(bool active) {
     }
 }
 
+// static
+QMap<QString, PerformanceTimerRecord> PerformanceTimer::getAllTimerRecords() {
+    std::lock_guard<std::mutex> guard(_mutex);
+    return _records;
+};
+
 // static
 void PerformanceTimer::tallyAllTimerRecords() {
+    std::lock_guard<std::mutex> guard(_mutex);
     QMap<QString, PerformanceTimerRecord>::iterator recordsItr = _records.begin();
     QMap<QString, PerformanceTimerRecord>::const_iterator recordsEnd = _records.end();
     quint64 now = usecTimestampNow();
@@ -150,6 +164,7 @@ void PerformanceTimer::tallyAllTimerRecords() {
 }
 
 void PerformanceTimer::dumpAllTimerRecords() {
+    std::lock_guard<std::mutex> guard(_mutex);
     QMapIterator<QString, PerformanceTimerRecord> i(_records);
     while (i.hasNext()) {
         i.next();

libraries/shared/src/PerfStat.h

@@ -84,8 +84,7 @@ public:
 
     static QString getContextName();
     static void addTimerRecord(const QString& fullName, quint64 elapsedUsec);
-    static const PerformanceTimerRecord& getTimerRecord(const QString& name) { return _records[name]; };
-    static const QMap<QString, PerformanceTimerRecord>& getAllTimerRecords() { return _records; };
+    static QMap<QString, PerformanceTimerRecord> getAllTimerRecords();
     static void tallyAllTimerRecords();
     static void dumpAllTimerRecords();
 
@@ -93,6 +92,8 @@ private:
     quint64 _start = 0;
     QString _name;
     static std::atomic<bool> _isActive;
+
+    static std::mutex _mutex;  // used to guard multi-threaded access to _fullNames and _records
     static QHash<QThread*, QString> _fullNames;
     static QMap<QString, PerformanceTimerRecord> _records;
 };