Bug 1304270 - Run FakeOnDeviceChangeEventRunnable on a dedicated thread rather than Camera IPC thread to prevent deadlock. r=pehrsons, a=gchang
authorMunro Chiang <mchiang@mozilla.com>
Fri, 30 Sep 2016 10:44:05 +0800
changeset 355992 01501b3a141277e9fe130bf53a5904ccd0339815
parent 355991 d3c4d153cd34a8b2d0955defc4ee3b476ae67881
child 355993 57ad439e58a9fcfabbedd269e27aa2223e67795f
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspehrsons, gchang
bugs1304270
milestone51.0a2
Bug 1304270 - Run FakeOnDeviceChangeEventRunnable on a dedicated thread rather than Camera IPC thread to prevent deadlock. r=pehrsons, a=gchang MozReview-Commit-ID: HU3wqcG0Gxg
dom/media/systemservices/CamerasChild.cpp
dom/media/systemservices/CamerasChild.h
--- a/dom/media/systemservices/CamerasChild.cpp
+++ b/dom/media/systemservices/CamerasChild.cpp
@@ -29,17 +29,18 @@ mozilla::LazyLogModule gCamerasChildLog(
 #define FAKE_ONDEVICECHANGE_EVENT_REPEAT_COUNT 30
 
 namespace mozilla {
 namespace camera {
 
 CamerasSingleton::CamerasSingleton()
   : mCamerasMutex("CamerasSingleton::mCamerasMutex"),
     mCameras(nullptr),
-    mCamerasChildThread(nullptr) {
+    mCamerasChildThread(nullptr),
+    mFakeDeviceChangeEventThread(nullptr) {
   LOG(("CamerasSingleton: %p", this));
 }
 
 CamerasSingleton::~CamerasSingleton() {
   LOG(("~CamerasSingleton: %p", this));
 }
 
 class FakeOnDeviceChangeEventRunnable : public Runnable
@@ -53,17 +54,17 @@ public:
     OffTheBooksMutexAutoLock lock(CamerasSingleton::Mutex());
 
     CamerasChild* child = CamerasSingleton::Child();
     if (child) {
       child->OnDeviceChange();
 
       if (mCounter++ < FAKE_ONDEVICECHANGE_EVENT_REPEAT_COUNT) {
         RefPtr<FakeOnDeviceChangeEventRunnable> evt = new FakeOnDeviceChangeEventRunnable(mCounter);
-        CamerasSingleton::Thread()->DelayedDispatch(evt.forget(),
+        CamerasSingleton::FakeDeviceChangeEventThread()->DelayedDispatch(evt.forget(),
           FAKE_ONDEVICECHANGE_EVENT_PERIOD_IN_MS);
       }
     }
 
     return NS_OK;
   }
 
 private:
@@ -585,16 +586,24 @@ CamerasChild::ShutdownChild()
                                              &nsIThread::Shutdown));
     CamerasSingleton::Thread()->Dispatch(runnable.forget(), NS_DISPATCH_NORMAL);
   } else {
     LOG(("Shutdown called without PBackground thread"));
   }
   LOG(("Erasing sCameras & thread refs (original thread)"));
   CamerasSingleton::Child() = nullptr;
   CamerasSingleton::Thread() = nullptr;
+
+  if (CamerasSingleton::FakeDeviceChangeEventThread()) {
+    RefPtr<ShutdownRunnable> runnable =
+      new ShutdownRunnable(NewRunnableMethod(CamerasSingleton::FakeDeviceChangeEventThread(),
+                                             &nsIThread::Shutdown));
+    CamerasSingleton::FakeDeviceChangeEventThread()->Dispatch(runnable.forget(), NS_DISPATCH_NORMAL);
+  }
+  CamerasSingleton::FakeDeviceChangeEventThread() = nullptr;
 }
 
 bool
 CamerasChild::RecvDeliverFrame(const int& capEngine,
                                const int& capId,
                                mozilla::ipc::Shmem&& shmem,
                                const size_t& size,
                                const uint32_t& time_stamp,
@@ -623,20 +632,29 @@ CamerasChild::RecvDeviceChange()
   return true;
 }
 
 int
 CamerasChild::SetFakeDeviceChangeEvents()
 {
   CamerasSingleton::Mutex().AssertCurrentThreadOwns();
 
+  if(!CamerasSingleton::FakeDeviceChangeEventThread()) {
+    nsresult rv = NS_NewNamedThread("Fake DC Event",
+                                    getter_AddRefs(CamerasSingleton::FakeDeviceChangeEventThread()));
+    if (NS_FAILED(rv)) {
+      LOG(("Error launching Fake OnDeviceChange Event Thread"));
+      return -1;
+    }
+  }
+
   // To simulate the devicechange event in mochitest,
   // we fire a fake devicechange event in Camera IPC thread periodically
   RefPtr<FakeOnDeviceChangeEventRunnable> evt = new FakeOnDeviceChangeEventRunnable(0);
-  CamerasSingleton::Thread()->Dispatch(evt.forget(), NS_DISPATCH_NORMAL);
+  CamerasSingleton::FakeDeviceChangeEventThread()->Dispatch(evt.forget(), NS_DISPATCH_NORMAL);
 
   return 0;
 }
 
 bool
 CamerasChild::RecvFrameSizeChange(const int& capEngine,
                                   const int& capId,
                                   const int& w, const int& h)
--- a/dom/media/systemservices/CamerasChild.h
+++ b/dom/media/systemservices/CamerasChild.h
@@ -89,32 +89,38 @@ public:
     return gTheInstance.get()->mCameras;
   }
 
   static nsCOMPtr<nsIThread>& Thread() {
     Mutex().AssertCurrentThreadOwns();
     return gTheInstance.get()->mCamerasChildThread;
   }
 
+  static nsCOMPtr<nsIThread>& FakeDeviceChangeEventThread() {
+    Mutex().AssertCurrentThreadOwns();
+    return gTheInstance.get()->mFakeDeviceChangeEventThread;
+  }
+
 private:
   static Singleton<CamerasSingleton> gTheInstance;
 
   // Reinitializing CamerasChild will change the pointers below.
   // We don't want this to happen in the middle of preparing IPC.
   // We will be alive on destruction, so this needs to be off the books.
   mozilla::OffTheBooksMutex mCamerasMutex;
 
   // This is owned by the IPC code, and the same code controls the lifetime.
   // It will set and clear this pointer as appropriate in setup/teardown.
   // We'd normally make this a WeakPtr but unfortunately the IPC code already
   // uses the WeakPtr mixin in a protected base class of CamerasChild, and in
   // any case the object becomes unusable as soon as IPC is tearing down, which
   // will be before actual destruction.
   CamerasChild* mCameras;
   nsCOMPtr<nsIThread> mCamerasChildThread;
+  nsCOMPtr<nsIThread> mFakeDeviceChangeEventThread;
 };
 
 // Get a pointer to a CamerasChild object we can use to do IPC with.
 // This does everything needed to set up, including starting the IPC
 // channel with PBackground, blocking until thats done, and starting the
 // thread to do IPC on. This will fail if we're in shutdown. On success
 // it will set up the CamerasSingleton.
 CamerasChild* GetCamerasChild();