1111508 - [EME] GMPDecryptorParent::Close() crash for mPlugin is null

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[JW Wang [:jwwang] [:jw_wang]]

Found this crash while testing CDM crash handling.

stack trace:
[805] ###!!! ASSERTION: You can't dereference a NULL nsRefPtr with operator->().: 'mRawPtr != 0', file /media/jwwang/DATA/codebase/mozilla-central2/xpcom/base/nsRefPtr.h, line 228
#01: mozilla::gmp::GMPDecryptorParent::Close() (/media/jwwang/DATA/codebase/mozilla-central2/dom/media/gmp/GMPDecryptorParent.cpp:315)
#02: mozilla::CDMProxy::gmp_Shutdown() (/media/jwwang/DATA/codebase/mozilla-central2/dom/media/eme/CDMProxy.cpp:345)
#03: nsRunnableMethodImpl<void (mozilla::CDMProxy::*)(), void, true>::Run() (/media/jwwang/DATA/codebase/mozilla-central2/obj-x86_64-unknown-linux-gnu/dom/media/eme/../../../dist/include/nsThreadUtils.h:391 (discriminator 1))
#04: nsThread::ProcessNextEvent(bool, bool*) (/media/jwwang/DATA/codebase/mozilla-central2/xpcom/threads/nsThread.cpp:830 (discriminator 1))
#05: NS_ProcessNextEvent(nsIThread*, bool) (/media/jwwang/DATA/codebase/mozilla-central2/xpcom/glue/nsThreadUtils.cpp:265)
#06: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (/media/jwwang/DATA/codebase/mozilla-central2/ipc/glue/MessagePump.cpp:340)
#07: MessageLoop::RunInternal() (/media/jwwang/DATA/codebase/mozilla-central2/ipc/chromium/src/base/message_loop.cc:234)
#08: ~AutoRunState (/media/jwwang/DATA/codebase/mozilla-central2/ipc/chromium/src/base/message_loop.cc:508)
#09: nsThread::ThreadFunc(void*) (/media/jwwang/DATA/codebase/mozilla-central2/xpcom/threads/nsThread.cpp:359)
#10: _pt_root (/media/jwwang/DATA/codebase/mozilla-central2/nsprpub/pr/src/pthreads/ptthread.c:215)
#11: start_thread (/build/buildd/eglibc-2.19/nptl/pthread_create.c:312 (discriminator 2))
#12: __clone (/build/buildd/eglibc-2.19/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:113)
#13: ??? (???:???)

This is because GMPDecryptorParent::ActorDestroy reset mPlugin which will be accessed later in GMPDecryptorParent::Close.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1111508_cache_gmp_thread.patch

1. cache GMP thread so we can access it after mPlugin is reset by GMPDecryptorParent.
2. fix a small bug in GMPDecryptorParent::Close

Try: https://tbpl.mozilla.org/?tree=Try&rev=134995bc3dc0

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8536936 [details] [diff] [review]
1111508_cache_gmp_thread.patch

Review of attachment 8536936 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/gmp/GMPDecryptorParent.cpp
@@ -319,5 @@
>    // Let Shutdown mark us as dead so it knows if we had been alive
>  
>    // In case this is the last reference
>    nsRefPtr<GMPDecryptorParent> kungfudeathgrip(this);
> -  NS_RELEASE(kungfudeathgrip);

NS_RELEASE(kungfudeathgrip) will remove self reference and kungfudeathgrip's reference immediately. This is wrong.

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fbe8cde84b77

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fbe8cde84b77

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Mass update firefox-status to track EME uplift.