If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Crash [@ mozilla::WebAudioDecodeJob::SizeOfExcludingThis]

RESOLVED FIXED in Firefox 43, Firefox OS v2.5

Status

()

Core
Web Audio
P1
critical
Rank:
10
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Jesse Ruderman, Assigned: karlt)

Tracking

(Blocks: 1 bug, {crash, regression, testcase})

Trunk
mozilla45
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox42 unaffected, firefox43 fixed, firefox44 fixed, firefox45 fixed, b2g-v2.5 fixed)

Details

(crash signature)

Attachments

(5 attachments)

(Reporter)

Description

2 years ago
Created attachment 8687773 [details]
testcase (requires extension)

The testcase crashes [@ mozilla::WebAudioDecodeJob::SizeOfExcludingThis].

(It might trip bug 1222202 first, but that's probably unrelated).

The testcase calls fuzzPriv.getMemoryReports, which does pretty much the same calls as clicking "Measure" in about:memory, but lets the testcase control the timing. Run with https://github.com/MozillaSecurity/funfuzz/tree/master/dom/extension installed in a temporary profile. (Or modify the testcase so the timing doesn't need to be so precise?)
(Reporter)

Comment 1

2 years ago
Created attachment 8687775 [details]
stack
(Assignee)

Comment 2

2 years ago
Thanks.  Similar to bug 1221855.

I should fix this too:
https://hg.mozilla.org/mozilla-central/annotate/a8ed7dd831d1/dom/media/webaudio/AudioDestinationNode.cpp#l181
Assignee: nobody → karlt
Blocks: 1199559
Status: NEW → ASSIGNED
status-firefox42: --- → unaffected
status-firefox43: --- → affected
status-firefox44: --- → affected
Keywords: regression
Priority: -- → P1
(Assignee)

Comment 3

2 years ago
Created attachment 8688193 [details] [diff] [review]
test no crashes in decodeAudioData() and offline context memory reporting

The testing of the offline context is limited due to bug 1225282, but does
catch the crash fixed here.
Attachment #8688193 - Flags: review?(erahm)
(Assignee)

Comment 4

2 years ago
Created attachment 8688194 [details] [diff] [review]
null-check mBuffer in SizeOfExcludingThis()
Attachment #8688194 - Flags: review?(padenot)
Comment on attachment 8688193 [details] [diff] [review]
test no crashes in decodeAudioData() and offline context memory reporting

Review of attachment 8688193 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/webaudio/test/test_WebAudioMemoryReporting.html
@@ +46,5 @@
>  SpecialPowers.Cc["@mozilla.org/memory-reporter-manager;1"].
>    getService(SpecialPowers.Ci.nsIMemoryReporterManager).
>    getReports(handleReport, null, finished, null, /* anonymized = */ false);
>  
> +ac.decodeAudioData(new ArrayBuffer(4), function(){}, function(){});

min: maybe add a note here about what we're trying to do. Is the goal to run |decodeAudioData| while reporting memory?
Attachment #8688193 - Flags: review?(erahm) → review+

Updated

2 years ago
Attachment #8688194 - Flags: review?(padenot) → review+
Rank: 10

Comment 6

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c4b0d791c5d
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c1b0d416870
(Assignee)

Comment 7

2 years ago
(In reply to Eric Rahm [:erahm] from comment #5)
> min: maybe add a note here about what we're trying to do. Is the goal to run
> |decodeAudioData| while reporting memory?

Yes.  Added a note.
Flags: in-testsuite+

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3c4b0d791c5d
https://hg.mozilla.org/mozilla-central/rev/4c1b0d416870
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox45: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
(Assignee)

Comment 9

2 years ago
Created attachment 8689325 [details] [diff] [review]
43 branch: test no crashes in decodeAudioData() and offline context memory reporting

Use this instead of attachment 8688193 [details] [diff] [review] on 43 branch, which does not have bug 1194555 fixed.
(Assignee)

Comment 10

2 years ago
Comment on attachment 8688194 [details] [diff] [review]
null-check mBuffer in SizeOfExcludingThis()

Approval Request Comment
[Feature/regressing bug #]: bug 1199559
[User impact if declined]: potential null-deref crash after running about:memory while OfflineAudioContext or decodeAudioData() is in use.
[Describe test coverage new/current, TreeHerder]: new test.
[Risks and why]: very low.  simple null check.
This is the same fix as was uplifted for bug 1221855 in different code.
[String/UUID change made/needed]: none.
Attachment #8688194 - Flags: approval-mozilla-beta?
Attachment #8688194 - Flags: approval-mozilla-aurora?
Comment on attachment 8688194 [details] [diff] [review]
null-check mBuffer in SizeOfExcludingThis()

Please uplift to aurora and beta, crash fix, includes a test.
Attachment #8688194 - Flags: approval-mozilla-beta?
Attachment #8688194 - Flags: approval-mozilla-beta+
Attachment #8688194 - Flags: approval-mozilla-aurora?
Attachment #8688194 - Flags: approval-mozilla-aurora+
Comment on attachment 8689325 [details] [diff] [review]
43 branch: test no crashes in decodeAudioData() and offline context memory reporting

[Triage Comment]

Test for crash fix. Please uplift this to beta (instead of attachment 8688193 [details] [diff] [review])
Attachment #8689325 - Flags: approval-mozilla-beta+
Comment on attachment 8688193 [details] [diff] [review]
test no crashes in decodeAudioData() and offline context memory reporting

[Triage Comment]

Test for crash fix, ok to uplift to aurora.
Attachment #8688193 - Flags: approval-mozilla-aurora+

Comment 14

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/0988f97bcef0
https://hg.mozilla.org/releases/mozilla-aurora/rev/a551f6125d8c
status-firefox44: affected → fixed

Comment 15

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/73f660f039c4
https://hg.mozilla.org/releases/mozilla-beta/rev/996c7627ef12
status-firefox43: affected → fixed

Comment 16

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/0988f97bcef0
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/a551f6125d8c
status-b2g-v2.5: --- → fixed
You need to log in before you can comment on or make changes to this bug.