From 28685dc08cb34f756f9200519fba3222ba3a66f2 Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Thu, 14 May 2020 13:54:17 +0000 Subject: [PATCH] Revert "Make sure that "current" rtc::Thread instances are always current for TaskQueueBase." This reverts commit 46b3bc6c24c233fe41a2401ce6e8eb8204a2d5a8. Reason for revert: Speculative revert. Breaks downstream project Original change's description: > Make sure that "current" rtc::Thread instances are always current for TaskQueueBase. > > This is a necessary part of fulfilling the TaskQueueBase > interface. If a thread does not register as the current TQ, yet offers > the TQ interface, TQ 'current' checks will not work as expected and > code that relies them (TaskQueueBase::Current() and IsCurrent()) > will run in unexpected ways. > > Bug: webrtc:11572 > Change-Id: Iab747bc474e74e6ce4f9e914cfd5b0578b19d19c > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175080 > Reviewed-by: Mirko Bonadei > Commit-Queue: Tommi > Cr-Commit-Position: refs/heads/master@{#31254} TBR=mbonadei@webrtc.org,tommi@webrtc.org Change-Id: I69ff3355f0ec447b25604bd95fdacbdb4d4f3f27 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:11572 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175104 Reviewed-by: Artem Titov Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/master@{#31259} --- api/task_queue/task_queue_test.cc | 6 ++---- rtc_base/thread.cc | 27 +-------------------------- rtc_base/thread.h | 8 -------- rtc_base/thread_unittest.cc | 12 ------------ test/run_loop_unittest.cc | 1 + 5 files changed, 4 insertions(+), 50 deletions(-) diff --git a/api/task_queue/task_queue_test.cc b/api/task_queue/task_queue_test.cc index 3f638b7c69..a8a799f11b 100644 --- a/api/task_queue/task_queue_test.cc +++ b/api/task_queue/task_queue_test.cc @@ -37,11 +37,9 @@ TEST_P(TaskQueueTest, PostAndCheckCurrent) { rtc::Event event; auto queue = CreateTaskQueue(factory, "PostAndCheckCurrent"); - // We're not running a task, so |queue| shouldn't be current. - // Note that because rtc::Thread also supports the TQ interface and - // TestMainImpl::Init wraps the main test thread (bugs.webrtc.org/9714), that - // means that TaskQueueBase::Current() will still return a valid value. + // We're not running a task, so there shouldn't be a current queue. EXPECT_FALSE(queue->IsCurrent()); + EXPECT_FALSE(TaskQueueBase::Current()); queue->PostTask(ToQueuedTask([&event, &queue] { EXPECT_TRUE(queue->IsCurrent()); diff --git a/rtc_base/thread.cc b/rtc_base/thread.cc index 5e48e4b857..0fb2e813e0 100644 --- a/rtc_base/thread.cc +++ b/rtc_base/thread.cc @@ -296,21 +296,6 @@ void ThreadManager::SetCurrentThread(Thread* thread) { RTC_DLOG(LS_ERROR) << "SetCurrentThread: Overwriting an existing value?"; } #endif // RTC_DLOG_IS_ON - - if (thread) { - thread->EnsureIsCurrentTaskQueue(); - } else { - Thread* current = CurrentThread(); - if (current) { - // The current thread is being cleared, e.g. as a result of - // UnwrapCurrent() being called or when a thread is being stopped - // (see PreRun()). This signals that the Thread instance is being detached - // from the thread, which also means that TaskQueue::Current() must not - // return a pointer to the Thread instance. - current->ClearCurrentTaskQueue(); - } - } - SetCurrentThreadInternal(thread); } @@ -839,6 +824,7 @@ void* Thread::PreRun(void* pv) { Thread* thread = static_cast(pv); ThreadManager::Instance()->SetCurrentThread(thread); rtc::SetCurrentThreadName(thread->name_.c_str()); + CurrentTaskQueueSetter set_current_task_queue(thread); #if defined(WEBRTC_MAC) ScopedAutoReleasePool pool; #endif @@ -949,17 +935,6 @@ void Thread::InvokeInternal(const Location& posted_from, Send(posted_from, &handler); } -// Called by the ThreadManager when being set as the current thread. -void Thread::EnsureIsCurrentTaskQueue() { - task_queue_registration_ = - std::make_unique(this); -} - -// Called by the ThreadManager when being set as the current thread. -void Thread::ClearCurrentTaskQueue() { - task_queue_registration_.reset(); -} - void Thread::QueuedTaskHandler::OnMessage(Message* msg) { RTC_DCHECK(msg); auto* data = static_cast*>(msg->pdata); diff --git a/rtc_base/thread.h b/rtc_base/thread.h index e25ed4ea8c..74aab623c8 100644 --- a/rtc_base/thread.h +++ b/rtc_base/thread.h @@ -551,12 +551,6 @@ class RTC_LOCKABLE RTC_EXPORT Thread : public webrtc::TaskQueueBase { void InvokeInternal(const Location& posted_from, rtc::FunctionView functor); - // Called by the ThreadManager when being set as the current thread. - void EnsureIsCurrentTaskQueue(); - - // Called by the ThreadManager when being unset as the current thread. - void ClearCurrentTaskQueue(); - // Returns a static-lifetime MessageHandler which runs message with // MessageLikeTask payload data. static MessageHandler* GetPostTaskMessageHandler(); @@ -601,8 +595,6 @@ class RTC_LOCKABLE RTC_EXPORT Thread : public webrtc::TaskQueueBase { // Runs webrtc::QueuedTask posted to the Thread. QueuedTaskHandler queued_task_handler_; - std::unique_ptr - task_queue_registration_; friend class ThreadManager; diff --git a/rtc_base/thread_unittest.cc b/rtc_base/thread_unittest.cc index e1011f4119..d53a387914 100644 --- a/rtc_base/thread_unittest.cc +++ b/rtc_base/thread_unittest.cc @@ -1148,18 +1148,6 @@ TEST(ThreadPostDelayedTaskTest, InvokesInDelayOrder) { EXPECT_TRUE(fourth.Wait(0)); } -TEST(ThreadPostDelayedTaskTest, IsCurrentTaskQueue) { - auto current_tq = webrtc::TaskQueueBase::Current(); - { - std::unique_ptr thread(rtc::Thread::Create()); - thread->WrapCurrent(); - EXPECT_EQ(webrtc::TaskQueueBase::Current(), - static_cast(thread.get())); - thread->UnwrapCurrent(); - } - EXPECT_EQ(webrtc::TaskQueueBase::Current(), current_tq); -} - class ThreadFactory : public webrtc::TaskQueueFactory { public: std::unique_ptr diff --git a/test/run_loop_unittest.cc b/test/run_loop_unittest.cc index 160aba0716..a356cc265a 100644 --- a/test/run_loop_unittest.cc +++ b/test/run_loop_unittest.cc @@ -17,6 +17,7 @@ namespace webrtc { TEST(RunLoopTest, TaskQueueOnThread) { + EXPECT_EQ(TaskQueueBase::Current(), nullptr); test::RunLoop loop; EXPECT_EQ(TaskQueueBase::Current(), loop.task_queue()); EXPECT_TRUE(loop.task_queue()->IsCurrent());