[Blocking Requested - why for this release]: We've seen a crash with the following signature more than 50 times in latest build: [@ mozilla::dom::PeriodicWave::SizeOfExcludingThisIfNotShared(unsigned int (*)(void const*)) const | mozilla::dom::PeriodicWave::SizeOfIncludingThisIfNotShared(unsigned int (*)(void const*)) const | mozilla::dom::OscillatorNode::SizeOfExcludingThis(unsigned int (*)(void const*)) const | mozilla::BaseMediaResource::SizeOfIncludingThis(unsigned int (*)(void const*)) const ]

Just realized this was on an engineering build. Will withdraw this for now, but I think we will see this on official builds, so it may get re-opened.

[Blocking Requested - why for this release]:

These were seen with gaia: a00d102abfe8ae15c4fd14771efa2335c6d3b8d9 gecko:19e14f4003cafc106784a4984018ab28e3d9c1ff

Probably a dupe of bug 1068981. We're going to uplift and wait for CAF to confirm.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #8) > Probably a dupe of bug 1068981. We're going to uplift and wait for CAF to > confirm. We asked our test team to confirm and we will update shortly.

BLocking preemptively as its a crash, once confirmed will DUP it.

Observed on: Device: msm8226 Gonk Version: AU_LINUX_GECKO_B2G_KK_2.0.01.04.00.114.103 Moz BuildID: 20140924000243 B2G Version: 2.1 Gecko Version: 34.0a2 Gaia: http://git.mozilla.org/?p=releases/gaia.git;a=commit;h=93a99bea0b40d81bd063f7d8b1964dc1ba35ba7b Gecko: http://git.mozilla.org/?p=releases/gecko.git;a=commit;h=c896bc5d04b8c08dcddd78195901503a3578a08f Patches: bug 1055299, bug 1074703, bug 1065511, bug 1063066, bug 1068978, bug 1072021, bug 1073088, bug 1073419, bug 1067205, bug 1076327, bug 1070431, bug 1068394, bug 1043712, bug 1059573, bug 1068981, bug 1051636, bug 1051633, bug 1068571, bug 1063568, bug 1074419

My best guess is that it's in ThreadSharedFloatArrayBufferList::SizeOfExcludingThis(), in particular looking a mContents, which is an AutoFallibleTArray<Storage,2> -- and we're crashing in nsAutoPtr somewhere with a null ptr. ThreadSharedFloatArrayBufferList::SizeOfExcludingThis() doesn't null-check mContents[i], and the ThreadSharedFloatArrayBufferList(aCount) constructor explicitly constructs them with null data (mContents.SetLength()). This might not be the problem, but it's certainly fishy/risky. It's likely not bug 1068981 (I *think* the report from Greg indicates the uplift of that bug didn't fix it; I can't find the gecko rev specified above in comment 3; I suspect it's a git rev not an hg rev). Is there a regression range?

Paul -- Can you take this?

(In reply to Randell Jesup [:jesup] from comment #14) > It's likely not bug 1068981 (I *think* the report from Greg indicates the > uplift of that bug didn't fix it; Right. Comment #13 basically says that the crash happened on a build that had the patch from bug 1068981 applied.

(In reply to Greg Grisco from comment #16) > (In reply to Randell Jesup [:jesup] from comment #14) > > > It's likely not bug 1068981 (I *think* the report from Greg indicates the > > uplift of that bug didn't fix it; > > Right. Comment #13 basically says that the crash happened on a build that > had the patch from bug 1068981 applied. I think that Comment #13 is wrong here. We are still waiting to see this crash is reproduced in any build which has Gonk Version >= AU_LINUX_GECKO_B2G_KK_2.0.01.04.00.114.104 Sorry for confusions.

If this is not fixed, here is a possible explanation: PeriodicWaves are not a widely used feature of Web Audio API. They are not used in Gaia, as far as I know, and we should not reach this code at all in the first place (or maybe reporters are going on websites that use PeriodicWave? I don't know). I found this [0], where a nullcheck on mPeriodicWave is missing. mozilla-central null checks this pointer, as it should be. What version are we reproducing on, here? 2.0 has the bug per [0], but I'm not sure it's what we are looking at. [0]: http://mxr.mozilla.org/mozilla-b2g32_v2_0/source/content/media/webaudio/OscillatorNode.cpp#556

Can you confirm if this is still a bug or not on versions with the bug 1068981 patch? Thanks.

(In reply to Randell Jesup [:jesup] from comment #19) > Can you confirm if this is still a bug or not on versions with the bug > 1068981 patch? Thanks. Sorry we are still waiting input from our test team. We will confirm you asap. Sorry for unexpected delays.

I still want to assert in the method instead of making it a no-op because I'm going to work on AudioContext sleep and wakeup soon, and this will catch errors.

OscillatorNode don't always have mPeriodicWave set, so we need to null-check this.

Comment on attachment 8507977 [details] [diff] [review] Part 2 - Don't try to measure a PeriodicWave size when an OscillatorNode is using a basic waveform. r= Review of attachment 8507977 [details] [diff] [review]: ----------------------------------------------------------------- Good catch! r=me As a followup could you file a bug to actually implement unit tests for web audio memory reporting? We've been burned by this too many times.

If we're ready to land, please request b2g34 approval ASAP

Comment on attachment 8507974 [details] [diff] [review] Part 1 - Make sure we are not waking up an OfflineGraphDriver. r= NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 848954 User impact if declined: possible crash when requesting memory usage in about:memory and using an OfflineAudioContext Testing completed: locally (this was 100% reproducible before) Risk to taking this patch (and alternatives if risky): low, this is triggered by dmd/about:memory String or UUID changes made by this patch: none

Comment on attachment 8507977 [details] [diff] [review] Part 2 - Don't try to measure a PeriodicWave size when an OscillatorNode is using a basic waveform. r= NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 967817 User impact if declined: possible crash when requesting memory usage in about:memory and using an OscillatorNode using a basic waveform (a rather common scenario) Testing completed: locally (this was 100% reproducible before) Risk to taking this patch (and alternatives if risky): low, this is triggered by dmd/about:memory String or UUID changes made by this patch: none

https://hg.mozilla.org/integration/mozilla-inbound/rev/9601061181bd https://hg.mozilla.org/integration/mozilla-inbound/rev/ad6146f9dfb0

(In reply to Randell Jesup [:jesup] from comment #19) > Can you confirm if this is still a bug or not on versions with the bug > 1068981 patch? Thanks. We are not seeing this issue anymore in v2.1 after landing fix from bug 1068981. Sorry for delayed confirmation.

(In reply to Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me) from comment #28) > (In reply to Randell Jesup [:jesup] from comment #19) > > Can you confirm if this is still a bug or not on versions with the bug > > 1068981 patch? Thanks. > > We are not seeing this issue anymore in v2.1 after landing fix from bug > 1068981. Sorry for delayed confirmation. :jesup/Paul, If the patch in 1068981 fixes the issue, do we still need this uplift?

https://hg.mozilla.org/mozilla-central/rev/9601061181bd https://hg.mozilla.org/mozilla-central/rev/ad6146f9dfb0

Does this affect Aurora35/Beta34?

(In reply to bhavana bajaj [:bajaj] from comment #29) > (In reply to Tapas[:tkundu on #b2g/gaia/memshrink/gfx] (always NI me) from > comment #28) > > (In reply to Randell Jesup [:jesup] from comment #19) > > > Can you confirm if this is still a bug or not on versions with the bug > > > 1068981 patch? Thanks. > > > > We are not seeing this issue anymore in v2.1 after landing fix from bug > > 1068981. Sorry for delayed confirmation. > > :jesup/Paul, > > If the patch in 1068981 fixes the issue, do we still need this uplift? We do, this fixes two other crashes with somewhat similar STR.

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #31) > Does this affect Aurora35/Beta34? It does, yes. Those patches are not risky (they only touch code used by about:memory), we can uplift them if needed.

(In reply to Paul Adenot (:padenot) from comment #33) > It does, yes. Those patches are not risky (they only touch code used by > about:memory), we can uplift them if needed. Please request Aurora/Beta approval in that case :) https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/1e052a34c4a0 https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f05da96a08bc

Comment on attachment 8507974 [details] [diff] [review] Part 1 - Make sure we are not waking up an OfflineGraphDriver. r= See comment 25 and comment 26

Comment on attachment 8507977 [details] [diff] [review] Part 2 - Don't try to measure a PeriodicWave size when an OscillatorNode is using a basic waveform. r= See comment 25 and comment 26

Comment on attachment 8507974 [details] [diff] [review] Part 1 - Make sure we are not waking up an OfflineGraphDriver. r= As this is confined to about:memory, there doesn't seem to be any risk for a normal browsing scenario. Beta+ Aurora+

https://hg.mozilla.org/releases/mozilla-aurora/rev/8bb5349b9b38 https://hg.mozilla.org/releases/mozilla-aurora/rev/3d6147b9c59e https://hg.mozilla.org/releases/mozilla-beta/rev/9d0a16097623 https://hg.mozilla.org/releases/mozilla-beta/rev/b185e7a13e18

Unable to verify at this location as this issue involves an automated test.