Bug 1418165 - Protect sVideoCaptureThread with sThreadMonitor. r?jib draft
authorMunro Mengjue Chiang <mchiang@mozilla.com>
Fri, 17 Nov 2017 09:56:55 +0800
changeset 699397 2f08c5fac5b3c4988ebd4e230bdd6a5ddda6b63b
parent 699396 c765c5e39ee94ff30cdc5ef94226fc4a91003fd5
child 699733 c8c66d63d01e5fa7cb806a3b69e21bdb55a3cd69
push id89558
push userbmo:mchiang@mozilla.com
push dateFri, 17 Nov 2017 05:20:12 +0000
reviewersjib
bugs1418165
milestone59.0a1
Bug 1418165 - Protect sVideoCaptureThread with sThreadMonitor. r?jib MozReview-Commit-ID: I6PJQ4bbPt8
dom/media/systemservices/CamerasParent.cpp
dom/media/systemservices/CamerasParent.h
--- a/dom/media/systemservices/CamerasParent.cpp
+++ b/dom/media/systemservices/CamerasParent.cpp
@@ -181,21 +181,20 @@ CamerasParent::Observe(nsISupports *aSub
 }
 
 nsresult
 CamerasParent::DispatchToVideoCaptureThread(Runnable* event)
 {
   // Don't try to dispatch if we're already on the right thread.
   // There's a potential deadlock because the sThreadMonitor is likely
   // to be taken already.
+  MonitorAutoLock lock(*sThreadMonitor);
   MOZ_ASSERT(!sVideoCaptureThread ||
              sVideoCaptureThread->thread_id() != PlatformThread::CurrentId());
 
-  MonitorAutoLock lock(*sThreadMonitor);
-
   while(mChildIsAlive && mWebRTCAlive &&
         (!sVideoCaptureThread || !sVideoCaptureThread->IsRunning())) {
     sThreadMonitor->Wait();
   }
   if (!sVideoCaptureThread || !sVideoCaptureThread->IsRunning()) {
     return NS_ERROR_FAILURE;
   }
   RefPtr<Runnable> addrefedEvent = event;
@@ -334,17 +333,16 @@ CamerasParent::RecvReleaseFrame(mozilla:
   mShmemPool.Put(ShmemBuffer(s));
   return IPC_OK();
 }
 
 bool
 CamerasParent::SetupEngine(CaptureEngine aCapEngine)
 {
   LOG((__PRETTY_FUNCTION__));
-  MOZ_ASSERT(sVideoCaptureThread->thread_id() == PlatformThread::CurrentId());
   RefPtr<mozilla::camera::VideoEngine>* engine = &sEngines[aCapEngine];
 
   if (!engine->get()) {
     webrtc::CaptureDeviceInfo *captureDeviceInfo = nullptr;
     UniquePtr<webrtc::Config> config(new webrtc::Config);
 
     switch (aCapEngine) {
     case ScreenEngine:
--- a/dom/media/systemservices/CamerasParent.h
+++ b/dom/media/systemservices/CamerasParent.h
@@ -133,18 +133,18 @@ protected:
   void StopIPC();
   void StopVideoCapture();
   // Can't take already_AddRefed because it can fail in stupid ways.
   nsresult DispatchToVideoCaptureThread(Runnable* event);
 
   // sEngines will be accessed by VideoCapture thread only
   // sNumOfCamerasParent, sNumOfOpenCamerasParentEngines, and sVideoCaptureThread will
   // be accessed by main thread / PBackground thread / VideoCapture thread
-  // all variables are protected by sThreadMonitor while sThreadMonitor
-  // creation and deletion is protected by sMutex
+  // sNumOfCamerasParent and sThreadMonitor create & delete are protected by sMutex
+  // sNumOfOpenCamerasParentEngines and sVideoCaptureThread are protected by sThreadMonitor
   static RefPtr<VideoEngine> sEngines[CaptureEngine::MaxEngine];
   static int32_t sNumOfOpenCamerasParentEngines;
   static int32_t sNumOfCamerasParents;
   nsTArray<CallbackHelper*> mCallbacks;
 
   // image buffers
   mozilla::ShmemPool mShmemPool;