Fixing a couple cases that cause ProcessAllMessageQueues to hang.

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>
@@ skipping 108 matching lines @@
 }
 
 void MessageQueueManager::ProcessAllMessageQueues() {
   if (!instance_) {
     return;
   }
   return Instance()->ProcessAllMessageQueuesInternal();
 }
 
 void MessageQueueManager::ProcessAllMessageQueuesInternal() {
-  // 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);
+  // This works by posting a delayed message at the current time and waiting
+  // 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 = 0;
+
+  // This class is used so that whether the posted message is processed, or the
+  // message queue is simply cleared, queues_not_done gets decremented.
+  class AutoDecrementer : public MessageData {

[honghaiz3, 2016/09/09 17:59:02]

Why do you replace the original Message handler with a new NoOpHandler and an
AutoDecrementer? 
You replaced two lines of code with two Classes. 
The NoOphandler is not really no op. You just hide the op in the Message
Constructor/Destructor. 

[Taylor Brandstetter, 2016/09/09 23:33:12]

I tried to explain in the comment above. It's needed for the case where the
message queue is cleared (typically due to the thread being destroyed) before
the posted message is processed. This will delete the MessageData, ultimately
decrementing the count, even though the message handler isn't invoked.

As far as the name "NoOpHandler": a MessageHandler is required to delete the
MessageData to avoid memory leaks. So I guess it's not a "no op", but it's the
smallest set of necessary operations. I'm open to suggestions on a name.

+   public:
+    AutoDecrementer(volatile int* value) : value_(value) {}

[pthatcher1, 2016/09/09 17:11:34]

Why not also have the class increment in the constructor?   Then it becomes kind
of like unique_ptr/scoped_ptr, but becomes something like ScopedIncrement.  

[Taylor Brandstetter, 2016/09/09 17:28:32]

Sure, might as well.

+    ~AutoDecrementer() override { AtomicOps::Decrement(value_); }
+
+   private:
+    volatile int* value_;
+  };
+
+  NoOpMessageHandler handler;
   {
     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);
+      if (queue->IsQuitting()) {
+        // If the queue is quitting, it's done processing messages so it can
+        // be ignored. If we tried to post a message to it, it would be dropped.
+        continue;
+      }
+      AtomicOps::Increment(&queues_not_done);
+      queue->PostDelayed(RTC_FROM_HERE, 0, &handler, 0,
+                         new AutoDecrementer(&queues_not_done));
     }
   }
   // 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 366 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