1367155 - 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)

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Treeherder Bug Filer]

Filed by: rvandermeulen [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=101261999&repo=autoland

https://queue.taskcluster.net/v1/task/Qir9VP9iQiOu2iBKWhCKAQ/runs/0/artifacts/public/logs/live_backing.log

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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...

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1367155 - Handle async-closed FileBlockCached::mFD when reading/writing -

Review commit: https://reviewboard.mozilla.org/r/142136/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/142136/

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/142136/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8839c7a642a7
Handle async-closed FileBlockCached::mFD when reading/writing - r=cpearce

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8839c7a642a7