From 32de9efebce4afadb599a2a24d85d65468a4ad5a Mon Sep 17 00:00:00 2001 From: Jamie Madill Date: Thu, 17 Sep 2020 12:03:12 -0400 Subject: [PATCH] Guard against race in perf warnings. Expose the "debug" mutex so insertPerfWarning can use it. This was detected using TSAN and MultithreadingTest. Bug: b/168744561 Change-Id: Idde95a7b217f21f007893192a4a1f0a69b4ed3a9 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2415184 Commit-Queue: Jamie Madill Reviewed-by: Shahbaz Youssefi Reviewed-by: Courtney Goeltzenleuchter --- src/common/debug.cpp | 7 ++++++- src/common/debug.h | 3 +++ src/libANGLE/Debug.cpp | 20 +++++++++++++++----- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/common/debug.cpp b/src/common/debug.cpp index a2f8bba29..e14a9e39f 100644 --- a/src/common/debug.cpp +++ b/src/common/debug.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include @@ -114,6 +113,12 @@ void InitializeDebugMutexIfNeeded() } } +std::mutex &GetDebugMutex() +{ + ASSERT(g_debugMutex); + return *g_debugMutex; +} + ScopedPerfEventHelper::ScopedPerfEventHelper(const char *format, ...) : mFunctionName(nullptr) { bool dbgTrace = DebugAnnotationsActive(); diff --git a/src/common/debug.h b/src/common/debug.h index 3e7e18b31..59a5000a0 100644 --- a/src/common/debug.h +++ b/src/common/debug.h @@ -15,6 +15,7 @@ #include #include +#include #include #include @@ -103,6 +104,8 @@ bool DebugAnnotationsInitialized(); void InitializeDebugMutexIfNeeded(); +std::mutex &GetDebugMutex(); + namespace priv { // This class is used to explicitly ignore values in the conditional logging macros. This avoids diff --git a/src/libANGLE/Debug.cpp b/src/libANGLE/Debug.cpp index 1443bb387..5125b2164 100644 --- a/src/libANGLE/Debug.cpp +++ b/src/libANGLE/Debug.cpp @@ -340,18 +340,28 @@ size_t Debug::getGroupStackDepth() const void Debug::insertPerfWarning(GLenum severity, const char *message, uint32_t *repeatCount) const { - constexpr uint32_t kMaxRepeat = 4; - if (*repeatCount >= kMaxRepeat) + bool repeatLast; + { - return; + constexpr uint32_t kMaxRepeat = 4; + std::lock_guard lock(GetDebugMutex()); + + if (*repeatCount >= kMaxRepeat) + { + return; + } + + ++*repeatCount; + repeatLast = (*repeatCount == kMaxRepeat); } - ++*repeatCount; std::string msg = message; - if (*repeatCount == kMaxRepeat) + if (repeatLast) { msg += " (this message will no longer repeat)"; } + + // Release the lock before we call insertMessage. It will re-acquire the lock. insertMessage(GL_DEBUG_SOURCE_API, GL_DEBUG_TYPE_PERFORMANCE, 0, severity, std::move(msg), gl::LOG_INFO); }