Closed Bug 1366208 Opened 3 years ago Closed 3 years ago

Crash in mozilla::MP4Demuxer::NotifyDataArrived. Part 2

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 + fixed
firefox55 --- fixed

People

(Reporter: jya, Assigned: jya)

References

Details

(Keywords: crash, regression)

Attachments

(1 file)

This is all on 32 bits.. I think it's due to an earlier OOM in MP4Demuxer::Init()
Assignee: nobody → jyavenard
Priority: -- → P1
Keywords: crash
Comment on attachment 8869374 [details]
Bug 1366208: Make sure no empty RefPtr are ever stored.

https://reviewboard.mozilla.org/r/141026/#review144554
Attachment #8869374 - Flags: review?(gsquelart) → review+
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a2ed738274c1
Make sure no empty RefPtr are ever stored. r=gerald
https://hg.mozilla.org/mozilla-central/rev/a2ed738274c1
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: 1343437
Keywords: regression
Comment on attachment 8869374 [details]
Bug 1366208: Make sure no empty RefPtr are ever stored.

Approval Request Comment
[Feature/Bug causing the regression]: 1343437
[User impact if declined]: crashes
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: won't know for sure until a few days, as it's an intermittent failure
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: we ensure that if an error occurred while attempting to read a mp4, we don't later use null pointers.
[String changes made/needed]: none
Attachment #8869374 - Flags: approval-mozilla-beta?
Track 54+ as the crashes still show in 54.0b8.
Hi :jya, 
I'm not sure if bug 1343437 in the uplift request template is correct because bug 1343437 is fixed in 55. Why will the fix of bug 1343437 cause the issue?
Flags: needinfo?(jyavenard)
Actually, the bug was first introduced with bug 1341454, but made worse with bug 1343437 as there's more cases that could put a null pointer in the array.
Flags: needinfo?(jyavenard)
Comment on attachment 8869374 [details]
Bug 1366208: Make sure no empty RefPtr are ever stored.

Fix a crash. Let's see how it goes. Beta54+. Should be in 54 beta 10.
Attachment #8869374 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Jean-Yves Avenard [:jya] from comment #6)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]: won't know for sure until a few
> days, as it's an intermittent failure
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on jya's assessment on manual testing needs.
Flags: qe-verify-
No crash since May 15th... I guess we got it \o/
Congrats!
You need to log in before you can comment on or make changes to this bug.