1225003 - Crash [@ mozilla::WebAudioDecodeJob::SizeOfExcludingThis]

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[Jesse Ruderman]

Attachment: 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?)

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Karl Tomlinson (:karlt)]

Thanks.  Similar to bug 1221855.

I should fix this too:
https://hg.mozilla.org/mozilla-central/annotate/a8ed7dd831d1/dom/media/webaudio/AudioDestinationNode.cpp#l181

Comment 3

[Karl Tomlinson (:karlt)]

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

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: null-check mBuffer in SizeOfExcludingThis()

Comment 5

[Eric Rahm [:erahm]]

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?

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3c4b0d791c5d
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c1b0d416870

Comment 7

[Karl Tomlinson (:karlt)]

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

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3c4b0d791c5d
https://hg.mozilla.org/mozilla-central/rev/4c1b0d416870

Comment 9

[Karl Tomlinson (:karlt)]

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

Comment 10

[Karl Tomlinson (:karlt)]

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.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

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

Please uplift to aurora and beta, crash fix, includes a test.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

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

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0988f97bcef0
https://hg.mozilla.org/releases/mozilla-aurora/rev/a551f6125d8c

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/73f660f039c4
https://hg.mozilla.org/releases/mozilla-beta/rev/996c7627ef12

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/0988f97bcef0
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/a551f6125d8c