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 <nisse@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24625}

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
+  static MessageQueueManager* const instance = new MessageQueueManager;
-  // spawned.
+  return instance;
-  if (!instance_)
-    instance_ = new MessageQueueManager;
-  return instance_;
-}
-
-bool MessageQueueManager::IsInitialized() {
-  return instance_ != nullptr;
 }
 
 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)) {

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<MessageQueue*> 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
+  // Does not take any lock. Must be called either while holding crit_, or by
-  // destructor.
+  // the destructor (by definition, the latter has exclusive access).
-  void DoDestroy();
+  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();
 

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.

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)