From 3de5584fce4a262293803888223c2035f4bbe0c6 Mon Sep 17 00:00:00 2001 From: Michael Froman Date: Wed, 22 May 2019 22:20:27 +0000 Subject: [PATCH] Bug 1553555 - Release RemoteDecoderChild on manager thread in the case of an InitIPDL error. r=jya Differential Revision: https://phabricator.services.mozilla.com/D32178 --HG-- extra : moz-landing-system : lando --- dom/media/ipc/RemoteDecoderModule.cpp | 32 +++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/dom/media/ipc/RemoteDecoderModule.cpp b/dom/media/ipc/RemoteDecoderModule.cpp index edadaf4cfc77..cdc35c3f700a 100644 --- a/dom/media/ipc/RemoteDecoderModule.cpp +++ b/dom/media/ipc/RemoteDecoderModule.cpp @@ -94,9 +94,21 @@ already_AddRefed RemoteDecoderModule::CreateAudioDecoder( RefPtr child = new RemoteAudioDecoderChild(); MediaResult result(NS_OK); - RefPtr task = NS_NewRunnableFunction( - "RemoteDecoderModule::CreateAudioDecoder", [&, child]() { + // We can use child as a ref here because this is a sync dispatch. In + // the error case for InitIPDL, we can't just let the RefPtr go out of + // scope at the end of the method because it will release the + // RemoteAudioDecoderChild on the wrong thread. This will assert in + // RemoteDecoderChild's destructor. Passing the RefPtr by reference + // allows us to release the RemoteAudioDecoderChild on the manager + // thread during this single dispatch. + RefPtr task = + NS_NewRunnableFunction("RemoteDecoderModule::CreateAudioDecoder", [&]() { result = child->InitIPDL(aParams.AudioConfig(), aParams.mOptions); + if (NS_FAILED(result)) { + // Release RemoteAudioDecoderChild here, while we're on + // manager thread. Don't just let the RefPtr go out of scope. + child = nullptr; + } }); SyncRunnable::DispatchToThread(mManagerThread, task); @@ -124,10 +136,22 @@ already_AddRefed RemoteDecoderModule::CreateVideoDecoder( RefPtr child = new RemoteVideoDecoderChild(); MediaResult result(NS_OK); - RefPtr task = NS_NewRunnableFunction( - "RemoteDecoderModule::CreateVideoDecoder", [&, child]() { + // We can use child as a ref here because this is a sync dispatch. In + // the error case for InitIPDL, we can't just let the RefPtr go out of + // scope at the end of the method because it will release the + // RemoteVideoDecoderChild on the wrong thread. This will assert in + // RemoteDecoderChild's destructor. Passing the RefPtr by reference + // allows us to release the RemoteVideoDecoderChild on the manager + // thread during this single dispatch. + RefPtr task = + NS_NewRunnableFunction("RemoteDecoderModule::CreateVideoDecoder", [&]() { result = child->InitIPDL(aParams.VideoConfig(), aParams.mRate.mValue, aParams.mOptions); + if (NS_FAILED(result)) { + // Release RemoteVideoDecoderChild here, while we're on + // manager thread. Don't just let the RefPtr go out of scope. + child = nullptr; + } }); SyncRunnable::DispatchToThread(mManagerThread, task);