Prevent data race in MessageQueue.

webrtc/base/messagequeue.cc

Index: webrtc/base/messagequeue.cc
diff --git a/webrtc/base/messagequeue.cc b/webrtc/base/messagequeue.cc
index bbdb941ffab4941107b347023f5ab23e0b762b42..26853f34612379e806191f0709bb4f2ad4bbe33b 100644
--- a/webrtc/base/messagequeue.cc
+++ b/webrtc/base/messagequeue.cc
@@ -159,18 +159,27 @@ void MessageQueue::DoDestroy() {
   SignalQueueDestroyed();
   MessageQueueManager::Remove(this);
   Clear(NULL);
+
+  SharedScope ss(&ss_lock_);
   if (ss_) {
     ss_->SetMessageQueue(NULL);
   }
 }
 
+SocketServer* MessageQueue::socketserver() {
+  SharedScope ss(&ss_lock_);
+  return ss_;
+}
+
 void MessageQueue::set_socketserver(SocketServer* ss) {
+  ExclusiveScope es(&ss_lock_);

[pthatcher1, 2016/02/12 00:16:33]

Can you leave a comment explaining why this place needs an exclusive lock,
whereas none of the others do?

[joachim, 2016/02/12 15:09:52]

Done.

   ss_ = ss ? ss : default_ss_.get();
   ss_->SetMessageQueue(this);
 }
 
 void MessageQueue::Quit() {
   fStop_ = true;
+  SharedScope ss(&ss_lock_);
   ss_->WakeUp();
 }
 
@@ -277,9 +286,12 @@ bool MessageQueue::Get(Message *pmsg, int cmsWait, bool process_io) {
         cmsNext = cmsDelayNext;
     }
 
-    // Wait and multiplex in the meantime
-    if (!ss_->Wait(cmsNext, process_io))
-      return false;
+    {
+      // Wait and multiplex in the meantime
+      SharedScope ss(&ss_lock_);
+      if (!ss_->Wait(cmsNext, process_io))
+        return false;
+    }
 
     // If the specified timeout expired, return
 
@@ -307,16 +319,21 @@ void MessageQueue::Post(MessageHandler* phandler,
   // Add the message to the end of the queue
   // Signal for the multiplexer to return
 
-  CritScope cs(&crit_);
-  Message msg;
-  msg.phandler = phandler;
-  msg.message_id = id;
-  msg.pdata = pdata;
-  if (time_sensitive) {
-    msg.ts_sensitive = Time() + kMaxMsgLatency;
+  {
+    CritScope cs(&crit_);
+    Message msg;
+    msg.phandler = phandler;
+    msg.message_id = id;
+    msg.pdata = pdata;
+    if (time_sensitive) {
+      msg.ts_sensitive = Time() + kMaxMsgLatency;
+    }
+    msgq_.push_back(msg);
+  }
+  {
+    SharedScope ss(&ss_lock_);
+    ss_->WakeUp();
   }
-  msgq_.push_back(msg);
-  ss_->WakeUp();
 }
 
 void MessageQueue::PostDelayed(int cmsDelay,
@@ -345,18 +362,23 @@ void MessageQueue::DoDelayPost(int cmsDelay,
   // Add to the priority queue. Gets sorted soonest first.
   // Signal for the multiplexer to return.
 
-  CritScope cs(&crit_);
-  Message msg;
-  msg.phandler = phandler;
-  msg.message_id = id;
-  msg.pdata = pdata;
-  DelayedMessage dmsg(cmsDelay, tstamp, dmsgq_next_num_, msg);
-  dmsgq_.push(dmsg);
-  // If this message queue processes 1 message every millisecond for 50 days,
-  // we will wrap this number.  Even then, only messages with identical times
-  // will be misordered, and then only briefly.  This is probably ok.
-  VERIFY(0 != ++dmsgq_next_num_);
-  ss_->WakeUp();
+  {
+    CritScope cs(&crit_);
+    Message msg;
+    msg.phandler = phandler;
+    msg.message_id = id;
+    msg.pdata = pdata;
+    DelayedMessage dmsg(cmsDelay, tstamp, dmsgq_next_num_, msg);
+    dmsgq_.push(dmsg);
+    // If this message queue processes 1 message every millisecond for 50 days,
+    // we will wrap this number.  Even then, only messages with identical times
+    // will be misordered, and then only briefly.  This is probably ok.
+    VERIFY(0 != ++dmsgq_next_num_);
+  }
+  {
+    SharedScope ss(&ss_lock_);
+    ss_->WakeUp();
+  }
 }
 
 int MessageQueue::GetDelay() {