Bug 1426129 - Hold CamerasChild via promoting "this" to a RefPtr. r=pehrsons, a=RyanVM
authorPaul Adenot <paul@paul.cx>
Mon, 16 Apr 2018 10:51:48 +0200
changeset 357085 3f2ec03c04055850e5b11f8836632c9d7a973115
parent 357084 621034a42acc8bd3a8249542bb4b452bb831084f
child 357086 f729bf78fb3a178c216ac4517d1fad81d53ff3ef
push id7663
push userryanvm@gmail.com
push dateMon, 30 Apr 2018 18:44:18 +0000
treeherdermozilla-esr52@f729bf78fb3a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspehrsons, RyanVM
bugs1426129
milestone52.8.0
Bug 1426129 - Hold CamerasChild via promoting "this" to a RefPtr. r=pehrsons, a=RyanVM
dom/media/systemservices/CamerasChild.cpp
dom/media/systemservices/CamerasChild.h
--- a/dom/media/systemservices/CamerasChild.cpp
+++ b/dom/media/systemservices/CamerasChild.cpp
@@ -30,17 +30,19 @@ mozilla::LazyLogModule gCamerasChildLog(
 
 namespace mozilla {
 namespace camera {
 
 CamerasSingleton::CamerasSingleton()
   : mCamerasMutex("CamerasSingleton::mCamerasMutex"),
     mCameras(nullptr),
     mCamerasChildThread(nullptr),
-    mFakeDeviceChangeEventThread(nullptr) {
+    mFakeDeviceChangeEventThread(nullptr),
+    mInShutdown(false)
+{
   LOG(("CamerasSingleton: %p", this));
 }
 
 CamerasSingleton::~CamerasSingleton() {
   LOG(("~CamerasSingleton: %p", this));
 }
 
 class FakeOnDeviceChangeEventRunnable : public Runnable
@@ -280,16 +282,17 @@ CamerasChild::DispatchToParent(nsIRunnab
 
 int
 CamerasChild::NumberOfCapabilities(CaptureEngine aCapEngine,
                                    const char* deviceUniqueIdUTF8)
 {
   LOG((__PRETTY_FUNCTION__));
   LOG(("NumberOfCapabilities for %s", deviceUniqueIdUTF8));
   nsCString unique_id(deviceUniqueIdUTF8);
+  RefPtr<CamerasChild> deathGrip = this;
   nsCOMPtr<nsIRunnable> runnable =
     mozilla::NewNonOwningRunnableMethod<CaptureEngine, nsCString>
     (this, &CamerasChild::SendNumberOfCapabilities, aCapEngine, unique_id);
   LockAndDispatch<> dispatcher(this, __func__, runnable, 0, mReplyInteger);
   LOG(("Capture capability count: %d", dispatcher.ReturnValue()));
   return dispatcher.ReturnValue();
 }
 
@@ -316,31 +319,33 @@ CamerasChild::RecvReplyNumberOfCaptureDe
   monitor.Notify();
   return true;
 }
 
 int
 CamerasChild::EnsureInitialized(CaptureEngine aCapEngine)
 {
   LOG((__PRETTY_FUNCTION__));
+  RefPtr<CamerasChild> deathGrip = this;
   nsCOMPtr<nsIRunnable> runnable =
     mozilla::NewNonOwningRunnableMethod<CaptureEngine>
     (this, &CamerasChild::SendEnsureInitialized, aCapEngine);
   LockAndDispatch<> dispatcher(this, __func__, runnable, 0, mReplyInteger);
   LOG(("Capture Devices: %d", dispatcher.ReturnValue()));
   return dispatcher.ReturnValue();
 }
 
 int
 CamerasChild::GetCaptureCapability(CaptureEngine aCapEngine,
                                    const char* unique_idUTF8,
                                    const unsigned int capability_number,
                                    webrtc::CaptureCapability& capability)
 {
   LOG(("GetCaptureCapability: %s %d", unique_idUTF8, capability_number));
+  RefPtr<CamerasChild> deathGrip = this;
   nsCString unique_id(unique_idUTF8);
   nsCOMPtr<nsIRunnable> runnable =
     mozilla::NewNonOwningRunnableMethod<CaptureEngine, nsCString, unsigned int>
     (this, &CamerasChild::SendGetCaptureCapability, aCapEngine, unique_id, capability_number);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   if (dispatcher.Success()) {
     capability = mReplyCapability;
   }
@@ -369,16 +374,17 @@ int
 CamerasChild::GetCaptureDevice(CaptureEngine aCapEngine,
                                unsigned int list_number, char* device_nameUTF8,
                                const unsigned int device_nameUTF8Length,
                                char* unique_idUTF8,
                                const unsigned int unique_idUTF8Length,
                                bool* scary)
 {
   LOG((__PRETTY_FUNCTION__));
+  RefPtr<CamerasChild> deathGrip = this;
   nsCOMPtr<nsIRunnable> runnable =
     mozilla::NewNonOwningRunnableMethod<CaptureEngine, unsigned int>
     (this, &CamerasChild::SendGetCaptureDevice, aCapEngine, list_number);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   if (dispatcher.Success()) {
     base::strlcpy(device_nameUTF8, mReplyDeviceName.get(), device_nameUTF8Length);
     base::strlcpy(unique_idUTF8, mReplyDeviceID.get(), unique_idUTF8Length);
     if (scary) {
@@ -408,16 +414,17 @@ CamerasChild::RecvReplyGetCaptureDevice(
 int
 CamerasChild::AllocateCaptureDevice(CaptureEngine aCapEngine,
                                     const char* unique_idUTF8,
                                     const unsigned int unique_idUTF8Length,
                                     int& capture_id,
                                     const nsACString& aOrigin)
 {
   LOG((__PRETTY_FUNCTION__));
+  RefPtr<CamerasChild> deathGrip = this;
   nsCString unique_id(unique_idUTF8);
   nsCString origin(aOrigin);
   nsCOMPtr<nsIRunnable> runnable =
     mozilla::NewNonOwningRunnableMethod<CaptureEngine, nsCString, nsCString>
     (this, &CamerasChild::SendAllocateCaptureDevice, aCapEngine, unique_id, origin);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   if (dispatcher.Success()) {
     LOG(("Capture Device allocated: %d", mReplyInteger));
@@ -439,16 +446,17 @@ CamerasChild::RecvReplyAllocateCaptureDe
   return true;
 }
 
 int
 CamerasChild::ReleaseCaptureDevice(CaptureEngine aCapEngine,
                                    const int capture_id)
 {
   LOG((__PRETTY_FUNCTION__));
+  RefPtr<CamerasChild> deathGrip = this;
   nsCOMPtr<nsIRunnable> runnable =
     mozilla::NewNonOwningRunnableMethod<CaptureEngine, int>
     (this, &CamerasChild::SendReleaseCaptureDevice, aCapEngine, capture_id);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   return dispatcher.ReturnValue();
 }
 
 void
@@ -486,27 +494,29 @@ CamerasChild::StartCapture(CaptureEngine
   AddCallback(aCapEngine, capture_id, cb);
   CaptureCapability capCap(webrtcCaps.width,
                            webrtcCaps.height,
                            webrtcCaps.maxFPS,
                            webrtcCaps.expectedCaptureDelay,
                            webrtcCaps.rawType,
                            webrtcCaps.codecType,
                            webrtcCaps.interlaced);
+  RefPtr<CamerasChild> deathGrip = this;
   nsCOMPtr<nsIRunnable> runnable =
     mozilla::NewNonOwningRunnableMethod<CaptureEngine, int, CaptureCapability>
     (this, &CamerasChild::SendStartCapture, aCapEngine, capture_id, capCap);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   return dispatcher.ReturnValue();
 }
 
 int
 CamerasChild::StopCapture(CaptureEngine aCapEngine, const int capture_id)
 {
   LOG((__PRETTY_FUNCTION__));
+  RefPtr<CamerasChild> deathGrip = this;
   nsCOMPtr<nsIRunnable> runnable =
     mozilla::NewNonOwningRunnableMethod<CaptureEngine, int>
     (this, &CamerasChild::SendStopCapture, aCapEngine, capture_id);
   LockAndDispatch<> dispatcher(this, __func__, runnable);
   if (dispatcher.Success()) {
     RemoveCallback(aCapEngine, capture_id);
   }
   return dispatcher.ReturnValue();
@@ -562,16 +572,17 @@ CamerasChild::ShutdownParent()
     mIPCIsAlive = false;
     monitor.NotifyAll();
   }
   if (CamerasSingleton::Thread()) {
     LOG(("Dispatching actor deletion"));
     // Delete the parent actor.
     // CamerasChild (this) will remain alive and is only deleted by the
     // IPC layer when SendAllDone returns.
+    RefPtr<CamerasChild> deathGrip = this;
     nsCOMPtr<nsIRunnable> deleteRunnable =
       mozilla::NewNonOwningRunnableMethod(this, &CamerasChild::SendAllDone);
     CamerasSingleton::Thread()->Dispatch(deleteRunnable, NS_DISPATCH_NORMAL);
   } else {
     LOG(("ShutdownParent called without PBackground thread"));
   }
 }
 
@@ -690,17 +701,17 @@ CamerasChild::CamerasChild()
 
   MOZ_COUNT_CTOR(CamerasChild);
 }
 
 CamerasChild::~CamerasChild()
 {
   LOG(("~CamerasChild: %p", this));
 
-  {
+  if (!CamerasSingleton::InShutdown()) {
     OffTheBooksMutexAutoLock lock(CamerasSingleton::Mutex());
     // In normal circumstances we've already shut down and the
     // following does nothing. But on fatal IPC errors we will
     // get destructed immediately, and should not try to reach
     // the parent.
     ShutdownChild();
   }
 
--- a/dom/media/systemservices/CamerasChild.h
+++ b/dom/media/systemservices/CamerasChild.h
@@ -84,16 +84,24 @@ public:
     return gTheInstance.get()->mCamerasChildThread;
   }
 
   static nsCOMPtr<nsIThread>& FakeDeviceChangeEventThread() {
     Mutex().AssertCurrentThreadOwns();
     return gTheInstance.get()->mFakeDeviceChangeEventThread;
   }
 
+  static bool InShutdown() {
+    return gTheInstance.get()->mInShutdown;
+  }
+
+  static void StartShutdown() {
+    gTheInstance.get()->mInShutdown = true;
+  }
+
 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;
 
@@ -101,16 +109,17 @@ private:
   // 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;
+  Atomic<bool> mInShutdown;
 };
 
 // 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();
@@ -140,17 +149,17 @@ class CamerasChild final : public PCamer
                           ,public DeviceChangeCallback
 {
   friend class mozilla::ipc::BackgroundChildImpl;
   template <class T> friend class mozilla::camera::LockAndDispatch;
 
 public:
   // We are owned by the PBackground thread only. CamerasSingleton
   // takes a non-owning reference.
-  NS_INLINE_DECL_REFCOUNTING(CamerasChild)
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CamerasChild)
 
   // IPC messages recevied, received on the PBackground thread
   // these are the actual callbacks with data
   virtual bool RecvDeliverFrame(const CaptureEngine&, const int&, mozilla::ipc::Shmem&&,
                                 const size_t&, const uint32_t&, const int64_t&,
                                 const int64_t&) override;
   virtual bool RecvFrameSizeChange(const CaptureEngine&, const int&,
                                    const int& w, const int& h) override;