From 40bc7779aa7caa0ecac413d768b89a9315fb87f1 Mon Sep 17 00:00:00 2001 From: "fischman@webrtc.org" Date: Mon, 19 May 2014 17:58:04 +0000 Subject: [PATCH] talk_base: remove lock inversion between MessageQueue and MessageQueueManager. Removes the concept of a MessageQueue being "active" in favor of considering all live MQ's to be active. (previously a MQ was active starting from the first Post to it and stopped being active in its dtor). BUG=3230 R=sriniv@google.com Review URL: https://webrtc-codereview.appspot.com/21489004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6190 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/base/messagequeue.cc | 19 ++++--------------- talk/base/messagequeue.h | 6 +----- talk/base/thread.cc | 4 +--- 3 files changed, 6 insertions(+), 23 deletions(-) diff --git a/talk/base/messagequeue.cc b/talk/base/messagequeue.cc index 64e63ae3dc..7bda92411c 100644 --- a/talk/base/messagequeue.cc +++ b/talk/base/messagequeue.cc @@ -126,7 +126,7 @@ void MessageQueueManager::ClearInternal(MessageHandler *handler) { // MessageQueue MessageQueue::MessageQueue(SocketServer* ss) - : ss_(ss), fStop_(false), fPeekKeep_(false), active_(false), + : ss_(ss), fStop_(false), fPeekKeep_(false), dmsgq_next_num_(0) { if (!ss_) { // Currently, MessageQueue holds a socket server, and is the base class for @@ -138,6 +138,7 @@ MessageQueue::MessageQueue(SocketServer* ss) ss_ = default_ss_.get(); } ss_->SetMessageQueue(this); + MessageQueueManager::Add(this); } MessageQueue::~MessageQueue() { @@ -145,10 +146,8 @@ MessageQueue::~MessageQueue() { // that it always gets called when the queue // is going away. SignalQueueDestroyed(); - if (active_) { - MessageQueueManager::Remove(this); - Clear(NULL); - } + MessageQueueManager::Remove(this); + Clear(NULL); if (ss_) { ss_->SetMessageQueue(NULL); } @@ -296,7 +295,6 @@ void MessageQueue::Post(MessageHandler *phandler, uint32 id, // Signal for the multiplexer to return CritScope cs(&crit_); - EnsureActive(); Message msg; msg.phandler = phandler; msg.message_id = id; @@ -318,7 +316,6 @@ void MessageQueue::DoDelayPost(int cmsDelay, uint32 tstamp, // Signal for the multiplexer to return. CritScope cs(&crit_); - EnsureActive(); Message msg; msg.phandler = phandler; msg.message_id = id; @@ -401,12 +398,4 @@ void MessageQueue::Dispatch(Message *pmsg) { pmsg->phandler->OnMessage(pmsg); } -void MessageQueue::EnsureActive() { - ASSERT(crit_.CurrentThreadIsOwner()); - if (!active_) { - active_ = true; - MessageQueueManager::Add(this); - } -} - } // namespace talk_base diff --git a/talk/base/messagequeue.h b/talk/base/messagequeue.h index fc72166e4b..dc2b5a89a5 100644 --- a/talk/base/messagequeue.h +++ b/talk/base/messagequeue.h @@ -75,7 +75,7 @@ class MessageQueueManager { void ClearInternal(MessageHandler *handler); static MessageQueueManager* instance_; - // This list contains 'active' MessageQueues. + // This list contains all live MessageQueues. std::vector message_queues_; CriticalSection crit_; }; @@ -247,7 +247,6 @@ class MessageQueue { void reheap() { make_heap(c.begin(), c.end(), comp); } }; - void EnsureActive(); void DoDelayPost(int cmsDelay, uint32 tstamp, MessageHandler *phandler, uint32 id, MessageData* pdata); @@ -258,9 +257,6 @@ class MessageQueue { bool fStop_; bool fPeekKeep_; Message msgPeek_; - // A message queue is active if it has ever had a message posted to it. - // This also corresponds to being in MessageQueueManager's global list. - bool active_; MessageList msgq_; PriorityQueue dmsgq_; uint32 dmsgq_next_num_; diff --git a/talk/base/thread.cc b/talk/base/thread.cc index d4cebc4d55..3fd1ca4bba 100644 --- a/talk/base/thread.cc +++ b/talk/base/thread.cc @@ -157,8 +157,7 @@ Thread::Thread(SocketServer* ss) Thread::~Thread() { Stop(); - if (active_) - Clear(NULL); + Clear(NULL); } bool Thread::SleepMs(int milliseconds) { @@ -403,7 +402,6 @@ void Thread::Send(MessageHandler *phandler, uint32 id, MessageData *pdata) { bool ready = false; { CritScope cs(&crit_); - EnsureActive(); _SendMessage smsg; smsg.thread = current_thread; smsg.msg = msg;