Replace reentrant check to be more explicit and avoid a data race when comparing

webrtc/base/messagequeue.cc

 /*
  *  Copyright 2004 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.
  */
 #include <algorithm>
 
 #include "webrtc/base/atomicops.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/common.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/messagequeue.h"
 #include "webrtc/base/stringencode.h"
 #include "webrtc/base/thread.h"
 #include "webrtc/base/trace_event.h"
 
 namespace rtc {
+namespace {
 
 const int kMaxMsgLatency = 150;  // 150 ms
 const int kSlowDispatchLoggingThreshold = 50;  // 50 ms
 
+class SCOPED_LOCKABLE DebugNonReentrantCritScope {
+ public:
+  DebugNonReentrantCritScope(const CriticalSection* cs, bool* locked)
+      EXCLUSIVE_LOCK_FUNCTION(cs)
+      : cs_(cs), locked_(locked) {
+    cs_->Enter();
+    ASSERT(!*locked_);
+    *locked_ = true;
+  }
+
+  ~DebugNonReentrantCritScope() UNLOCK_FUNCTION() {
+    *locked_ = false;
+    cs_->Leave();
+  }
+
+ private:
+  const CriticalSection* const cs_;
+  bool* locked_;
+
+  RTC_DISALLOW_COPY_AND_ASSIGN(DebugNonReentrantCritScope);
+};
+}  // namespace
+
 //------------------------------------------------------------------
 // MessageQueueManager
 
 MessageQueueManager* MessageQueueManager::instance_ = NULL;
 
 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_ != NULL;
 }
 
-MessageQueueManager::MessageQueueManager() {}
+MessageQueueManager::MessageQueueManager() : locked_(false) {}
 
 MessageQueueManager::~MessageQueueManager() {
 }
 
 void MessageQueueManager::Add(MessageQueue *message_queue) {
   return Instance()->AddInternal(message_queue);
 }
 void MessageQueueManager::AddInternal(MessageQueue *message_queue) {
-  // MessageQueueManager methods should be non-reentrant, so we
-  // ASSERT that is the case.  If any of these ASSERT, please
-  // contact bpm or jbeda.
-#if CS_DEBUG_CHECKS  // CurrentThreadIsOwner returns true by default.
-  ASSERT(!crit_.CurrentThreadIsOwner());
-#endif
-  CritScope cs(&crit_);
+  DebugNonReentrantCritScope cs(&crit_, &locked_);
   message_queues_.push_back(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 CS_DEBUG_CHECKS  // CurrentThreadIsOwner returns true by default.
-  ASSERT(!crit_.CurrentThreadIsOwner());  // See note above.
-#endif
   // 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_);
+    DebugNonReentrantCritScope cs(&crit_, &locked_);
     std::vector<MessageQueue *>::iterator iter;
     iter = std::find(message_queues_.begin(), message_queues_.end(),
                      message_queue);
     if (iter != message_queues_.end()) {
       message_queues_.erase(iter);
     }
     destroy = message_queues_.empty();
   }
   if (destroy) {
     instance_ = NULL;
     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) {
-#if CS_DEBUG_CHECKS  // CurrentThreadIsOwner returns true by default.
-  ASSERT(!crit_.CurrentThreadIsOwner());  // See note above.
-#endif
-  CritScope cs(&crit_);
+  DebugNonReentrantCritScope cs(&crit_, &locked_);
   std::vector<MessageQueue *>::iterator iter;
   for (iter = message_queues_.begin(); iter != message_queues_.end(); iter++)
     (*iter)->Clear(handler);
 }
 
 void MessageQueueManager::ProcessAllMessageQueues() {
   if (!instance_) {
     return;
   }
   return Instance()->ProcessAllMessageQueuesInternal();
 }
 
 void MessageQueueManager::ProcessAllMessageQueuesInternal() {
-#if CS_DEBUG_CHECKS  // CurrentThreadIsOwner returns true by default.
-  ASSERT(!crit_.CurrentThreadIsOwner());  // See note above.
-#endif
   // Post a delayed message at the current time and wait for it to be dispatched
   // on all queues, which will ensure that all messages that came before it were
   // also dispatched.
   volatile int queues_not_done;
   auto functor = [&queues_not_done] { AtomicOps::Decrement(&queues_not_done); };
   FunctorMessageHandler<void, decltype(functor)> handler(functor);
   {
-    CritScope cs(&crit_);
+    DebugNonReentrantCritScope cs(&crit_, &locked_);
     queues_not_done = static_cast<int>(message_queues_.size());
     for (MessageQueue* queue : message_queues_) {
       queue->PostDelayed(RTC_FROM_HERE, 0, &handler);
     }
   }
   // Note: One of the message queues may have been on this thread, which is why
   // we can't synchronously wait for queues_not_done to go to 0; we need to
   // process messages as well.
   while (AtomicOps::AcquireLoad(&queues_not_done) > 0) {
     rtc::Thread::Current()->ProcessMessages(0);
@@ skipping 365 matching lines @@
   pmsg->phandler->OnMessage(pmsg);
   int64_t end_time = TimeMillis();
   int64_t diff = TimeDiff(end_time, start_time);
   if (diff >= kSlowDispatchLoggingThreshold) {
     LOG(LS_INFO) << "Message took " << diff << "ms to dispatch. Posted from: "
                  << pmsg->posted_from.ToString();
   }
 }
 
 }  // namespace rtc