Protect MessageQueue stop field with a critical section to avoid data races.

webrtc/base/messagequeue.cc

Index: webrtc/base/messagequeue.cc
diff --git a/webrtc/base/messagequeue.cc b/webrtc/base/messagequeue.cc
index da50e2304f7a686af4245df733566528d279d185..455edd809b3994c77e6e04e8958a3f1830d22e20 100644
--- a/webrtc/base/messagequeue.cc
+++ b/webrtc/base/messagequeue.cc
@@ -133,8 +133,12 @@ void MessageQueueManager::OnMessage(Message* pmsg) {
 //------------------------------------------------------------------
 // MessageQueue
 MessageQueue::MessageQueue(SocketServer* ss, bool init_queue)
-    : fStop_(false), fPeekKeep_(false),
-      dmsgq_next_num_(0), fInitialized_(false), fDestroyed_(false), ss_(ss) {
+    : fPeekKeep_(false),
+      dmsgq_next_num_(0),
+      fInitialized_(false),
+      fDestroyed_(false),
+      stop_(false),
+      ss_(ss) {
   RTC_DCHECK(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
@@ -206,16 +210,21 @@ void MessageQueue::WakeUpSocketServer() {
 }
 
 void MessageQueue::Quit() {
-  fStop_ = true;
+  {
+    CritScope cs(&stop_crit_);
+    stop_ = true;

[pthatcher1, 2016/06/01 15:29:14]

Might as well rename "stop_" to "quitting_".

+  }
   WakeUpSocketServer();
 }
 
 bool MessageQueue::IsQuitting() {
-  return fStop_;
+  CritScope cs(&stop_crit_);

[pthatcher1, 2016/06/01 15:29:14]

Should we use an std::atomic here once they are allowed?  Perhaps with a comment
similar to the one in thread_unittest.cc above AtomicBool.

[tommi (sloooow) - chröme, 2016/06/01 15:53:29]

We do have alternatives to std::atomic that we can use instead.  See
webrtc/base/atomicops.h.

+  return stop_;
 }
 
 void MessageQueue::Restart() {
-  fStop_ = false;
+  CritScope cs(&stop_crit_);
+  stop_ = false;
 }
 
 bool MessageQueue::Peek(Message *pmsg, int cmsWait) {
@@ -299,7 +308,7 @@ bool MessageQueue::Get(Message *pmsg, int cmsWait, bool process_io) {
       return true;
     }
 
-    if (fStop_)
+    if (IsQuitting())
       break;
 
     // Which is shorter, the delay wait or the asked wait?
@@ -339,7 +348,7 @@ void MessageQueue::Post(MessageHandler* phandler,
                         uint32_t id,
                         MessageData* pdata,
                         bool time_sensitive) {
-  if (fStop_)
+  if (IsQuitting())
     return;
 
   // Keep thread safe
@@ -388,7 +397,7 @@ void MessageQueue::DoDelayPost(int64_t cmsDelay,
                                MessageHandler* phandler,
                                uint32_t id,
                                MessageData* pdata) {
-  if (fStop_) {
+  if (IsQuitting()) {
     return;
   }