From 4d177eb1bd1fb67f0399e9cf4fb8f2b628729413 Mon Sep 17 00:00:00 2001 From: Tomas Gunnarsson Date: Mon, 8 Jun 2020 23:08:46 +0200 Subject: [PATCH] Add diagnostic printout to RTC_DCHECK_RUN_ON. When using a SequenceChecker, this adds a bit more information about why the check failed. Example (The "Expects" line is new): # Fatal error in: foo.cc, line 380 # last system error: 0 # Check failed: (&thread_checker_)->IsCurrent() # Expects: System queue: 0x7fff69541330, TaskQueue: 0x101804370 (not current), Thread: 0x10053cdc0 Bug: none Change-Id: I3743e1d80f369f15219de5946e9e081f998b9b17 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176569 Commit-Queue: Tommi Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#31466} --- rtc_base/synchronization/BUILD.gn | 1 + rtc_base/synchronization/sequence_checker.cc | 68 +++++++++++++++++++ rtc_base/synchronization/sequence_checker.h | 20 +++++- .../sequence_checker_unittest.cc | 2 +- 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/rtc_base/synchronization/BUILD.gn b/rtc_base/synchronization/BUILD.gn index 6ecb0542f1..ba63f853ff 100644 --- a/rtc_base/synchronization/BUILD.gn +++ b/rtc_base/synchronization/BUILD.gn @@ -72,6 +72,7 @@ rtc_library("sequence_checker") { "..:criticalsection", "..:macromagic", "..:platform_thread_types", + "..:stringutils", "../../api/task_queue", "../system:rtc_export", ] diff --git a/rtc_base/synchronization/sequence_checker.cc b/rtc_base/synchronization/sequence_checker.cc index d64f32a616..d5d981307b 100644 --- a/rtc_base/synchronization/sequence_checker.cc +++ b/rtc_base/synchronization/sequence_checker.cc @@ -9,10 +9,16 @@ */ #include "rtc_base/synchronization/sequence_checker.h" +#include +#include +#include + #if defined(WEBRTC_MAC) #include #endif +#include "rtc_base/strings/string_builder.h" + namespace webrtc { namespace { // On Mac, returns the label of the current dispatch queue; elsewhere, return @@ -24,8 +30,28 @@ const void* GetSystemQueueRef() { return nullptr; #endif } + +template ::value>::type* = nullptr> +uintptr_t CastToUintPtr(T t) { + return reinterpret_cast(t); +} + +template ::value>::type* = nullptr> +uintptr_t CastToUintPtr(T t) { + return static_cast(t); +} + } // namespace +std::string ExpectationToString(const webrtc::SequenceChecker* checker) { +#if RTC_DCHECK_IS_ON + return checker->ExpectationToString(); +#endif + return std::string(); +} + SequenceCheckerImpl::SequenceCheckerImpl() : attached_(true), valid_thread_(rtc::CurrentThreadRef()), @@ -62,4 +88,46 @@ void SequenceCheckerImpl::Detach() { // reset on the next call to IsCurrent(). } +#if RTC_DCHECK_IS_ON +std::string SequenceCheckerImpl::ExpectationToString() const { + const TaskQueueBase* const current_queue = TaskQueueBase::Current(); + const rtc::PlatformThreadRef current_thread = rtc::CurrentThreadRef(); + const void* const current_system_queue = GetSystemQueueRef(); + rtc::CritScope scoped_lock(&lock_); + if (!attached_) + return "Checker currently not attached."; + + // NOTE: The format of thie string built here is meant to compliment the one + // we have inside of FatalLog() (checks.cc). + // + // Example: + // + // Expectations vs Actual: + // # Exp: TQ: 0000000000000000 SysQ: 00007fff69541330 Thread: 0000000113aafdc0 + // # Act: TQ: 00007fcde7a22210 SysQ: 00007fcde78553c0 Thread: 0000700005ddc000 + // TaskQueue doesn't match + + rtc::StringBuilder message; + message.AppendFormat( + "Expectations vs Actual:\n# Exp: " + "TQ: %016" PRIxPTR " SysQ: %016" PRIxPTR " Thread: %016" PRIxPTR + "\n# Act: " + "TQ: %016" PRIxPTR " SysQ: %016" PRIxPTR " Thread: %016" PRIxPTR "\n", + CastToUintPtr(valid_queue_), CastToUintPtr(valid_system_queue_), + CastToUintPtr(valid_thread_), CastToUintPtr(current_queue), + CastToUintPtr(current_system_queue), CastToUintPtr(current_thread)); + + if ((valid_queue_ || current_queue) && valid_queue_ != current_queue) { + message << "TaskQueue doesn't match\n"; + } else if (valid_system_queue_ && + valid_system_queue_ != current_system_queue) { + message << "System queue doesn't match\n"; + } else if (!rtc::IsThreadRefEqual(valid_thread_, current_thread)) { + message << "Threads don't match\n"; + } + + return message.Release(); +} +#endif // RTC_DCHECK_IS_ON + } // namespace webrtc diff --git a/rtc_base/synchronization/sequence_checker.h b/rtc_base/synchronization/sequence_checker.h index fe644fa14e..fd0a69983a 100644 --- a/rtc_base/synchronization/sequence_checker.h +++ b/rtc_base/synchronization/sequence_checker.h @@ -10,6 +10,8 @@ #ifndef RTC_BASE_SYNCHRONIZATION_SEQUENCE_CHECKER_H_ #define RTC_BASE_SYNCHRONIZATION_SEQUENCE_CHECKER_H_ +#include + #include "api/task_queue/task_queue_base.h" #include "rtc_base/critical_section.h" #include "rtc_base/platform_thread_types.h" @@ -34,6 +36,11 @@ class RTC_EXPORT SequenceCheckerImpl { // used exclusively on another thread. void Detach(); + // Returns a string that is formatted to match with the error string printed + // by RTC_CHECK() when a condition is not met. + // This is used in conjunction with the RTC_DCHECK_RUN_ON() macro. + std::string ExpectationToString() const; + private: rtc::CriticalSection lock_; // These are mutable so that IsCurrent can set them. @@ -162,8 +169,19 @@ class RTC_SCOPED_LOCKABLE SequenceCheckerScope { #define RTC_RUN_ON(x) \ RTC_THREAD_ANNOTATION_ATTRIBUTE__(exclusive_locks_required(x)) +namespace webrtc { +std::string ExpectationToString(const webrtc::SequenceChecker* checker); + +// Catch-all implementation for types other than explicitly supported above. +template +std::string ExpectationToString(const ThreadLikeObject*) { + return std::string(); +} + +} // namespace webrtc + #define RTC_DCHECK_RUN_ON(x) \ webrtc::webrtc_seq_check_impl::SequenceCheckerScope seq_check_scope(x); \ - RTC_DCHECK((x)->IsCurrent()) + RTC_DCHECK((x)->IsCurrent()) << webrtc::ExpectationToString(x) #endif // RTC_BASE_SYNCHRONIZATION_SEQUENCE_CHECKER_H_ diff --git a/rtc_base/synchronization/sequence_checker_unittest.cc b/rtc_base/synchronization/sequence_checker_unittest.cc index a173a825bd..6fcb522c54 100644 --- a/rtc_base/synchronization/sequence_checker_unittest.cc +++ b/rtc_base/synchronization/sequence_checker_unittest.cc @@ -31,7 +31,7 @@ class CompileTimeTestForGuardedBy { int CalledOnSequence() RTC_RUN_ON(sequence_checker_) { return guarded_; } void CallMeFromSequence() { - RTC_DCHECK_RUN_ON(&sequence_checker_) << "Should be called on sequence"; + RTC_DCHECK_RUN_ON(&sequence_checker_); guarded_ = 41; }