981931 - Leak of realP and imagP in PeriodicWave::createBandLimitedTables

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[Andrew McCreight [:mccr8]]

According to LSAN, we leak 590KB of memory when running the web audio mochitests, from |realP| and |imagP| in PeriodicWave::createBandLimitedTables.  These local variables are allocated, but never freed.  Simply calling |delete| on them at the end of their block fixes the leak, but I don't know if that's the approach you want to take.

Blink trunk seems to do something different (no explicit new call), but I'm not sure how it frees the memory:
  https://chromium.googlesource.com/chromium/blink/+/master/Source/modules/webaudio/PeriodicWave.cpp

Comment 1

[(no longer active)]

Oops.  I should have caught this.

Comment 2

[(no longer active)]

Attachment: Stop leaking the allocated buffers in PeriodicWave::createBandLimitedTables; r=padenot

Comment 3

[Andrew McCreight [:mccr8]]

It looks like createOscillator (which seems to be the audio node that uses PeriodicWave) is called in two places in Gaia:
  http://mxr.mozilla.org/gaia/search?string=createOscillator
Kyle, do you think this would be worth backporting for Terako?

Comment 4

[Paul Adenot (:padenot)]

(In reply to Andrew McCreight [:mccr8] from comment #3)
> It looks like createOscillator (which seems to be the audio node that uses
> PeriodicWave) is called in two places in Gaia:
>   http://mxr.mozilla.org/gaia/search?string=createOscillator
> Kyle, do you think this would be worth backporting for Terako?

Andrew, fwiw, those particular uses of OscillatorNode don't use PeriodicWave.

Comment 5

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f86398fa5fac

Comment 6

[(no longer active)]

Comment on attachment 8388899 [details] [diff] [review]
Stop leaking the allocated buffers in PeriodicWave::createBandLimitedTables; r=padenot

I think we should consider this for Aurora.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 865256
User impact if declined: memory leak on pages which use this feature.
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Doesn't seem like it's worth the effort for Tarako.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f86398fa5fac

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5afb2b0805a5