From 842a4a6b50beb31293f3a622d54728156977d9e1 Mon Sep 17 00:00:00 2001 From: Tommi Date: Mon, 30 Mar 2015 14:16:12 +0000 Subject: [PATCH] Add locks to Start(), Stop() methods in ProcessThread. This is necessary unfortunately since there are a few places where DeRegisterModule does not reliably occur on the same thread. BUG=4473 R=pbos@webrtc.org Review URL: https://webrtc-codereview.appspot.com/42979004 Cr-Commit-Position: refs/heads/master@{#8891} --- .../utility/source/process_thread_impl.cc | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/webrtc/modules/utility/source/process_thread_impl.cc b/webrtc/modules/utility/source/process_thread_impl.cc index 5b505390da..6ebf4a2f07 100644 --- a/webrtc/modules/utility/source/process_thread_impl.cc +++ b/webrtc/modules/utility/source/process_thread_impl.cc @@ -66,8 +66,15 @@ void ProcessThreadImpl::Start() { DCHECK(!stop_); - for (ModuleCallback& m : modules_) - m.module->ProcessThreadAttached(this); + { + // TODO(tommi): Since DeRegisterModule is currently being called from + // different threads in some cases (ChannelOwner), we need to lock access to + // the modules_ collection even on the controller thread. + // Once we've cleaned up those places, we can remove this lock. + rtc::CritScope lock(&lock_); + for (ModuleCallback& m : modules_) + m.module->ProcessThreadAttached(this); + } thread_ = ThreadWrapper::CreateThread( &ProcessThreadImpl::Run, this, "ProcessThread"); @@ -90,12 +97,18 @@ void ProcessThreadImpl::Stop() { thread_.reset(); stop_ = false; + // TODO(tommi): Since DeRegisterModule is currently being called from + // different threads in some cases (ChannelOwner), we need to lock access to + // the modules_ collection even on the controller thread. + // Once we've cleaned up those places, we can remove this lock. + rtc::CritScope lock(&lock_); for (ModuleCallback& m : modules_) m.module->ProcessThreadAttached(nullptr); } void ProcessThreadImpl::WakeUp(Module* module) { // Allowed to be called on any thread. + // TODO(tommi): Disallow this ^^^ { rtc::CritScope lock(&lock_); for (ModuleCallback& m : modules_) { @@ -108,6 +121,7 @@ void ProcessThreadImpl::WakeUp(Module* module) { void ProcessThreadImpl::PostTask(rtc::scoped_ptr task) { // Allowed to be called on any thread. + // TODO(tommi): Disallow this ^^^ { rtc::CritScope lock(&lock_); queue_.push(task.release());