Fix race between Thread ctor/dtor and MessageQueueManager registrations.

webrtc/base/messagequeue.cc

Index: webrtc/base/messagequeue.cc
diff --git a/webrtc/base/messagequeue.cc b/webrtc/base/messagequeue.cc
index 857cf12927cd89eb6cea31210065edf3574e5b24..0efecff6850cd705c25b343681087dfb988f6212 100644
--- a/webrtc/base/messagequeue.cc
+++ b/webrtc/base/messagequeue.cc
@@ -116,9 +116,9 @@ void MessageQueueManager::ClearInternal(MessageHandler *handler) {
 //------------------------------------------------------------------
 // MessageQueue
 
-MessageQueue::MessageQueue(SocketServer* ss)
+MessageQueue::MessageQueue(SocketServer* ss, bool add_to_queue_manager)
     : ss_(ss), fStop_(false), fPeekKeep_(false),
-      dmsgq_next_num_(0) {
+      dmsgq_next_num_(0), fDestroyed_(false) {
   if (!ss_) {
     // Currently, MessageQueue holds a socket server, and is the base class for
     // Thread.  It seems like it makes more sense for Thread to hold the socket
@@ -129,10 +129,21 @@ MessageQueue::MessageQueue(SocketServer* ss)
     ss_ = default_ss_.get();
   }
   ss_->SetMessageQueue(this);
-  MessageQueueManager::Add(this);
+  if (add_to_queue_manager) {
+    MessageQueueManager::Add(this);

[pthatcher1, 2016/02/03 23:51:47]

If ww want to call MessageQueueManager::Add before ss_->SetMessageQueue(this) in
side of Thread(), why not just always do that?  What's so special about
Thread()?

Also, why is that necessary to fix the race condition?

[joachim, 2016/02/04 00:01:20]

With this call flow:
1. Start of Thread ctor
2. MessageQueue ctor
3. Object is added to MessageQueueManager
4. Remaining Thread ctor

If between 3 and 4 a different thread calls MessageQueueManager::Clear, the
partially created object will be used and a race happens.

+  }
 }
 
 MessageQueue::~MessageQueue() {
+  DoDestroy();
+}
+
+void MessageQueue::DoDestroy() {
+  if (fDestroyed_) {
+    return;
+  }
+
+  fDestroyed_ = true;
   // The signal is done from here to ensure
   // that it always gets called when the queue
   // is going away.