From 48d570573bb97af316997d445fff136932ead95d Mon Sep 17 00:00:00 2001 From: Chris Pearce Date: Mon, 11 Jul 2016 21:52:31 +1200 Subject: [PATCH] Bug 1282577 - Guard against multiple intr messages causes us to multi-delete GMP actors. r=jesup. This is a regression from bug 1162358. We must be hitting the #ifndef SHMEM_ALLOC_IN_CHILD block in GMPVideoDecoderChild::Alloc() with multiple allocs doing intr calls at once. If this happens when a DecodingComplete() comes in, we'll end up sending one task to re-call RecvDecodingComplete for every Alloc() blocked on an intr response. This would result in us ending up trying to Send__delete__() in RecvDecodingComplete() twice. Which causes the runtime abort we're seeing here. I think that could happen in the WidevineVideoDecoder if a Decode message comes in, and goes into a ReturnOutput(), tries to alloc a frame and has to spin on an intr message response, and another Decode message comes in and does the same, so GMPVideoDecoderChild::mNeedShmemIntrCount will be 2, and then a DecodingComplete comes in, and when two tasks on the stack in GMPVideoDecoderChild::Alloc() finish they both end up dispatching a task to re-call GMPVideoDecoderChild::RecvDecodingComplete(). So we end up trying to Send__delete__() in RecvDecodingComplete() twice. I expect the same problem exists in the GMPVideoEncoder too. intr, or spinning event loops in general for that matter, is evil. MozReview-Commit-ID: AKsvP62G3Cx --HG-- extra : rebase_source : 53ca12dbbbf3ab071788e2322b7c926ec7c0325f --- dom/media/gmp/GMPVideoDecoderChild.cpp | 3 ++- dom/media/gmp/GMPVideoEncoderChild.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dom/media/gmp/GMPVideoDecoderChild.cpp b/dom/media/gmp/GMPVideoDecoderChild.cpp index 259690628776..fcccbdf10a1b 100644 --- a/dom/media/gmp/GMPVideoDecoderChild.cpp +++ b/dom/media/gmp/GMPVideoDecoderChild.cpp @@ -225,7 +225,8 @@ GMPVideoDecoderChild::Alloc(size_t aSize, ++mNeedShmemIntrCount; rv = CallNeedShmem(aSize, aMem); --mNeedShmemIntrCount; - if (mPendingDecodeComplete) { + if (mPendingDecodeComplete && mNeedShmemIntrCount == 0) { + mPendingDecodeComplete = false; mPlugin->GMPMessageLoop()->PostTask( NewRunnableMethod(this, &GMPVideoDecoderChild::RecvDecodingComplete)); } diff --git a/dom/media/gmp/GMPVideoEncoderChild.cpp b/dom/media/gmp/GMPVideoEncoderChild.cpp index fa139cef4900..1b9196a1e3c2 100644 --- a/dom/media/gmp/GMPVideoEncoderChild.cpp +++ b/dom/media/gmp/GMPVideoEncoderChild.cpp @@ -206,7 +206,8 @@ GMPVideoEncoderChild::Alloc(size_t aSize, ++mNeedShmemIntrCount; rv = CallNeedShmem(aSize, aMem); --mNeedShmemIntrCount; - if (mPendingEncodeComplete) { + if (mPendingEncodeComplete && mNeedShmemIntrCount == 0) { + mPendingEncodeComplete = false; mPlugin->GMPMessageLoop()->PostTask( NewRunnableMethod(this, &GMPVideoEncoderChild::RecvEncodingComplete)); }