From 5e007b77f15835147ff6c30126f30efffef0a445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 7 Sep 2018 12:35:44 +0200 Subject: [PATCH] Use function-local static variable for MessageQueueManager singleton. Rely on C++11 thread-safe initialization on first call to MessageQueueManager::Instance(), in the same way as for ThreadManager::Instance(). Bug: None Change-Id: I26244f90c5d7f94a2454688297f55bf96617e78c Reviewed-on: https://webrtc-review.googlesource.com/97721 Commit-Queue: Niels Moller Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#24625} --- rtc_base/messagequeue.cc | 41 ++++++------------------------- rtc_base/messagequeue.h | 19 +++++++------- rtc_base/messagequeue_unittest.cc | 16 ------------ rtc_base/thread.cc | 2 +- 4 files changed, 18 insertions(+), 60 deletions(-) diff --git a/rtc_base/messagequeue.cc b/rtc_base/messagequeue.cc index 6ff73b5d65..a561af42eb 100644 --- a/rtc_base/messagequeue.cc +++ b/rtc_base/messagequeue.cc @@ -48,18 +48,9 @@ class RTC_SCOPED_LOCKABLE MarkProcessingCritScope { //------------------------------------------------------------------ // MessageQueueManager -MessageQueueManager* MessageQueueManager::instance_ = nullptr; - MessageQueueManager* MessageQueueManager::Instance() { - // Note: This is not thread safe, but it is first called before threads are - // spawned. - if (!instance_) - instance_ = new MessageQueueManager; - return instance_; -} - -bool MessageQueueManager::IsInitialized() { - return instance_ != nullptr; + static MessageQueueManager* const instance = new MessageQueueManager; + return instance; } MessageQueueManager::MessageQueueManager() : processing_(0) {} @@ -77,18 +68,9 @@ void MessageQueueManager::AddInternal(MessageQueue* message_queue) { } void MessageQueueManager::Remove(MessageQueue* message_queue) { - // If there isn't a message queue manager instance, then there isn't a queue - // to remove. - if (!instance_) - return; return Instance()->RemoveInternal(message_queue); } void MessageQueueManager::RemoveInternal(MessageQueue* message_queue) { - // If this is the last MessageQueue, destroy the manager as well so that - // we don't leak this object at program shutdown. As mentioned above, this is - // not thread-safe, but this should only happen at program termination (when - // the ThreadManager is destroyed, and threads are no longer active). - bool destroy = false; { CritScope cs(&crit_); // Prevent changes while the list of message queues is processed. @@ -99,19 +81,10 @@ void MessageQueueManager::RemoveInternal(MessageQueue* message_queue) { if (iter != message_queues_.end()) { message_queues_.erase(iter); } - destroy = message_queues_.empty(); - } - if (destroy) { - instance_ = nullptr; - delete this; } } void MessageQueueManager::Clear(MessageHandler* handler) { - // If there isn't a message queue manager instance, then there aren't any - // queues to remove this handler from. - if (!instance_) - return; return Instance()->ClearInternal(handler); } void MessageQueueManager::ClearInternal(MessageHandler* handler) { @@ -125,9 +98,6 @@ void MessageQueueManager::ClearInternal(MessageHandler* handler) { } void MessageQueueManager::ProcessAllMessageQueuesForTesting() { - if (!instance_) { - return; - } return Instance()->ProcessAllMessageQueuesInternal(); } @@ -225,7 +195,7 @@ void MessageQueue::DoDestroy() { // is going away. SignalQueueDestroyed(); MessageQueueManager::Remove(this); - Clear(nullptr); + ClearInternal(nullptr, MQID_ANY, nullptr); if (ss_) { ss_->SetMessageQueue(nullptr); @@ -480,7 +450,12 @@ void MessageQueue::Clear(MessageHandler* phandler, uint32_t id, MessageList* removed) { CritScope cs(&crit_); + ClearInternal(phandler, id, removed); +} +void MessageQueue::ClearInternal(MessageHandler* phandler, + uint32_t id, + MessageList* removed) { // Remove messages with phandler if (fPeekKeep_ && msgPeek_.Match(phandler, id)) { diff --git a/rtc_base/messagequeue.h b/rtc_base/messagequeue.h index d64ea86cbe..e7e479285d 100644 --- a/rtc_base/messagequeue.h +++ b/rtc_base/messagequeue.h @@ -43,12 +43,6 @@ class MessageQueueManager { static void Remove(MessageQueue* message_queue); static void Clear(MessageHandler* handler); - // For testing purposes, we expose whether or not the MessageQueueManager - // instance has been initialized. It has no other use relative to the rest of - // the functions of this class, which auto-initialize the underlying - // MessageQueueManager instance when necessary. - static bool IsInitialized(); - // TODO(nisse): Delete alias, as soon as downstream code is updated. static void ProcessAllMessageQueues() { ProcessAllMessageQueuesForTesting(); } @@ -68,7 +62,6 @@ class MessageQueueManager { void ClearInternal(MessageHandler* handler); void ProcessAllMessageQueuesInternal(); - static MessageQueueManager* instance_; // This list contains all live MessageQueues. std::vector message_queues_ RTC_GUARDED_BY(crit_); @@ -305,9 +298,15 @@ class MessageQueue { // if false was passed as init_queue to the MessageQueue constructor. void DoInit(); - // Perform cleanup, subclasses that override Clear must call this from the - // destructor. - void DoDestroy(); + // Does not take any lock. Must be called either while holding crit_, or by + // the destructor (by definition, the latter has exclusive access). + void ClearInternal(MessageHandler* phandler, + uint32_t id, + MessageList* removed) RTC_EXCLUSIVE_LOCKS_REQUIRED(&crit_); + + // Perform cleanup; subclasses must call this from the destructor, + // and are not expected to actually hold the lock. + void DoDestroy() RTC_EXCLUSIVE_LOCKS_REQUIRED(&crit_); void WakeUpSocketServer(); diff --git a/rtc_base/messagequeue_unittest.cc b/rtc_base/messagequeue_unittest.cc index b031bdebd1..d8e8b1173e 100644 --- a/rtc_base/messagequeue_unittest.cc +++ b/rtc_base/messagequeue_unittest.cc @@ -134,22 +134,6 @@ struct UnwrapMainThreadScope { bool rewrap_; }; -TEST(MessageQueueManager, Clear) { - UnwrapMainThreadScope s; - if (MessageQueueManager::IsInitialized()) { - RTC_LOG(LS_INFO) - << "Unable to run MessageQueueManager::Clear test, since the " - << "MessageQueueManager was already initialized by some " - << "other test in this run."; - return; - } - bool deleted = false; - DeletedMessageHandler* handler = new DeletedMessageHandler(&deleted); - delete handler; - EXPECT_TRUE(deleted); - EXPECT_FALSE(MessageQueueManager::IsInitialized()); -} - // Ensure that ProcessAllMessageQueues does its essential function; process // all messages (both delayed and non delayed) up until the current time, on // all registered message queues. diff --git a/rtc_base/thread.cc b/rtc_base/thread.cc index 61aea9086a..2d5704e0be 100644 --- a/rtc_base/thread.cc +++ b/rtc_base/thread.cc @@ -482,7 +482,7 @@ void Thread::Clear(MessageHandler* phandler, ++iter; } - MessageQueue::Clear(phandler, id, removed); + ClearInternal(phandler, id, removed); } #if !defined(WEBRTC_MAC)