From 3f7990d38b1ad8d4ae2d8e31385f727560f58b2a Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Wed, 3 Feb 2021 16:23:47 +0100 Subject: [PATCH] Split sequence checker on two headers before moving to API Bug: webrtc:12419 Change-Id: I8d5acfec0c0654efc70ca089dc6a862503939220 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/205524 Commit-Queue: Artem Titov Reviewed-by: Danil Chapovalov Reviewed-by: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#33152} --- rtc_base/synchronization/BUILD.gn | 18 +++- rtc_base/synchronization/sequence_checker.h | 84 ++++------------- ...hecker.cc => sequence_checker_internal.cc} | 23 +++-- .../sequence_checker_internal.h | 93 +++++++++++++++++++ 4 files changed, 136 insertions(+), 82 deletions(-) rename rtc_base/synchronization/{sequence_checker.cc => sequence_checker_internal.cc} (92%) create mode 100644 rtc_base/synchronization/sequence_checker_internal.h diff --git a/rtc_base/synchronization/BUILD.gn b/rtc_base/synchronization/BUILD.gn index a37779b031..29e46b4936 100644 --- a/rtc_base/synchronization/BUILD.gn +++ b/rtc_base/synchronization/BUILD.gn @@ -44,15 +44,25 @@ rtc_library("mutex") { } } -rtc_library("sequence_checker") { +rtc_source_set("sequence_checker") { + sources = [ "sequence_checker.h" ] + deps = [ + ":sequence_checker_internal", + "..:checks", + "..:deprecation", + "..:macromagic", + ] +} + +rtc_library("sequence_checker_internal") { + visibility = [ ":sequence_checker" ] sources = [ - "sequence_checker.cc", - "sequence_checker.h", + "sequence_checker_internal.cc", + "sequence_checker_internal.h", ] deps = [ ":mutex", "..:checks", - "..:criticalsection", "..:macromagic", "..:platform_thread_types", "..:stringutils", diff --git a/rtc_base/synchronization/sequence_checker.h b/rtc_base/synchronization/sequence_checker.h index ecf8490cec..31314a477a 100644 --- a/rtc_base/synchronization/sequence_checker.h +++ b/rtc_base/synchronization/sequence_checker.h @@ -12,53 +12,12 @@ #include -#include "api/task_queue/task_queue_base.h" -#include "rtc_base/platform_thread_types.h" -#include "rtc_base/synchronization/mutex.h" -#include "rtc_base/system/rtc_export.h" +#include "rtc_base/checks.h" +#include "rtc_base/deprecation.h" +#include "rtc_base/synchronization/sequence_checker_internal.h" #include "rtc_base/thread_annotations.h" namespace webrtc { -// Real implementation of SequenceChecker, for use in debug mode, or -// for temporary use in release mode (e.g. to RTC_CHECK on a threading issue -// seen only in the wild). -// -// Note: You should almost always use the SequenceChecker class to get the -// right version for your build configuration. -class RTC_EXPORT SequenceCheckerImpl { - public: - SequenceCheckerImpl(); - ~SequenceCheckerImpl(); - - bool IsCurrent() const; - // Changes the task queue or thread that is checked for in IsCurrent. This can - // be useful when an object may be created on one task queue / thread and then - // 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: - mutable Mutex lock_; - // These are mutable so that IsCurrent can set them. - mutable bool attached_ RTC_GUARDED_BY(lock_); - mutable rtc::PlatformThreadRef valid_thread_ RTC_GUARDED_BY(lock_); - mutable const TaskQueueBase* valid_queue_ RTC_GUARDED_BY(lock_); - mutable const void* valid_system_queue_ RTC_GUARDED_BY(lock_); -}; - -// Do nothing implementation, for use in release mode. -// -// Note: You should almost always use the SequenceChecker class to get the -// right version for your build configuration. -class SequenceCheckerDoNothing { - public: - bool IsCurrent() const { return true; } - void Detach() {} -}; // SequenceChecker is a helper class used to help verify that some methods // of a class are called on the same task queue or thread. A @@ -80,27 +39,16 @@ class SequenceCheckerDoNothing { // // In Release mode, IsCurrent will always return true. #if RTC_DCHECK_IS_ON -class RTC_LOCKABLE SequenceChecker : public SequenceCheckerImpl {}; +class RTC_LOCKABLE SequenceChecker + : public webrtc_sequence_checker_internal::SequenceCheckerImpl {}; #else -class RTC_LOCKABLE SequenceChecker : public SequenceCheckerDoNothing {}; +class RTC_LOCKABLE SequenceChecker + : public webrtc_sequence_checker_internal::SequenceCheckerDoNothing {}; #endif // RTC_ENABLE_THREAD_CHECKER namespace webrtc_seq_check_impl { -// Helper class used by RTC_DCHECK_RUN_ON (see example usage below). -class RTC_SCOPED_LOCKABLE SequenceCheckerScope { - public: - template - explicit SequenceCheckerScope(const ThreadLikeObject* thread_like_object) - RTC_EXCLUSIVE_LOCK_FUNCTION(thread_like_object) {} - SequenceCheckerScope(const SequenceCheckerScope&) = delete; - SequenceCheckerScope& operator=(const SequenceCheckerScope&) = delete; - ~SequenceCheckerScope() RTC_UNLOCK_FUNCTION() {} - - template - static bool IsCurrent(const ThreadLikeObject* thread_like_object) { - return thread_like_object->IsCurrent(); - } -}; +// TODO(titovartem): Remove when downstream deps are updated +using ::webrtc::webrtc_sequence_checker_internal::SequenceCheckerScope; } // namespace webrtc_seq_check_impl } // namespace webrtc @@ -170,18 +118,18 @@ class RTC_SCOPED_LOCKABLE SequenceCheckerScope { 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. +// Deprecated. Do not use. template -std::string ExpectationToString(const ThreadLikeObject*) { - return std::string(); +RTC_DEPRECATED std::string ExpectationToString(const ThreadLikeObject* o) { + return webrtc::webrtc_sequence_checker_internal::ExpectationToString(o); } } // namespace webrtc -#define RTC_DCHECK_RUN_ON(x) \ - webrtc::webrtc_seq_check_impl::SequenceCheckerScope seq_check_scope(x); \ - RTC_DCHECK((x)->IsCurrent()) << webrtc::ExpectationToString(x) +#define RTC_DCHECK_RUN_ON(x) \ + webrtc::webrtc_sequence_checker_internal::SequenceCheckerScope scope(x); \ + RTC_DCHECK((x)->IsCurrent()) \ + << webrtc::webrtc_sequence_checker_internal::ExpectationToString(x) #endif // RTC_BASE_SYNCHRONIZATION_SEQUENCE_CHECKER_H_ diff --git a/rtc_base/synchronization/sequence_checker.cc b/rtc_base/synchronization/sequence_checker_internal.cc similarity index 92% rename from rtc_base/synchronization/sequence_checker.cc rename to rtc_base/synchronization/sequence_checker_internal.cc index 1de26cf0fe..7b66d8020a 100644 --- a/rtc_base/synchronization/sequence_checker.cc +++ b/rtc_base/synchronization/sequence_checker_internal.cc @@ -7,15 +7,19 @@ * in the file PATENTS. All contributing project authors may * be found in the AUTHORS file in the root of the source tree. */ -#include "rtc_base/synchronization/sequence_checker.h" +#include "rtc_base/synchronization/sequence_checker_internal.h" + +#include #if defined(WEBRTC_MAC) #include #endif +#include "rtc_base/checks.h" #include "rtc_base/strings/string_builder.h" namespace webrtc { +namespace webrtc_sequence_checker_internal { namespace { // On Mac, returns the label of the current dispatch queue; elsewhere, return // null. @@ -29,21 +33,12 @@ const void* GetSystemQueueRef() { } // 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()), valid_queue_(TaskQueueBase::Current()), valid_system_queue_(GetSystemQueueRef()) {} -SequenceCheckerImpl::~SequenceCheckerImpl() = default; - bool SequenceCheckerImpl::IsCurrent() const { const TaskQueueBase* const current_queue = TaskQueueBase::Current(); const rtc::PlatformThreadRef current_thread = rtc::CurrentThreadRef(); @@ -109,4 +104,12 @@ std::string SequenceCheckerImpl::ExpectationToString() const { } #endif // RTC_DCHECK_IS_ON +std::string ExpectationToString(const SequenceCheckerImpl* checker) { +#if RTC_DCHECK_IS_ON + return checker->ExpectationToString(); +#endif + return std::string(); +} + +} // namespace webrtc_sequence_checker_internal } // namespace webrtc diff --git a/rtc_base/synchronization/sequence_checker_internal.h b/rtc_base/synchronization/sequence_checker_internal.h new file mode 100644 index 0000000000..f7ac6de125 --- /dev/null +++ b/rtc_base/synchronization/sequence_checker_internal.h @@ -0,0 +1,93 @@ +/* + * Copyright 2020 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ +#ifndef RTC_BASE_SYNCHRONIZATION_SEQUENCE_CHECKER_INTERNAL_H_ +#define RTC_BASE_SYNCHRONIZATION_SEQUENCE_CHECKER_INTERNAL_H_ + +#include +#include + +#include "api/task_queue/task_queue_base.h" +#include "rtc_base/platform_thread_types.h" +#include "rtc_base/synchronization/mutex.h" +#include "rtc_base/system/rtc_export.h" +#include "rtc_base/thread_annotations.h" + +namespace webrtc { +namespace webrtc_sequence_checker_internal { + +// Real implementation of SequenceChecker, for use in debug mode, or +// for temporary use in release mode (e.g. to RTC_CHECK on a threading issue +// seen only in the wild). +// +// Note: You should almost always use the SequenceChecker class to get the +// right version for your build configuration. +class RTC_EXPORT SequenceCheckerImpl { + public: + SequenceCheckerImpl(); + ~SequenceCheckerImpl() = default; + + bool IsCurrent() const; + // Changes the task queue or thread that is checked for in IsCurrent. This can + // be useful when an object may be created on one task queue / thread and then + // 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: + mutable Mutex lock_; + // These are mutable so that IsCurrent can set them. + mutable bool attached_ RTC_GUARDED_BY(lock_); + mutable rtc::PlatformThreadRef valid_thread_ RTC_GUARDED_BY(lock_); + mutable const TaskQueueBase* valid_queue_ RTC_GUARDED_BY(lock_); + mutable const void* valid_system_queue_ RTC_GUARDED_BY(lock_); +}; + +// Do nothing implementation, for use in release mode. +// +// Note: You should almost always use the SequenceChecker class to get the +// right version for your build configuration. +class SequenceCheckerDoNothing { + public: + bool IsCurrent() const { return true; } + void Detach() {} +}; + +// Helper class used by RTC_DCHECK_RUN_ON (see example usage below). +class RTC_SCOPED_LOCKABLE SequenceCheckerScope { + public: + template + explicit SequenceCheckerScope(const ThreadLikeObject* thread_like_object) + RTC_EXCLUSIVE_LOCK_FUNCTION(thread_like_object) {} + SequenceCheckerScope(const SequenceCheckerScope&) = delete; + SequenceCheckerScope& operator=(const SequenceCheckerScope&) = delete; + ~SequenceCheckerScope() RTC_UNLOCK_FUNCTION() {} + + template + static bool IsCurrent(const ThreadLikeObject* thread_like_object) { + return thread_like_object->IsCurrent(); + } +}; + +std::string ExpectationToString(const SequenceCheckerImpl* checker); + +// Catch-all implementation for types other than explicitly supported above. +template +std::string ExpectationToString(const ThreadLikeObject*) { + return std::string(); +} + +} // namespace webrtc_sequence_checker_internal +} // namespace webrtc + +#endif // RTC_BASE_SYNCHRONIZATION_SEQUENCE_CHECKER_INTERNAL_H_