Bug 1224442: null-check GMP Parent Shmem messages from the Child to handle messages after shutdown r=cpearce
authorRandell Jesup <rjesup@jesup.org>
Fri, 13 Nov 2015 01:08:01 -0500
changeset 272387 09dc1434c390cf5843e7c30f95e8b1abe20b9fb2
parent 272386 77d4a5db9002feb4f921e8cd658940f41248875b
child 272388 80a79987da8b7d11e63c40c1cdd7da63297d0782
push id29671
push usercbook@mozilla.com
push dateFri, 13 Nov 2015 11:07:02 +0000
treeherdermozilla-central@faf815a0fa9b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce
bugs1224442
milestone45.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 1224442: null-check GMP Parent Shmem messages from the Child to handle messages after shutdown r=cpearce
dom/media/gmp/GMPVideoDecoderParent.cpp
dom/media/gmp/GMPVideoEncoderParent.cpp
dom/media/gmp/GMPVideoHost.cpp
--- a/dom/media/gmp/GMPVideoDecoderParent.cpp
+++ b/dom/media/gmp/GMPVideoDecoderParent.cpp
@@ -273,17 +273,16 @@ GMPVideoDecoderParent::Shutdown()
 // Note: Keep this sync'd up with Shutdown
 void
 GMPVideoDecoderParent::ActorDestroy(ActorDestroyReason aWhy)
 {
   LOGD(("GMPVideoDecoderParent[%p]::ActorDestroy reason=%d", this, aWhy));
 
   mIsOpen = false;
   mActorDestroyed = true;
-  mVideoHost.DoneWithAPI();
 
   // Ensure if we've received a destroy while waiting for a ResetComplete
   // or DrainComplete notification, we'll unblock the caller before processing
   // the error.
   UnblockResetAndDrain();
 
   if (mCallback) {
     // May call Close() (and Shutdown()) immediately or with a delay
--- a/dom/media/gmp/GMPVideoEncoderParent.cpp
+++ b/dom/media/gmp/GMPVideoEncoderParent.cpp
@@ -224,17 +224,16 @@ GMPVideoEncoderParent::Shutdown()
   }
   mShuttingDown = true;
 
   // Notify client we're gone!  Won't occur after Close()
   if (mCallback) {
     mCallback->Terminated();
     mCallback = nullptr;
   }
-  mVideoHost.DoneWithAPI();
 
   mIsOpen = false;
   if (!mActorDestroyed) {
     Unused << SendEncodingComplete();
   }
 }
 
 static void
@@ -264,17 +263,17 @@ GMPVideoEncoderParent::ActorDestroy(Acto
                      nsCOMPtr<nsIThread> >(&ShutdownEncodedThread, mEncodedThread));
     mEncodedThread = nullptr;
   }
   if (mPlugin) {
     // Ignore any return code. It is OK for this to fail without killing the process.
     mPlugin->VideoEncoderDestroyed(this);
     mPlugin = nullptr;
   }
-  mVideoHost.ActorDestroyed();
+  mVideoHost.ActorDestroyed(); // same as DoneWithAPI
 }
 
 static void
 EncodedCallback(GMPVideoEncoderCallbackProxy* aCallback,
                 GMPVideoEncodedFrame* aEncodedFrame,
                 nsTArray<uint8_t>* aCodecSpecificInfo,
                 nsCOMPtr<nsIThread> aThread)
 {
@@ -326,29 +325,39 @@ GMPVideoEncoderParent::RecvShutdown()
   Shutdown();
   return true;
 }
 
 bool
 GMPVideoEncoderParent::RecvParentShmemForPool(Shmem&& aFrameBuffer)
 {
   if (aFrameBuffer.IsWritable()) {
-    mVideoHost.SharedMemMgr()->MgrDeallocShmem(GMPSharedMem::kGMPFrameData,
-                                               aFrameBuffer);
+    // This test may be paranoia now that we don't shut down the VideoHost
+    // in ::Shutdown, but doesn't hurt
+    if (mVideoHost.SharedMemMgr()) {
+      mVideoHost.SharedMemMgr()->MgrDeallocShmem(GMPSharedMem::kGMPFrameData,
+                                                 aFrameBuffer);
+    } else {
+      LOGD(("%s::%s: %p Called in shutdown, ignoring and freeing directly", __CLASS__, __FUNCTION__, this));
+      DeallocShmem(aFrameBuffer);
+    }
   }
   return true;
 }
 
 bool
 GMPVideoEncoderParent::AnswerNeedShmem(const uint32_t& aEncodedBufferSize,
                                        Shmem* aMem)
 {
   ipc::Shmem mem;
 
-  if (!mVideoHost.SharedMemMgr()->MgrAllocShmem(GMPSharedMem::kGMPEncodedData,
+  // This test may be paranoia now that we don't shut down the VideoHost
+  // in ::Shutdown, but doesn't hurt
+  if (!mVideoHost.SharedMemMgr() ||
+      !mVideoHost.SharedMemMgr()->MgrAllocShmem(GMPSharedMem::kGMPEncodedData,
                                                 aEncodedBufferSize,
                                                 ipc::SharedMemory::TYPE_BASIC, &mem))
   {
     LOG(LogLevel::Error, ("%s::%s: Failed to get a shared mem buffer for Child! size %u",
                        __CLASS__, __FUNCTION__, aEncodedBufferSize));
     return false;
   }
   *aMem = mem;
--- a/dom/media/gmp/GMPVideoHost.cpp
+++ b/dom/media/gmp/GMPVideoHost.cpp
@@ -66,45 +66,38 @@ GMPVideoHostImpl::CreatePlane(GMPPlane**
 }
 
 GMPSharedMemManager*
 GMPVideoHostImpl::SharedMemMgr()
 {
   return mSharedMemMgr;
 }
 
+// XXX This should merge with ActorDestroyed
 void
 GMPVideoHostImpl::DoneWithAPI()
 {
+  ActorDestroyed();
+}
+
+void
+GMPVideoHostImpl::ActorDestroyed()
+{
   for (uint32_t i = mPlanes.Length(); i > 0; i--) {
     mPlanes[i - 1]->DoneWithAPI();
     mPlanes.RemoveElementAt(i - 1);
   }
   for (uint32_t i = mEncodedFrames.Length(); i > 0; i--) {
     mEncodedFrames[i - 1]->DoneWithAPI();
     mEncodedFrames.RemoveElementAt(i - 1);
   }
   mSharedMemMgr = nullptr;
 }
 
 void
-GMPVideoHostImpl::ActorDestroyed()
-{
-  for (uint32_t i = mPlanes.Length(); i > 0; i--) {
-    mPlanes[i - 1]->ActorDestroyed();
-    mPlanes.RemoveElementAt(i - 1);
-  }
-  for (uint32_t i = mEncodedFrames.Length(); i > 0; i--) {
-    mEncodedFrames[i - 1]->ActorDestroyed();
-    mEncodedFrames.RemoveElementAt(i - 1);
-  }
-  mSharedMemMgr = nullptr;
-}
-
-void
 GMPVideoHostImpl::PlaneCreated(GMPPlaneImpl* aPlane)
 {
   mPlanes.AppendElement(aPlane);
 }
 
 void
 GMPVideoHostImpl::PlaneDestroyed(GMPPlaneImpl* aPlane)
 {