Intermittent dom/media/test/test_decode_error.html,dom/media/test/test_seek-10.html | application crashed [@ mozilla::FileBlockCache::Run()] (Assertion failure: mFD, at dom/media/FileBlockCache.cpp:292)

RESOLVED FIXED in Firefox 55

Status

()

Core
Audio/Video: Playback
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Treeherder Bug Filer, Assigned: gerald)

Tracking

({assertion, crash, intermittent-failure})

unspecified
mozilla55
assertion, crash, intermittent-failure
Points:
---

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox53 unaffected, firefox54 unaffected, firefox55 fixed)

Details

(crash signature)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Keywords: assertion
(Assignee)

Comment 1

a year ago
Most probably due to bug 1365534, which made opening the file (and hence setting mFD) asynchronous, and also resets mFD earlier. I'll investigate shortly...
Assignee: nobody → gsquelart
Blocks: 1365534
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8870664 [details]
Bug 1367155 - Handle async-closed FileBlockCached::mFD when reading/writing -

https://reviewboard.mozilla.org/r/142136/#review145800

r+ with comment addressed.

::: dom/media/FileBlockCache.cpp:118
(Diff revision 1)
> +
>  FileBlockCache::~FileBlockCache()
>  {
>    NS_ASSERTION(!mIsOpen, "Should Close() FileBlockCache before destroying");
> +  if (mFD) {
> +    // This could happen in the rare case where Close() was called shortly

Optional: Maybe you could instead just close the file descriptor in SetCacheFile in the !mIsOpen path? If you do that, you should assert the file descriptor is null here though.

::: dom/media/FileBlockCache.cpp:342
(Diff revision 1)
>      {
>        MutexAutoUnlock unlock(mDataMutex);
>        MutexAutoLock lock(mFileMutex);
> +      if (!mFD) {
> +        // We may be here if:
> +        // - mFD is not initialized yet. We can just exit, because this task

I think the "mFD is not initialized yet" case mentioned in this comment can't happen.

I don't see how we can get here before SetCacheFile() runs, as we don't dispatch a runnable to run this unless mInitialized is true, and that's set to true in SetCacheFile().

It seems more likely that SetCacheFile() ran, and then Close() ran before Run() ran.

So I think this comment is misleading.
Attachment #8870664 - Flags: review?(cpearce) → review+
Summary: Intermittent dom/media/test/test_decode_error.html | application crashed [@ mozilla::FileBlockCache::Run()] (Assertion failure: mFD, at dom/media/FileBlockCache.cpp:292) → Intermittent dom/media/test/test_decode_error.html,dom/media/test/test_seek-10.html | application crashed [@ mozilla::FileBlockCache::Run()] (Assertion failure: mFD, at dom/media/FileBlockCache.cpp:292)
Comment hidden (mozreview-request)

Comment 5

a year ago
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8839c7a642a7
Handle async-closed FileBlockCached::mFD when reading/writing - r=cpearce

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8839c7a642a7
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
status-firefox53: --- → unaffected
status-firefox54: --- → unaffected
status-firefox-esr52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.