From 4447aba5a41d23fe91cf1a7a07630d2c929e06a9 Mon Sep 17 00:00:00 2001 From: Andreas Pehrson Date: Tue, 23 May 2023 06:14:38 +0000 Subject: [PATCH] Bug 1832751 - cherry-pick upstream libwebrtc commit 301e546a68. r=webrtc-reviewers,mjf Upstream commit: https://webrtc.googlesource.com/src/+/301e546a689020320f919a660591759e993ef051 Remove SequenceCheckerImpl::valid_system_queue_ As pointed out in issue webrtc:15146 this Mac/iOS specific variable, makes the SequenceChecker behave incorrectly on those platforms. The variable was introduced in a CL that merged the previous checker classes, ThreadChecker and SequencedTaskChecker, but curiously neither one of them had such a variable. So I'm not exactly sure what problem was being solved. Hence I'm wondering if we actually need it. Reference: https://webrtc-review.googlesource.com/c/src/+/129721 Bug: webrtc:15146 Change-Id: Ia7a9eb17b993c4f8a1e8204c658bf0b3dbdaa1e0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304401 Reviewed-by: Peter Hanspers Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#40019} Differential Revision: https://phabricator.services.mozilla.com/D177875 --- .../api/sequence_checker_unittest.cc | 13 +++++++ .../sequence_checker_internal.cc | 37 +++---------------- .../sequence_checker_internal.h | 1 - 3 files changed, 18 insertions(+), 33 deletions(-) diff --git a/third_party/libwebrtc/api/sequence_checker_unittest.cc b/third_party/libwebrtc/api/sequence_checker_unittest.cc index 3efb5c78eed8..f117926d7363 100644 --- a/third_party/libwebrtc/api/sequence_checker_unittest.cc +++ b/third_party/libwebrtc/api/sequence_checker_unittest.cc @@ -98,6 +98,19 @@ TEST(SequenceCheckerTest, MethodNotAllowedOnDifferentThreadInDebug) { [&] { EXPECT_EQ(sequence_checker.IsCurrent(), !RTC_DCHECK_IS_ON); }); } +#if RTC_DCHECK_IS_ON +TEST(SequenceCheckerTest, OnlyCurrentOnOneThread) { + SequenceChecker sequence_checker(SequenceChecker::kDetached); + RunOnDifferentThread([&] { + EXPECT_TRUE(sequence_checker.IsCurrent()); + // Spawn a new thread from within the first one to guarantee that we have + // two concurrently active threads (and that there's no chance of the + // thread ref being reused). + RunOnDifferentThread([&] { EXPECT_FALSE(sequence_checker.IsCurrent()); }); + }); +} +#endif + TEST(SequenceCheckerTest, MethodNotAllowedOnDifferentTaskQueueInDebug) { SequenceChecker sequence_checker; TaskQueueForTest queue; diff --git a/third_party/libwebrtc/rtc_base/synchronization/sequence_checker_internal.cc b/third_party/libwebrtc/rtc_base/synchronization/sequence_checker_internal.cc index 9831f07d7ddd..3e205b91d501 100644 --- a/third_party/libwebrtc/rtc_base/synchronization/sequence_checker_internal.cc +++ b/third_party/libwebrtc/rtc_base/synchronization/sequence_checker_internal.cc @@ -11,52 +11,30 @@ #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. -const void* GetSystemQueueRef() { -#if defined(WEBRTC_MAC) - return dispatch_queue_get_label(DISPATCH_CURRENT_QUEUE_LABEL); -#else - return nullptr; -#endif -} - -} // namespace SequenceCheckerImpl::SequenceCheckerImpl(bool attach_to_current_thread) : attached_(attach_to_current_thread), valid_thread_(rtc::CurrentThreadRef()), - valid_queue_(TaskQueueBase::Current()), - valid_system_queue_(GetSystemQueueRef()) {} + valid_queue_(TaskQueueBase::Current()) {} bool SequenceCheckerImpl::IsCurrent() const { const TaskQueueBase* const current_queue = TaskQueueBase::Current(); const rtc::PlatformThreadRef current_thread = rtc::CurrentThreadRef(); - const void* const current_system_queue = GetSystemQueueRef(); MutexLock scoped_lock(&lock_); if (!attached_) { // Previously detached. attached_ = true; valid_thread_ = current_thread; valid_queue_ = current_queue; - valid_system_queue_ = current_system_queue; return true; } if (valid_queue_) { return valid_queue_ == current_queue; } - if (valid_system_queue_ && valid_system_queue_ == current_system_queue) { - return true; - } return rtc::IsThreadRefEqual(valid_thread_, current_thread); } @@ -71,7 +49,6 @@ void SequenceCheckerImpl::Detach() { 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(); MutexLock scoped_lock(&lock_); if (!attached_) return "Checker currently not attached."; @@ -85,17 +62,13 @@ std::string SequenceCheckerImpl::ExpectationToString() const { rtc::StringBuilder message; message.AppendFormat( - "# Expected: TQ: %p SysQ: %p Thread: %p\n" - "# Actual: TQ: %p SysQ: %p Thread: %p\n", - valid_queue_, valid_system_queue_, - reinterpret_cast(valid_thread_), current_queue, - current_system_queue, reinterpret_cast(current_thread)); + "# Expected: TQ: %p Thread: %p\n" + "# Actual: TQ: %p Thread: %p\n", + valid_queue_, reinterpret_cast(valid_thread_), current_queue, + reinterpret_cast(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"; } diff --git a/third_party/libwebrtc/rtc_base/synchronization/sequence_checker_internal.h b/third_party/libwebrtc/rtc_base/synchronization/sequence_checker_internal.h index a66e9ee8ec9c..22503027a560 100644 --- a/third_party/libwebrtc/rtc_base/synchronization/sequence_checker_internal.h +++ b/third_party/libwebrtc/rtc_base/synchronization/sequence_checker_internal.h @@ -50,7 +50,6 @@ class RTC_EXPORT SequenceCheckerImpl { 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.