Bug 1200614 - Protect the capture engines array from concurrent access (during shutdown). r=jesup
authorGian-Carlo Pascutto <gcp@mozilla.com>
Wed, 02 Sep 2015 18:41:06 +0200
changeset 260611 b45b4b3f1ed5e7df24e62551e6a5e9fdd5c1acc9
parent 260610 d92d88957742e5aa095bb4563e8108ffc5744b78
child 260612 384ff965768a40a70907eb491c42543c6431bd76
push id29318
push usercbook@mozilla.com
push dateThu, 03 Sep 2015 11:15:07 +0000
treeherdermozilla-central@74fbd245369c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs1200614
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1200614 - Protect the capture engines array from concurrent access (during shutdown). r=jesup
dom/media/systemservices/CamerasParent.cpp
dom/media/systemservices/CamerasParent.h
--- a/dom/media/systemservices/CamerasParent.cpp
+++ b/dom/media/systemservices/CamerasParent.cpp
@@ -319,31 +319,34 @@ CamerasParent::CloseEngines()
         RecvStopCapture(capEngine, capNum);
         RecvReleaseCaptureDevice(capEngine, capNum);
       }
       // The callbacks list might have changed while we released the lock,
       // but note that due to the loop construct this will not break us.
     }
   }
 
-  for (int i = 0; i < CaptureEngine::MaxEngine; i++) {
-    if (mEngines[i].mEngineIsRunning) {
-      LOG(("Being closed down while engine %d is running!", i));
-    }
-    if (mEngines[i].mPtrViERender) {
-      mEngines[i].mPtrViERender->Release();
-      mEngines[i].mPtrViERender = nullptr;
-    }
-    if (mEngines[i].mPtrViECapture) {
-      mEngines[i].mPtrViECapture->Release();
-      mEngines[i].mPtrViECapture = nullptr;
-    }
-    if(mEngines[i].mPtrViEBase) {
-      mEngines[i].mPtrViEBase->Release();
-      mEngines[i].mPtrViEBase = nullptr;
+  {
+    MutexAutoLock lock(mEngineMutex);
+    for (int i = 0; i < CaptureEngine::MaxEngine; i++) {
+      if (mEngines[i].mEngineIsRunning) {
+        LOG(("Being closed down while engine %d is running!", i));
+      }
+      if (mEngines[i].mPtrViERender) {
+        mEngines[i].mPtrViERender->Release();
+        mEngines[i].mPtrViERender = nullptr;
+      }
+      if (mEngines[i].mPtrViECapture) {
+        mEngines[i].mPtrViECapture->Release();
+        mEngines[i].mPtrViECapture = nullptr;
+      }
+      if(mEngines[i].mPtrViEBase) {
+        mEngines[i].mPtrViEBase->Release();
+        mEngines[i].mPtrViEBase = nullptr;
+      }
     }
   }
 }
 
 bool
 CamerasParent::EnsureInitialized(int aEngine)
 {
   LOG((__PRETTY_FUNCTION__));
@@ -368,16 +371,17 @@ CamerasParent::RecvNumberOfCaptureDevice
     LOG(("RecvNumberOfCaptureDevices fails to initialize"));
     unused << SendReplyFailure();
     return false;
   }
 
   nsRefPtr<CamerasParent> self(this);
   nsRefPtr<nsRunnable> webrtc_runnable =
     media::NewRunnableFrom([self, aCapEngine]() -> nsresult {
+      MutexAutoLock lock(self->mEngineMutex);
       int num = self->mEngines[aCapEngine].mPtrViECapture->NumberOfCaptureDevices();
       nsRefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, num]() -> nsresult {
           if (self->IsShuttingDown()) {
             return NS_ERROR_FAILURE;
           }
           if (num < 0) {
             LOG(("RecvNumberOfCaptureDevices couldn't find devices"));
@@ -407,16 +411,17 @@ CamerasParent::RecvNumberOfCapabilities(
     unused << SendReplyFailure();
     return false;
   }
 
   LOG(("Getting caps for %s", unique_id.get()));
   nsRefPtr<CamerasParent> self(this);
   nsRefPtr<nsRunnable> webrtc_runnable =
     media::NewRunnableFrom([self, unique_id, aCapEngine]() -> nsresult {
+      MutexAutoLock lock(self->mEngineMutex);
       int num =
         self->mEngines[aCapEngine].mPtrViECapture->NumberOfCapabilities(
          unique_id.get(),
           MediaEngineSource::kMaxUniqueIdLength);
       nsRefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, num]() -> nsresult {
           if (self->IsShuttingDown()) {
             return NS_ERROR_FAILURE;
@@ -451,16 +456,17 @@ CamerasParent::RecvGetCaptureCapability(
   }
 
   LOG(("RecvGetCaptureCapability: %s %d", unique_id.get(), num));
 
   nsRefPtr<CamerasParent> self(this);
   nsRefPtr<nsRunnable> webrtc_runnable =
     media::NewRunnableFrom([self, unique_id, aCapEngine, num]() -> nsresult {
       webrtc::CaptureCapability webrtcCaps;
+      MutexAutoLock lock(self->mEngineMutex);
       int error = self->mEngines[aCapEngine].mPtrViECapture->GetCaptureCapability(
         unique_id.get(), MediaEngineSource::kMaxUniqueIdLength, num, webrtcCaps);
       nsRefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, webrtcCaps, error]() -> nsresult {
           if (self->IsShuttingDown()) {
             return NS_ERROR_FAILURE;
           }
           CaptureCapability capCap(webrtcCaps.width,
@@ -505,17 +511,17 @@ CamerasParent::RecvGetCaptureDevice(cons
   LOG(("RecvGetCaptureDevice"));
   nsRefPtr<CamerasParent> self(this);
   nsRefPtr<nsRunnable> webrtc_runnable =
     media::NewRunnableFrom([self, aCapEngine, aListNumber]() -> nsresult {
       char deviceName[MediaEngineSource::kMaxDeviceNameLength];
       char deviceUniqueId[MediaEngineSource::kMaxUniqueIdLength];
       nsCString name;
       nsCString uniqueId;
-
+      MutexAutoLock lock(self->mEngineMutex);
       int error =
         self->mEngines[aCapEngine].mPtrViECapture->GetCaptureDevice(aListNumber,
                                                                     deviceName,
                                                                     sizeof(deviceName),
                                                                     deviceUniqueId,
                                                                     sizeof(deviceUniqueId));
       if (!error) {
         name.Assign(deviceName);
@@ -554,16 +560,17 @@ CamerasParent::RecvAllocateCaptureDevice
     unused << SendReplyFailure();
     return false;
   }
 
   nsRefPtr<CamerasParent> self(this);
   nsRefPtr<nsRunnable> webrtc_runnable =
     media::NewRunnableFrom([self, aCapEngine, unique_id]() -> nsresult {
       int numdev;
+      MutexAutoLock lock(self->mEngineMutex);
       int error = self->mEngines[aCapEngine].mPtrViECapture->AllocateCaptureDevice(
         unique_id.get(), MediaEngineSource::kMaxUniqueIdLength, numdev);
       nsRefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, numdev, error]() -> nsresult {
           if (self->IsShuttingDown()) {
             return NS_ERROR_FAILURE;
           }
           if (error) {
@@ -592,16 +599,17 @@ CamerasParent::RecvReleaseCaptureDevice(
     unused << SendReplyFailure();
     return false;
   }
 
   nsRefPtr<CamerasParent> self(this);
   nsRefPtr<nsRunnable> webrtc_runnable =
     media::NewRunnableFrom([self, aCapEngine, numdev]() -> nsresult {
       LOG(("RecvReleaseCamera device nr %d", numdev));
+      MutexAutoLock lock(self->mEngineMutex);
       int error = self->mEngines[aCapEngine].mPtrViECapture->ReleaseCaptureDevice(numdev);
       nsRefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, error, numdev]() -> nsresult {
           if (self->IsShuttingDown()) {
             return NS_ERROR_FAILURE;
           }
           if (error) {
             unused << self->SendReplyFailure();
@@ -634,45 +642,51 @@ CamerasParent::RecvStartCapture(const in
     LOG(("Failure to initialize"));
     unused << SendReplyFailure();
     return false;
   }
 
   nsRefPtr<CamerasParent> self(this);
   nsRefPtr<nsRunnable> webrtc_runnable =
     media::NewRunnableFrom([self, aCapEngine, capnum, ipcCaps]() -> nsresult {
-      MutexAutoLock lock(self->mCallbackMutex);
-      auto cbh = self->mCallbacks.AppendElement(
-        new CallbackHelper(static_cast<CaptureEngine>(aCapEngine), capnum, self));
-      auto render = static_cast<webrtc::ExternalRenderer*>(*cbh);
-
-      EngineHelper* helper = &self->mEngines[aCapEngine];
-
-      int error =
-        helper->mPtrViERender->AddRenderer(capnum, webrtc::kVideoI420, render);
-
-      if (!error) {
-        error = helper->mPtrViERender->StartRender(capnum);
+      CallbackHelper** cbh;
+      webrtc::ExternalRenderer* render;
+      EngineHelper* helper;
+      int error;
+      {
+        MutexAutoLock lockCallback(self->mCallbackMutex);
+        cbh = self->mCallbacks.AppendElement(
+          new CallbackHelper(static_cast<CaptureEngine>(aCapEngine), capnum, self));
+        render = static_cast<webrtc::ExternalRenderer*>(*cbh);
       }
+      {
+        MutexAutoLock lockEngine(self->mEngineMutex);
+        helper = &self->mEngines[aCapEngine];
+        error =
+          helper->mPtrViERender->AddRenderer(capnum, webrtc::kVideoI420, render);
 
-      webrtc::CaptureCapability capability;
-      capability.width = ipcCaps.width();
-      capability.height = ipcCaps.height();
-      capability.maxFPS = ipcCaps.maxFPS();
-      capability.expectedCaptureDelay = ipcCaps.expectedCaptureDelay();
-      capability.rawType = static_cast<webrtc::RawVideoType>(ipcCaps.rawType());
-      capability.codecType = static_cast<webrtc::VideoCodecType>(ipcCaps.codecType());
-      capability.interlaced = ipcCaps.interlaced();
+        if (!error) {
+          error = helper->mPtrViERender->StartRender(capnum);
+        }
 
-      if (!error) {
-        error = helper->mPtrViECapture->StartCapture(capnum, capability);
-      }
+        webrtc::CaptureCapability capability;
+        capability.width = ipcCaps.width();
+        capability.height = ipcCaps.height();
+        capability.maxFPS = ipcCaps.maxFPS();
+        capability.expectedCaptureDelay = ipcCaps.expectedCaptureDelay();
+        capability.rawType = static_cast<webrtc::RawVideoType>(ipcCaps.rawType());
+        capability.codecType = static_cast<webrtc::VideoCodecType>(ipcCaps.codecType());
+        capability.interlaced = ipcCaps.interlaced();
 
-      if (!error) {
-        helper->mEngineIsRunning = true;
+        if (!error) {
+          error = helper->mPtrViECapture->StartCapture(capnum, capability);
+        }
+        if (!error) {
+          helper->mEngineIsRunning = true;
+        }
       }
 
       nsRefPtr<nsIRunnable> ipc_runnable =
         media::NewRunnableFrom([self, error]() -> nsresult {
           if (self->IsShuttingDown()) {
             return NS_ERROR_FAILURE;
           }
           if (!error) {
@@ -699,21 +713,23 @@ CamerasParent::RecvStopCapture(const int
     LOG(("Failure to initialize"));
     unused << SendReplyFailure();
     return false;
   }
 
   nsRefPtr<CamerasParent> self(this);
   nsRefPtr<nsRunnable> webrtc_runnable =
     media::NewRunnableFrom([self, aCapEngine, capnum]() -> nsresult {
-      self->mEngines[aCapEngine].mPtrViECapture->StopCapture(capnum);
-      self->mEngines[aCapEngine].mPtrViERender->StopRender(capnum);
-      self->mEngines[aCapEngine].mPtrViERender->RemoveRenderer(capnum);
-      self->mEngines[aCapEngine].mEngineIsRunning = false;
-
+      {
+        MutexAutoLock lock(self->mEngineMutex);
+        self->mEngines[aCapEngine].mPtrViECapture->StopCapture(capnum);
+        self->mEngines[aCapEngine].mPtrViERender->StopRender(capnum);
+        self->mEngines[aCapEngine].mPtrViERender->RemoveRenderer(capnum);
+        self->mEngines[aCapEngine].mEngineIsRunning = false;
+      }
       MutexAutoLock lock(self->mCallbackMutex);
       for (unsigned int i = 0; i < self->mCallbacks.Length(); i++) {
         if (self->mCallbacks[i]->mCapEngine == aCapEngine
             && self->mCallbacks[i]->mCapturerId == capnum) {
           delete self->mCallbacks[i];
           self->mCallbacks.RemoveElementAt(i);
           break;
         }
@@ -734,21 +750,24 @@ CamerasParent::RecvAllDone()
   return Send__delete__(this);
 }
 
 void CamerasParent::DoShutdown()
 {
   LOG((__PRETTY_FUNCTION__));
   CloseEngines();
 
-  for (int i = 0; i < CaptureEngine::MaxEngine; i++) {
-    if (mEngines[i].mEngine) {
-      mEngines[i].mEngine->SetTraceCallback(nullptr);
-      webrtc::VideoEngine::Delete(mEngines[i].mEngine);
-      mEngines[i].mEngine = nullptr;
+  {
+    MutexAutoLock lock(mEngineMutex);
+    for (int i = 0; i < CaptureEngine::MaxEngine; i++) {
+      if (mEngines[i].mEngine) {
+        mEngines[i].mEngine->SetTraceCallback(nullptr);
+        webrtc::VideoEngine::Delete(mEngines[i].mEngine);
+        mEngines[i].mEngine = nullptr;
+      }
     }
   }
 
   mShmemPool.Cleanup(this);
 
   mPBackgroundThread = nullptr;
 
   if (mVideoCaptureThread) {
@@ -769,16 +788,17 @@ CamerasParent::ActorDestroy(ActorDestroy
   // forward them anymore anyway.
   mChildIsAlive = false;
   mDestroyed = true;
   CloseEngines();
 }
 
 CamerasParent::CamerasParent()
   : mCallbackMutex("CamerasParent.mCallbackMutex"),
+    mEngineMutex("CamerasParent.mEngineMutex"),
     mShmemPool(CaptureEngine::MaxEngine),
     mVideoCaptureThread(nullptr),
     mChildIsAlive(true),
     mDestroyed(false)
 {
   if (!gCamerasParentLog) {
     gCamerasParentLog = PR_NewLogModule("CamerasParent");
   }
--- a/dom/media/systemservices/CamerasParent.h
+++ b/dom/media/systemservices/CamerasParent.h
@@ -117,16 +117,18 @@ protected:
   void CloseEngines();
   bool EnsureInitialized(int aEngine);
   void DoShutdown();
 
   EngineHelper mEngines[CaptureEngine::MaxEngine];
   nsTArray<CallbackHelper*> mCallbacks;
   // Protects the callback arrays
   Mutex mCallbackMutex;
+  // Protects the engines array
+  Mutex mEngineMutex;
 
   // image buffers
   mozilla::ShmemPool mShmemPool;
 
   // PBackground parent thread
   nsCOMPtr<nsIThread> mPBackgroundThread;
 
   // video processing thread - where webrtc.org capturer code runs