1221855 - Crash [@ mozilla::dom::ScriptProcessorNodeEngine::SizeOfExcludingThis const ]

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[Jesse Ruderman]

Attachment: testcase (for use with about:memory)

1. Load the testcase.
2. Open about:memory in a new tab.
3. Click "Measure".

Result: Crash
[@ mozilla::dom::ScriptProcessorNodeEngine::SizeOfExcludingThis const ]

bp-c45d4f8a-94df-44bc-816e-e062e2151105

Comment 1

[Jesse Ruderman]

Attachment: stack for crash

Maybe a regression from bug 1199559:

https://hg.mozilla.org/mozilla-central/diff/c143ace1a0c7/dom/media/webaudio/ScriptProcessorNode.cpp#l1.99

Comment 2

[Jesse Ruderman]

Attachment: stack for “QI needed”

This non-fatal assertion shows up before the crash, but might be unrelated?

Comment 3

[Karl Tomlinson (:karlt)]

Thanks.  Yes, looks like a regression from
https://hg.mozilla.org/mozilla-central/rev/c143ace1a0c7

Comment 4

[Karl Tomlinson (:karlt)]

(In reply to Jesse Ruderman from comment #2)
> This non-fatal assertion shows up before the crash, but might be unrelated?

Yes, I don't think that is related to ScriptProcessor.

Comment 5

[Jesse Ruderman]

Okay, I split the "QI needed" assertion out into bug 1222202. If you fix this first, you can annotate the test with "asserts ... # Bug 1222202".

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1221855 null-check mInputBuffer in SizeOfExcludingThis() r?padenot

bug 1221855 null-check mInputBuffer in SizeOfExcludingThis() r?padenot

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: MozReview Request: bug 1221855 test Web Audio memory reporting r?erahm

bug 1221855 test Web Audio memory reporting r?erahm

https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/build/buildsystem/test_manifests.html
doesn't offer any asserts-if syntax and fails-if expects failure from explicit
tests, but doesn't expect assertion failures.

Comment 8

[Paul Adenot (:padenot)]

Comment on attachment 8684024 [details]
MozReview Request: bug 1221855 null-check mInputBuffer in SizeOfExcludingThis() r?padenot

https://reviewboard.mozilla.org/r/24477/#review22005

Comment 9

[Eric Rahm [:erahm]]

Comment on attachment 8684025 [details]
MozReview Request: bug 1221855 test Web Audio memory reporting r?erahm

https://reviewboard.mozilla.org/r/24479/#review22085

Looks good, thanks for taking the time to do this! We might want to make note in bug 1086262 that work on adding a web audio memory reporter test has started here.

::: dom/media/webaudio/test/mochitest.ini:183
(Diff revision 1)
> +skip-if = debug # assertion failures: bug 1222202

Can you add a note to bug 1222202 to reenable this once it's fixed?

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b346a16f9d06
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d2ceb469ea6

Comment 11

[Karl Tomlinson (:karlt)]

(In reply to Eric Rahm [:erahm] from comment #9)
> We might want to make
> note in bug 1086262 that work on adding a web audio memory reporter test has
> started here.

> Can you add a note to bug 1222202 to reenable this once it's fixed?

Done, thanks.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b346a16f9d06
https://hg.mozilla.org/mozilla-central/rev/4d2ceb469ea6

Comment 13

[Karl Tomlinson (:karlt)]

Comment on attachment 8684024 [details]
MozReview Request: bug 1221855 null-check mInputBuffer in SizeOfExcludingThis() r?padenot

Approval Request Comment
[Feature/regressing bug #]: bug 1199559
[User impact if declined]: potential null-deref crash when running about:memory while ScriptProcessorNode is in use.
[Describe test coverage new/current, TreeHerder]: new test.
[Risks and why]: very low.  simple null check.
[String/UUID change made/needed]: none.

Comment 14

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

Tracking since this is a recent regression.

Comment 15

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

Comment on attachment 8684024 [details]
MozReview Request: bug 1221855 null-check mInputBuffer in SizeOfExcludingThis() r?padenot

Crash fix, adds a test, OK to uplift to aurora and beta.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/00e7eb06006a
https://hg.mozilla.org/releases/mozilla-aurora/rev/f7c63d36e163

Comment 17

[Carsten Book [:Tomcat]]

The test in this commit does not apply to beta cleanly

grafting 314088:f7c63d36e163 "Bug 1221855 - test Web Audio memory reporting. r=erahm, a=lizzard"
merging dom/media/webaudio/test/mochitest.ini
warning: conflicts during merge.
merging dom/media/webaudio/test/mochitest.ini incomplete! (edit conflicts, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue

could you take a look, thanks!

Comment 18

[Karl Tomlinson (:karlt)]

Attachment: for 43: test Web Audio memory reporting

Rebased 4d2ceb469ea6 to 43.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/00e7eb06006a
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/f7c63d36e163

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9fbc92fa9e4b
https://hg.mozilla.org/releases/mozilla-beta/rev/decfff8497ce

Comment 21

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

Backed out on beta in https://hg.mozilla.org/releases/mozilla-beta/rev/e4629e309487 because the test was failing:
https://treeherder.mozilla.org/logviewer.html#?job_id=637866&repo=mozilla-beta

Comment 22

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

And the first patch backed out too: https://hg.mozilla.org/releases/mozilla-beta/rev/51f0f13e7985

Comment 23

[Karl Tomlinson (:karlt)]

The test fails on 43 because bug 1194555 was not fixed until 44.

It is still worthwhile having the null-check patch on 43.

Comment 24

[Karl Tomlinson (:karlt)]

Attachment: for 43: test Web Audio memory reporting v2

Adjusted for bug 1224022 and bug 1194555.

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/93d92b8c2b6c

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/294b55e22276