904926 - Unnecessary and dangerous decoder lock in MediaDecoder::MetadataLoaded

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

I think I checked everything.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2454e6a5edbd

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/31c08ca022b3 for turning most Windows debug unittests orange with this fatal assertion:

22:18:29     INFO -  Crash reason:  EXCEPTION_BREAKPOINT
22:18:29     INFO -  Crash address: 0x7522381b
22:18:29     INFO -  Thread 0 (crashed)
22:18:29     INFO -   0  KERNELBASE.dll + 0x3381b
22:18:29     INFO -      eip = 0x7522381b   esp = 0x0028f0e0   ebp = 0x0028f0f0   ebx = 0x684c4064
22:18:29     INFO -      esi = 0x68171440   edi = 0x684c3f90   eax = 0x00000000   ecx = 0xf33d94b7
22:18:29     INFO -      edx = 0x6826e4d8   efl = 0x00000216
22:18:29     INFO -      Found by: given as instruction pointer in context
22:18:29     INFO -   1  nss3.dll!PR_AssertCurrentThreadOwnsLock [prulock.c:533ac6b8ee38 : 373 + 0x1b]
22:18:29     INFO -      eip = 0x683fbb34   esp = 0x0028f0f8   ebp = 0x0028f104
22:18:29     INFO -      Found by: previous frame's frame pointer
22:18:29     INFO -   2  nss3.dll!PR_AssertCurrentThreadInMonitor [prmon.c:533ac6b8ee38 : 121 + 0xd]
22:18:29     INFO -      eip = 0x683ee771   esp = 0x0028f10c   ebp = 0x0028f110
22:18:29     INFO -      Found by: call frame info
22:18:29     INFO -   3  xul.dll!mozilla::MediaDecoder::IsDataCachedToEndOfResource() [MediaDecoder.cpp:533ac6b8ee38 : 712 + 0x10]
22:18:29     INFO -      eip = 0x65141e7f   esp = 0x0028f118   ebp = 0x0028f140
22:18:29     INFO -      Found by: call frame info
22:18:29     INFO -   4  xul.dll!mozilla::MediaDecoder::MetadataLoaded(int,int,bool,bool,nsDataHashtable<nsCStringHashKey,nsCString> *) [MediaDecoder.cpp:533ac6b8ee38 : 757 + 0x10]
22:18:29     INFO -      eip = 0x65144459   esp = 0x0028f124   ebp = 0x0028f140
22:18:29     INFO -      Found by: call frame info
22:18:29     INFO -   5  xul.dll!mozilla::AudioMetadataEventRunner::Run() [AbstractMediaDecoder.h:533ac6b8ee38 : 148 + 0x24]
22:18:29     INFO -      eip = 0x65149f19   esp = 0x0028f148   ebp = 0x0028f160
22:18:29     INFO -      Found by: call frame info
22:18:29     INFO -   6  xul.dll!nsThread::ProcessNextEvent(bool,bool *) [nsThread.cpp:533ac6b8ee38 : 622 + 0xd]
22:18:29     INFO -      eip = 0x65e1a5dc   esp = 0x0028f168   ebp = 0x0028f1c4
22:18:29     INFO -      Found by: call frame info
22:18:29     INFO -   7  xul.dll!NS_ProcessNextEvent(nsIThread *,bool) [nsThreadUtils.cpp:533ac6b8ee38 : 238 + 0xc]
22:18:29     INFO -      eip = 0x65dd4257   esp = 0x0028f1cc   ebp = 0x0028f1d8
22:18:29     INFO -      Found by: call frame info
22:18:29     INFO -   8  xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) [MessagePump.cpp:533ac6b8ee38 : 81 + 0x9]
22:18:29     INFO -      eip = 0x65940c0c   esp = 0x0028f1e0   ebp = 0x0028f204
22:18:29     INFO -      Found by: call frame info

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v2

IsDataCachedToEndOfResource needs to acquire the monitor to avoid racing on mDecoderPosition (which is updated by the decoder and state machine threads).

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43566dc5dfbb

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/43566dc5dfbb