Closed Bug 981931 Opened 6 years ago Closed 6 years ago

Leak of realP and imagP in PeriodicWave::createBandLimitedTables

Categories

(Core :: Web Audio, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox29 --- fixed
firefox30 --- fixed

People

(Reporter: mccr8, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

(Whiteboard: [lsan][MemShrink])

Attachments

(1 file)

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
Oops.  I should have caught this.
Assignee: nobody → ehsan
Attachment #8388899 - Flags: review?(paul)
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?
Flags: needinfo?(khuey)
Attachment #8388899 - Flags: review?(paul) → review+
(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 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
Attachment #8388899 - Flags: approval-mozilla-aurora?
Doesn't seem like it's worth the effort for Tarako.
Flags: needinfo?(khuey)
https://hg.mozilla.org/mozilla-central/rev/f86398fa5fac
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Attachment #8388899 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.