Unnecessary and dangerous decoder lock in MediaDecoder::MetadataLoaded

RESOLVED FIXED in mozilla26

Status

()

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

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
mozilla26
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
Created attachment 789899 [details] [diff] [review]
fix

I think I checked everything.
Attachment #789899 - Flags: review?(cpearce)
Attachment #789899 - Flags: review?(cpearce) → review+
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
Created attachment 795276 [details] [diff] [review]
fix v2

IsDataCachedToEndOfResource needs to acquire the monitor to avoid racing on mDecoderPosition (which is updated by the decoder and state machine threads).
Attachment #789899 - Attachment is obsolete: true
Attachment #795276 - Flags: review?(cpearce)
Attachment #795276 - Flags: review?(cpearce) → review+
https://hg.mozilla.org/mozilla-central/rev/43566dc5dfbb
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.