fix(denoise-tester): dispose model nodes on playback stop; guard async lifecycle
Settings → Calls A/B denoise tester leaked audio resources: - play() built a denoise model node (DeepFilterNet/DTLN worker/WASM) + optional gate but stopPlayback only closed the AudioContext, never disposing them — each A/B playback-through-a-model leaked a worker. stopPlayback now mirrors stopLive (gate.disconnect → model.dispose → node.disconnect). - A generation token (playGenRef, bumped by stopPlayback) makes play() discard what it built if superseded during the async WASM/worklet load — closing the same leak in the rapid-Play-click race, the Stop-during-load case, and the unmount-during-load case, and stopping a superseded rejection from tearing down the winning playback. - A mountedRef guards the getUserMedia paths (startLive/startRecord) so closing Settings during the mic permission prompt doesn't create untracked resources / setState-after-unmount; its effect sets true on mount (not only false on cleanup) so it survives a StrictMode/Activity same-fiber remount. Bug-hunt findings from LOTUS_TODO. Three review passes: the first two confirmed the base fixes and surfaced the concurrent-load leak + StrictMode fragility; a third traced all six play() interleavings of the generation token. Gate-green (tsc, eslint, prettier, 914 tests, build). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -129,6 +129,11 @@ export function DenoiseTester({ model, useGate, gateThreshold, nativeNS }: Denoi
|
||||
try {
|
||||
const ctx = new AudioContext({ sampleRate: sampleRateFor(model) });
|
||||
const stream = await navigator.mediaDevices.getUserMedia(MIC_CONSTRAINTS(nativeNS));
|
||||
if (!mountedRef.current) {
|
||||
stream.getTracks().forEach((t) => t.stop());
|
||||
ctx.close().catch(() => undefined);
|
||||
return;
|
||||
}
|
||||
const source = ctx.createMediaStreamSource(stream);
|
||||
const inAnalyser = ctx.createAnalyser();
|
||||
inAnalyser.fftSize = 1024;
|
||||
@@ -182,7 +187,12 @@ export function DenoiseTester({ model, useGate, gateThreshold, nativeNS }: Denoi
|
||||
timer: number;
|
||||
} | null>(null);
|
||||
const clipRef = useRef<AudioBuffer | null>(null);
|
||||
const playRef = useRef<{ ctx: AudioContext; source: AudioBufferSourceNode } | null>(null);
|
||||
const playRef = useRef<{
|
||||
ctx: AudioContext;
|
||||
source: AudioBufferSourceNode;
|
||||
model: DenoiseNode | null;
|
||||
gate: AudioWorkletNode | null;
|
||||
} | null>(null);
|
||||
const [recording, setRecording] = useState(false);
|
||||
const [recDb, setRecDb] = useState(-100);
|
||||
const [hasClip, setHasClip] = useState(false);
|
||||
@@ -208,6 +218,10 @@ export function DenoiseTester({ model, useGate, gateThreshold, nativeNS }: Denoi
|
||||
const startRecord = async () => {
|
||||
try {
|
||||
const stream = await navigator.mediaDevices.getUserMedia(RAW_CONSTRAINTS);
|
||||
if (!mountedRef.current) {
|
||||
stream.getTracks().forEach((t) => t.stop());
|
||||
return;
|
||||
}
|
||||
const ctx = new AudioContext();
|
||||
const source = ctx.createMediaStreamSource(stream);
|
||||
const analyser = ctx.createAnalyser();
|
||||
@@ -249,7 +263,15 @@ export function DenoiseTester({ model, useGate, gateThreshold, nativeNS }: Denoi
|
||||
}
|
||||
};
|
||||
|
||||
// Bumped whenever a playback starts or stops. An in-flight `play()` compares
|
||||
// the generation it claimed against this after its awaits; if it no longer
|
||||
// matches (a newer play, a Stop, or unmount happened during model load) it
|
||||
// discards what it built instead of orphaning it — closes the rapid-click /
|
||||
// unmount-during-load leak.
|
||||
const playGenRef = useRef(0);
|
||||
|
||||
const stopPlayback = useCallback(() => {
|
||||
playGenRef.current += 1;
|
||||
const p = playRef.current;
|
||||
playRef.current = null;
|
||||
if (p) {
|
||||
@@ -259,6 +281,15 @@ export function DenoiseTester({ model, useGate, gateThreshold, nativeNS }: Denoi
|
||||
} catch {
|
||||
/* noop */
|
||||
}
|
||||
try {
|
||||
// Mirror stopLive: dispose the model node (worker/WASM) + gate, else each
|
||||
// A/B playback through a model leaks a DeepFilterNet/DTLN worker.
|
||||
p.gate?.disconnect();
|
||||
p.model?.dispose();
|
||||
p.model?.node.disconnect();
|
||||
} catch {
|
||||
/* noop */
|
||||
}
|
||||
p.ctx.close().catch(() => undefined);
|
||||
}
|
||||
setPlaying(null);
|
||||
@@ -268,37 +299,73 @@ export function DenoiseTester({ model, useGate, gateThreshold, nativeNS }: Denoi
|
||||
stopPlayback();
|
||||
const clip = clipRef.current;
|
||||
if (!clip) return;
|
||||
// Claim this generation AFTER stopPlayback's bump; a later play/stop/unmount
|
||||
// moves it past `gen`, signalling us to discard what we built below.
|
||||
const gen = playGenRef.current;
|
||||
try {
|
||||
// bufferSource auto-resamples the 48 kHz clip to the context rate, so DTLN
|
||||
// gets the 16 kHz it needs while raw/RNNoise/Speex stay at 48 kHz.
|
||||
const ctx = new AudioContext({ sampleRate: sampleRateFor(playModel ?? 'rnnoise') });
|
||||
const source = ctx.createBufferSource();
|
||||
source.buffer = clip;
|
||||
let playGate: AudioWorkletNode | null = null;
|
||||
let playModelNode: DenoiseNode | null = null;
|
||||
if (playModel) {
|
||||
let head: AudioNode = source;
|
||||
if (useGate) {
|
||||
const gate = await buildGateNode(ctx, gateThreshold);
|
||||
head.connect(gate);
|
||||
head = gate;
|
||||
playGate = await buildGateNode(ctx, gateThreshold);
|
||||
head.connect(playGate);
|
||||
head = playGate;
|
||||
}
|
||||
const denoise = await buildModelNode(ctx, playModel);
|
||||
head.connect(denoise.node);
|
||||
denoise.node.connect(ctx.destination);
|
||||
playModelNode = await buildModelNode(ctx, playModel);
|
||||
head.connect(playModelNode.node);
|
||||
playModelNode.node.connect(ctx.destination);
|
||||
} else {
|
||||
source.connect(ctx.destination);
|
||||
}
|
||||
// Superseded while the WASM/worklet loaded (another Play, a Stop, or the
|
||||
// panel unmounted)? Tear down this now-orphaned graph instead of storing
|
||||
// it — otherwise its worker/WASM + ctx would leak and its audio would play
|
||||
// over the winner.
|
||||
if (playGenRef.current !== gen || !mountedRef.current) {
|
||||
try {
|
||||
playGate?.disconnect();
|
||||
playModelNode?.dispose();
|
||||
playModelNode?.node.disconnect();
|
||||
} catch {
|
||||
/* noop */
|
||||
}
|
||||
ctx.close().catch(() => undefined);
|
||||
return;
|
||||
}
|
||||
source.onended = () => {
|
||||
if (playRef.current?.ctx === ctx) stopPlayback();
|
||||
};
|
||||
playRef.current = { ctx, source };
|
||||
playRef.current = { ctx, source, model: playModelNode, gate: playGate };
|
||||
source.start();
|
||||
setPlaying(label);
|
||||
} catch (e) {
|
||||
console.error('[denoise-tester] playback failed', e);
|
||||
stopPlayback();
|
||||
// Only tear down if we're still the current playback — a superseded
|
||||
// invocation must not stop the winner that replaced it.
|
||||
if (playGenRef.current === gen) stopPlayback();
|
||||
}
|
||||
};
|
||||
|
||||
// Guards the async getUserMedia paths: if Settings closes while the mic
|
||||
// permission prompt is open, the resolved stream/ctx would otherwise be
|
||||
// created after the unmount cleanup already ran, leaking + setState-after-
|
||||
// unmount. Own [] effect so it only flips on real unmount.
|
||||
const mountedRef = useRef(true);
|
||||
useEffect(() => {
|
||||
// Set on mount (not just cleared on unmount) so a setup→cleanup→setup
|
||||
// remount of the same fiber (StrictMode/Activity) leaves it true.
|
||||
mountedRef.current = true;
|
||||
return () => {
|
||||
mountedRef.current = false;
|
||||
};
|
||||
}, []);
|
||||
|
||||
useEffect(
|
||||
() => () => {
|
||||
stopLive();
|
||||
|
||||
Reference in New Issue
Block a user