Memory Leak in High-Frequency triggerAttackRelease() Loops
- https://tonejs.github.io/examples/events
You can see from the DevTools memory browser that this example has a leak. I encountered this in the app I am working on.
Reproduction:
- Single Tone.Synth with basic ADSR envelope
- Tone.Loop calling synth.triggerAttackRelease(440, '32n', time) at 8Hz
- Chrome heap snapshot after 60 seconds: 1,738+ AudioParam objects with methods {linearRampToValueAtTime, cancelScheduledValues, setValueCurveAtTime} retained in memory
Workaround I'm using (more or less)
// Setup (once)
const osc = new Tone.Oscillator(440, 'sine').start();
const gain = new Tone.Gain(0);
osc.connect(gain).toDestination();
// Loop (8Hz)
new Tone.Loop((time) => {
const attack = 0.005;
const duration = Tone.Time('32n').toSeconds();
const release = 0.005;
// Cancel automation at pulse start only
gain.gain.cancelScheduledValues(time);
// Manual envelope with smooth ramps
gain.gain.setValueAtTime(0, time);
gain.gain.linearRampToValueAtTime(1, time + attack);
gain.gain.setValueAtTime(1, time + attack + (duration - attack - release));
gain.gain.linearRampToValueAtTime(0, time + duration);
}, '16n');
Hi @kdulcet, thanks for reporting this.
It looks like the objects are created here: https://github.com/chrisguttandin/standardized-audio-context/blob/3efd17925abcb3f7a47676da5576e887cc9e70e1/src/factories/audio-param-factory.ts#L27-L160. Something seems to be holding a reference to these objects longer than necessary.
I'm not sure if I understand your workaround. What's the thing that makes the bug disappear?
I want to say up front my understanding is not deep of tone.js but I can describe troubleshooting this issue. I have been coding with AI and although I have a clear idea of the systems at play, there's still a lot of mystery box things that I would have to look into.
This was in particular notable that the sample code was doing the same thing but perhaps that was an oversight?
In troubleshooting the issue:
I've been using a 32n tone.loop and I those envelopes appear to not be disposable. It quickly accumulates into the 1000s
I tried canceling scheduled events, disposing of the tone.loops, a whole lot of things.
It appears to be residual envelopes and I'm using a continuous, fast arpeggiated synth with triggerAttackRelease and it's almost like the release isn't being triggered but then I looked at the example code and not seeing if there is a workaround.
In this case, only spawning the synth once and apply volume fades maintains most of the functionality I'm looking for immediately but later I would like to add a full blown arpeggiatior and at least within the context of using a tone.loop, there's a lot of buildup from these envelopes.
Using a single synth and then using those volumes like an LFO is alright for pulses with binaural beats but it wouldn't work for an arpeggiator very well unless there is some memory management I'm missing!
Thanks for looking into this!
After poking around a bit I noticed that there are many connections made to an AudioParam which never get disconnected again. It looks like those connections are all coming from here:
https://github.com/Tonejs/Tone.js/blob/ee43c940574c1110f11c2bf382383fdd7f73f6ed/Tone/source/oscillator/Oscillator.ts#L151-L152
_start()creates a new ToneOscillatorNode but doesn't clean up the previous one in case it exists.
Doing a manual cleanup makes the memory leak go away.
/* 67 */ const oscillator = new ToneOscillatorNode({
/* 68 */ context: this.context,
/* 69 */ onended: () => this.onstop(this),
/* 70 */ });
if (this._oscillator !== null) {
this.frequency.disconnect(this._oscillator.frequency);
this.detune.disconnect(this._oscillator.detune);
this._oscillator.dispose();
}
/* 71 */ this._oscillator = oscillator;
I'm not sure though if that should be handled with more care. OmniOscillator for example schedules a timeout for doing a similar thing: https://github.com/Tonejs/Tone.js/blob/ee43c940574c1110f11c2bf382383fdd7f73f6ed/Tone/source/oscillator/OmniOscillator.ts#L263-L268
@tambien Could you please take a look.
thanks for looking into this @chrisguttandin and @kdulcet i'll investigate a bit when i have a moment, seems like some kind of cleanup on stop would make sense