1092932 - shutdown crash in mozilla::TrackBuffer::DiscardDecoder()

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[(Away)]

This bug was filed from the Socorro interface and is 
report bp-75d0e5cb-40a0-4937-9417-ce3922141026.
=============================================================

mParentDecoder is null at TrackBuffer::DiscardDecoder. #4 crash on nightly 36. This seems to have spiked on 20141025030202l, although there were a few crashes earlier.

:kinetik can you point me to an owner for this?

0015f530 602fb22a xul!mozilla::TrackBuffer::DiscardDecoder+0xa
0015f538 602faaba xul!mozilla::dom::SourceBuffer::Detach+0x2e
0015f550 602fb1ea xul!mozilla::dom::SourceBufferList::Clear+0x22
0015f568 602ff18b xul!mozilla::dom::MediaSource::Detach+0x7e
0015f570 60dee5f0 xul!mozilla::MediaSourceDecoder::Shutdown+0x32
0015f578 5fc04f95 xul!mozilla::ShutdownMediaDecoder+0xe
0015f588 60de20a9 xul!nsTHashtable<nsPtrHashKey<mozilla::DOMEventTargetHelper> >::s_EnumStub+0xe
0015f5d0 60dee544 xul!nsTHashtable<nsRefPtrHashKey<mozilla::MediaDecoder> >::EnumerateEntries+0x5d
0015f5e0 60de5eb3 xul!mozilla::MediaShutdownManager::Shutdown+0x28
0015f5e8 5fe4d456 xul!mozilla::MediaShutdownManager::Observe+0x21
0015f60c 5fe4cedb xul!nsObserverList::NotifyObservers+0x34
0015f628 5fec4d64 xul!nsObserverService::NotifyObservers+0xc0
0015f670 5fec4c63 xul!ScopedXPCOMStartup::~ScopedXPCOMStartup+0xf2
0015f67c 5fe2e192 xul!ScopedXPCOMStartup::`scalar deleting destructor'+0x9
0015f69c 5ff7a6c2 xul!XREMain::XRE_main+0x119
0015f7f4 0015f804 xul!XRE_main+0x39

Comment 1

[Matthew Gregan [:kinetik]]

Karl's knows this code well.

Comment 2

[Karl Tomlinson (:karlt)]

Same as reported in bug 1091008 comment 5, I assume.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: detach MediaSource before initiating MDStateMachine shutdown

to avoid races between the detach process and decoder task queue shutdown.

mCurrentDecoder is really a main thread field (bug 1092915), so I'd prefer not
to mess with it on the decode queue.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5620758c2419
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e7b65d978ebc

Comment 4

[Bobby Holley (:bholley)]

Comment on attachment 8516491 [details] [diff] [review]
detach MediaSource before initiating MDStateMachine shutdown

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

Hm, I guess so. It sure would be nice if we had clearer assertions about which shutdown stuff was running on which thread. Can you at least add some comments to this function indicating why the ordering is important?

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to Bobby Holley (:bholley) from comment #4)
> It sure would be nice if we had clearer assertions about
> which shutdown stuff was running on which thread. Can you at least add some
> comments to this function indicating why the ordering is important?

Added a comment.

Yes, it would be nice to make the threading model clearer, but it is hard to assert against race conditions.  Access protection classes like attachment 830568 [details] [diff] [review] could enforce threading models.  When bug 1092915 is fixed, we can add a main thread assertion in DiscardDecoder.

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

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 7

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Socorro [1] shows no more instances of this crash over the past 4 weeks for Firefox 36 or newer.

[1] - https://crash-stats.mozilla.com/report/list?product=Firefox&range_unit=days&range_value=28&signature=mozilla%3A%3ATrackBuffer%3A%3ADiscardDecoder%28%29