[EME] Assertion failure in MediaKeyStatusMap::UpdateInternal on slow CDM shutdown

RESOLVED FIXED in Firefox 37

Status

()

Core
Audio/Video
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: eflores, Assigned: eflores)

Tracking

unspecified
mozilla38
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox37 fixed, firefox38 fixed)

Details

Attachments

(2 attachments)

Trying to reproduce bug 1113474, ran into the following assertion running test_eme_persistent_sessions.html.

Happens after inserting a sleep(2) at [1].

Looks like the CDM is racing the JS code to shutdown?


[1] http://hg.mozilla.org/mozilla-central/file/2297934585ab/media/gmp-clearkey/0.1/ClearKeySessionManager.cpp#l288


#0  0x00007f29e391f7bf in js::MapObject::clear (cx=0x7f29e8673690, obj=...) at /home/me/src/evil-central/js/src/builtin/MapObject.cpp:1557
1557	    MOZ_ASSERT(MapObject::is(obj));

(gdb) bt
#0  0x00007f29e391f7bf in js::MapObject::clear (cx=0x7f29e8673690, obj=...) at /home/me/src/evil-central/js/src/builtin/MapObject.cpp:1557
#1  0x00007f29e3921d63 in JS::MapClear (cx=0x7f29e8673690, obj=...) at /home/me/src/evil-central/js/src/builtin/MapObject.cpp:2142
#2  0x00007f29e1828a63 in mozilla::dom::MediaKeyStatusMap::UpdateInternal (this=0x7f29b16fcd80, keys=...) at /home/me/src/evil-central/dom/media/eme/MediaKeyStatusMap.cpp:219
#3  0x00007f29e1828d95 in mozilla::dom::MediaKeyStatusMap::Update (this=0x7f29b16fcd80, keys=...) at /home/me/src/evil-central/dom/media/eme/MediaKeyStatusMap.cpp:243
#4  0x00007f29e1826c35 in mozilla::dom::MediaKeySession::UpdateKeyStatusMap (this=0x7f29b1460f20) at /home/me/src/evil-central/dom/media/eme/MediaKeySession.cpp:127
#5  0x00007f29e1827869 in mozilla::dom::MediaKeySession::DispatchKeysChange (this=0x7f29b1460f20) at /home/me/src/evil-central/dom/media/eme/MediaKeySession.cpp:305
#6  0x00007f29e18244c2 in mozilla::CDMProxy::OnKeysChange (this=0x7f29b582f2e0, aSessionId=...) at /home/me/src/evil-central/dom/media/eme/CDMProxy.cpp:447
#7  0x00007f29e1834c64 in nsRunnableMethodImpl<void (mozilla::CDMProxy::*)(nsAString_internal const&), nsString, true>::Run (this=0x7f29b14b0d80) at ../../../dist/include/nsThreadUtils.h:361
#8  0x00007f29df43a50f in nsThread::ProcessNextEvent (this=0x7f29e8638a90, aMayWait=false, aResult=0x7fff074d739f) at /home/me/src/evil-central/xpcom/threads/nsThread.cpp:855
#9  0x00007f29df47e3fa in NS_ProcessNextEvent (aThread=0x7f29e8638a90, aMayWait=false) at /home/me/src/evil-central/xpcom/glue/nsThreadUtils.cpp:265
#10 0x00007f29df93f1be in mozilla::ipc::MessagePump::Run (this=0x7f29d6235440, aDelegate=0x7f29e866dd20) at /home/me/src/evil-central/ipc/glue/MessagePump.cpp:99
#11 0x00007f29df8eaf85 in MessageLoop::RunInternal (this=0x7f29e866dd20) at /home/me/src/evil-central/ipc/chromium/src/base/message_loop.cc:233
#12 0x00007f29df8eaf1a in MessageLoop::RunHandler (this=0x7f29e866dd20) at /home/me/src/evil-central/ipc/chromium/src/base/message_loop.cc:226
#13 0x00007f29df8eaeab in MessageLoop::Run (this=0x7f29e866dd20) at /home/me/src/evil-central/ipc/chromium/src/base/message_loop.cc:200
#14 0x00007f29e1db2490 in nsBaseAppShell::Run (this=0x7f29cd06a400) at /home/me/src/evil-central/widget/nsBaseAppShell.cpp:164
#15 0x00007f29e2a5d858 in nsAppStartup::Run (this=0x7f29cd068060) at /home/me/src/evil-central/toolkit/components/startup/nsAppStartup.cpp:281
#16 0x00007f29e2ae9259 in XREMain::XRE_mainRun (this=0x7fff074d7760) at /home/me/src/evil-central/toolkit/xre/nsAppRunner.cpp:4160
#17 0x00007f29e2ae95b9 in XREMain::XRE_main (this=0x7fff074d7760, argc=5, argv=0x7fff074d8c68, aAppData=0x7fff074d7980) at /home/me/src/evil-central/toolkit/xre/nsAppRunner.cpp:4236
#18 0x00007f29e2ae9853 in XRE_main (argc=5, argv=0x7fff074d8c68, aAppData=0x7fff074d7980, aFlags=0) at /home/me/src/evil-central/toolkit/xre/nsAppRunner.cpp:4456
#19 0x0000000000404ad7 in do_main (argc=5, argv=0x7fff074d8c68, xreDirectory=0x7f29e86516c0) at /home/me/src/evil-central/browser/app/nsBrowserApp.cpp:294
#20 0x0000000000404f56 in main (argc=5, argv=0x7fff074d8c68) at /home/me/src/evil-central/browser/app/nsBrowserApp.cpp:667
Weird. Looks like the JS GC fails to traverse the MediaKeyStatusMap and just steamrolls mMap. Unsure why.
I see JSObject::group_ becomes null for the map object... it should be resulted from GC... but I wonder why MediaKeyStatusMap didn't keep mMap alive.
Another finding:

in mozilla::dom::MediaKeyStatusMap::UpdateInternal

(gdb) p * map.ptr->group_.value.clasp_
$14 = {
  name = 0x7fffc25f3e80 'K' <repeats 96 times>, "@?_\302\377\177",

The class name should be "Map".

Is it possible for MediaKeyStatusMap::mMap to point to another object due to moving GC?
Flags: needinfo?(bzbarsky)
As indicated by Edwin (https://developer.mozilla.org/en/docs/Interfacing_with_the_XPCOM_cycle_collector#Handling.C2.A0_JSObjects_fields) , it looks like we have to call |mozilla::HoldJSObjects(this)| in MediaKeyStatusMap constructor and |mozilla::DropJSObjects(this)| in the destructor.
Flags: needinfo?(bzbarsky)
Comment on attachment 8559579 [details] [diff] [review]
1129722.patch

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

Include bz to make sure this is all we need to keep a JSObject from GC.
Attachment #8559579 - Flags: review?(jwwang)
Attachment #8559579 - Flags: review?(bzbarsky)
Attachment #8559579 - Flags: feedback+
Comment on attachment 8559579 [details] [diff] [review]
1129722.patch

Yikes, yes.  I should have caught that....

>   printf("MediaKeyStatusMap dtor %p\n", this);

Please remove this line while you're here.
Attachment #8559579 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #7)
> >   printf("MediaKeyStatusMap dtor %p\n", this);
> 
> Please remove this line while you're here.

Whoops, that was from above this patch in my queue.
https://hg.mozilla.org/mozilla-central/rev/733843602df8
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Blocks: 1137045
Created attachment 8572353 [details] [diff] [review]
Beta patch

Patch for beta branch as part of EME platform uplift.
Comment on attachment 8572353 [details] [diff] [review]
Beta patch

Requesting retroactive approval for Beta landing as part of EME platform uplift.
Attachment #8572353 - Flags: approval-mozilla-beta?
Comment on attachment 8572353 [details] [diff] [review]
Beta patch

Approved for Beta as part of EME platform uplift.
Attachment #8572353 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.