From 93f2dd3f92cce1cf84ced6f496986fb308c336fd Mon Sep 17 00:00:00 2001 From: 11suixing11 <11suixing11@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:16:10 +0800 Subject: [PATCH 1/2] fix(sound): guard recording lifecycle --- .../sound/src/__tests__/recording.test.ts | 107 ++++++++++++++++++ src/bundles/sound/src/functions.ts | 72 ++++++++---- 2 files changed, 157 insertions(+), 22 deletions(-) diff --git a/src/bundles/sound/src/__tests__/recording.test.ts b/src/bundles/sound/src/__tests__/recording.test.ts index ce4ec3e1b7..bb75a56f0a 100644 --- a/src/bundles/sound/src/__tests__/recording.test.ts +++ b/src/bundles/sound/src/__tests__/recording.test.ts @@ -9,6 +9,7 @@ beforeEach(() => { funcs.setSoundIO(io); funcs.globalVars.micPermissionGranted = null; funcs.globalVars.activePlayCount = 0; + funcs.globalVars.recordingInProgress = false; }); describe(funcs.init_record, () => { @@ -70,6 +71,20 @@ describe('Recording functions', () => { expect(() => funcs.record(1)).toThrowError('record: Cannot record while another sound is playing!'); }); + test('reserves recording state synchronously before startRecording fires', async () => { + vi.useRealTimers(); + await funcs.init_record(); + vi.useFakeTimers(); + + funcs.record(1); + + expect(() => funcs.record(1)).toThrowError('record: Cannot record while another recording is in progress!'); + expect(() => funcs.record_for(1, 1)).toThrowError( + 'record_for: Cannot record while another recording is in progress!' + ); + expect(io.startRecording).not.toHaveBeenCalled(); + }); + test(`${funcs.record.name} works`, async () => { vi.useRealTimers(); await funcs.init_record(); @@ -94,6 +109,28 @@ describe('Recording functions', () => { expect(funcs.get_duration(sound)).toBeCloseTo(samples.length / sampleRate); // A mono mic (left === right, same Float32Array) produces a Sound with left=right automatically. expect(funcs.get_left_wave(sound)).toBe(funcs.get_right_wave(sound)); + expect(funcs.globalVars.recordingInProgress).toBe(false); + }); + + test('stop is idempotent and only stops the recording once', async () => { + vi.useRealTimers(); + await funcs.init_record(); + vi.useFakeTimers(); + + const samples = new Float32Array([0]); + io.stopRecording.mockResolvedValue({ left: samples, right: samples, sampleRate: 8000 }); + + const stop = funcs.record(0); + await vi.advanceTimersByTimeAsync(300); + + const soundPromise0 = stop(); + const soundPromise1 = stop(); + await vi.advanceTimersByTimeAsync(0); + + expect(io.stopRecording).toHaveBeenCalledOnce(); + expect(soundPromise0()).toBe(soundPromise1()); + await expect(soundPromise0()).resolves.toBeDefined(); + expect(funcs.globalVars.recordingInProgress).toBe(false); }); test('the sound promise resolves only once processing has actually finished, not immediately', async () => { @@ -122,6 +159,40 @@ describe('Recording functions', () => { expect(resolved).toBe(true); }); + test('releases recording state if stopRecording rejects', async () => { + vi.useRealTimers(); + await funcs.init_record(); + vi.useFakeTimers(); + + io.stopRecording.mockRejectedValueOnce(new Error('stop failed')); + + const stop = funcs.record(0); + await vi.advanceTimersByTimeAsync(300); + + const soundPromise = stop(); + const recording = soundPromise(); + void recording.catch(() => undefined); + await vi.advanceTimersByTimeAsync(0); + + await expect(recording).rejects.toThrow('stop failed'); + expect(funcs.globalVars.recordingInProgress).toBe(false); + }); + + test('releases recording state if startRecording rejects before stop is called', async () => { + vi.useRealTimers(); + await funcs.init_record(); + vi.useFakeTimers(); + + io.startRecording.mockRejectedValueOnce(new Error('start failed')); + + funcs.record(0); + await vi.advanceTimersByTimeAsync(300); + + expect(io.startRecording).toHaveBeenCalledOnce(); + expect(io.stopRecording).not.toHaveBeenCalled(); + expect(funcs.globalVars.recordingInProgress).toBe(false); + }); + test('a genuinely stereo input device produces a Sound with different left/right channels', async () => { vi.useRealTimers(); await funcs.init_record(); @@ -152,6 +223,20 @@ describe('Recording functions', () => { expect(() => funcs.record_for(1, 1)).toThrowError('record_for: Cannot record while another sound is playing!'); }); + test('reserves recording state synchronously before startRecording fires', async () => { + vi.useRealTimers(); + await funcs.init_record(); + vi.useFakeTimers(); + + funcs.record_for(1, 1); + + expect(() => funcs.record(1)).toThrowError('record: Cannot record while another recording is in progress!'); + expect(() => funcs.record_for(1, 1)).toThrowError( + 'record_for: Cannot record while another recording is in progress!' + ); + expect(io.startRecording).not.toHaveBeenCalled(); + }); + test(`${funcs.record_for.name} works`, async () => { vi.useRealTimers(); await funcs.init_record(); @@ -170,6 +255,28 @@ describe('Recording functions', () => { const sound = await promise(); expect(funcs.get_duration(sound)).toBeCloseTo(samples.length / sampleRate); + expect(funcs.globalVars.recordingInProgress).toBe(false); + }); + + test('the returned promise is idempotent and releases recording state if stopRecording rejects', async () => { + vi.useRealTimers(); + await funcs.init_record(); + vi.useFakeTimers(); + + io.stopRecording.mockRejectedValueOnce(new Error('stop failed')); + + const soundPromise = funcs.record_for(1, 0); + const recording0 = soundPromise(); + const recording1 = soundPromise(); + expect(recording0).toBe(recording1); + void recording0.catch(() => undefined); + + await vi.advanceTimersByTimeAsync(1300); + + expect(io.startRecording).toHaveBeenCalledOnce(); + expect(io.stopRecording).toHaveBeenCalledOnce(); + await expect(recording0).rejects.toThrow('stop failed'); + expect(funcs.globalVars.recordingInProgress).toBe(false); }); }); }); diff --git a/src/bundles/sound/src/functions.ts b/src/bundles/sound/src/functions.ts index 0b519d95fe..fd90261645 100644 --- a/src/bundles/sound/src/functions.ts +++ b/src/bundles/sound/src/functions.ts @@ -58,11 +58,17 @@ interface BundleGlobalVars { * the mic picking up whatever's playing through the speakers. */ activePlayCount: number; + /** + * True from the moment record()/record_for() accepts a request until that request either fails + * before starting or stopRecording() has settled. + */ + recordingInProgress: boolean; } export const globalVars: BundleGlobalVars = { micPermissionGranted: null, - activePlayCount: 0 + activePlayCount: 0, + recordingInProgress: false }; /** @@ -354,6 +360,21 @@ function assertMicPermission(func_name: string): void { } } +function reserveRecording(func_name: string): void { + if (globalVars.activePlayCount > 0) { + throw new EvaluatorRuntimeError(`${func_name}: Cannot record while another sound is playing!`); + } + if (globalVars.recordingInProgress) { + throw new EvaluatorRuntimeError(`${func_name}: Cannot record while another recording is in progress!`); + } + assertMicPermission(func_name); + globalVars.recordingInProgress = true; +} + +function releaseRecording(): void { + globalVars.recordingInProgress = false; +} + /** * Records a sound until the returned stop function is called. Takes a buffer duration (in * seconds) as argument, and returns a nullary stop function. Calling the stop function returns a @@ -375,10 +396,7 @@ function assertMicPermission(func_name: string): void { */ export function record(buffer: number): () => () => Promise { validateDuration('record', buffer); - if (globalVars.activePlayCount > 0) { - throw new EvaluatorRuntimeError(`${record.name}: Cannot record while another sound is playing!`); - } - assertMicPermission('record'); + reserveRecording(record.name); const started = (async () => { await delay(pre_recording_signal_pause_ms + buffer * 1000); @@ -386,13 +404,22 @@ export function record(buffer: number): () => () => Promise { await delay(recording_signal_ms); await io().startRecording(); })(); + let recordingDone: Promise | undefined; + void started.catch(() => { + if (!recordingDone) { + releaseRecording(); + } + }); return () => { - const recordingDone: Promise = started - .then(() => io().stopRecording()) - .then(({ left, right, sampleRate }) => samplesToSound(left, right, sampleRate)); - void play_recording_signal(); - return () => recordingDone; + if (!recordingDone) { + recordingDone = started + .then(() => io().stopRecording()) + .then(({ left, right, sampleRate }) => samplesToSound(left, right, sampleRate)) + .finally(releaseRecording); + void play_recording_signal(); + } + return () => recordingDone!; }; } @@ -420,23 +447,24 @@ export function record(buffer: number): () => () => Promise { export function record_for(duration: number, buffer: number): () => Promise { validateDuration('record_for', duration); validateDuration('record_for', buffer); - if (globalVars.activePlayCount > 0) { - throw new EvaluatorRuntimeError(`${record_for.name}: Cannot record while another sound is playing!`); - } - assertMicPermission('record_for'); + reserveRecording(record_for.name); // order of events for record_for: // pre-recording-signal pause | recording signal | // pre-recording pause | recording | recording signal const recordingDone: Promise = (async () => { - await delay(pre_recording_signal_pause_ms); - await play_recording_signal(); - await delay(recording_signal_ms + buffer * 1000); - await io().startRecording(); - await delay(duration * 1000); - const { left, right, sampleRate } = await io().stopRecording(); - void play_recording_signal(); - return samplesToSound(left, right, sampleRate); + try { + await delay(pre_recording_signal_pause_ms); + await play_recording_signal(); + await delay(recording_signal_ms + buffer * 1000); + await io().startRecording(); + await delay(duration * 1000); + const { left, right, sampleRate } = await io().stopRecording(); + void play_recording_signal(); + return samplesToSound(left, right, sampleRate); + } finally { + releaseRecording(); + } })(); return () => recordingDone; From 49d1d9855d168b13e8eb0e88a06423cd67abf0a8 Mon Sep 17 00:00:00 2001 From: 11suixing11 <11suixing11@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:47:37 +0800 Subject: [PATCH 2/2] fix(sound): guard stale recording release --- .../sound/src/__tests__/recording.test.ts | 29 +++++++++++++++++++ src/bundles/sound/src/functions.ts | 21 +++++++++----- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/bundles/sound/src/__tests__/recording.test.ts b/src/bundles/sound/src/__tests__/recording.test.ts index bb75a56f0a..0d13fd3078 100644 --- a/src/bundles/sound/src/__tests__/recording.test.ts +++ b/src/bundles/sound/src/__tests__/recording.test.ts @@ -193,6 +193,35 @@ describe('Recording functions', () => { expect(funcs.globalVars.recordingInProgress).toBe(false); }); + test('a stale stop after a failed start cannot release a newer recording', async () => { + vi.useRealTimers(); + await funcs.init_record(); + vi.useFakeTimers(); + + io.startRecording.mockRejectedValueOnce(new Error('start failed')); + + const staleStop = funcs.record(0); + await vi.advanceTimersByTimeAsync(300); + + expect(funcs.globalVars.recordingInProgress).toBe(false); + + const samples = new Float32Array([0]); + io.stopRecording.mockResolvedValue({ left: samples, right: samples, sampleRate: 8000 }); + const currentStop = funcs.record(0); + + const staleSoundPromise = staleStop(); + await expect(staleSoundPromise()).rejects.toThrow('start failed'); + + expect(funcs.globalVars.recordingInProgress).toBe(true); + + await vi.advanceTimersByTimeAsync(300); + const currentSoundPromise = currentStop(); + await vi.advanceTimersByTimeAsync(0); + + await expect(currentSoundPromise()).resolves.toBeDefined(); + expect(funcs.globalVars.recordingInProgress).toBe(false); + }); + test('a genuinely stereo input device produces a Sound with different left/right channels', async () => { vi.useRealTimers(); await funcs.init_record(); diff --git a/src/bundles/sound/src/functions.ts b/src/bundles/sound/src/functions.ts index fd90261645..385d9b7907 100644 --- a/src/bundles/sound/src/functions.ts +++ b/src/bundles/sound/src/functions.ts @@ -78,6 +78,7 @@ export const globalVars: BundleGlobalVars = { * on that count itself. */ let playGeneration = 0; +let recordingGeneration = 0; let soundIO: SoundTabRpc | undefined; @@ -360,7 +361,7 @@ function assertMicPermission(func_name: string): void { } } -function reserveRecording(func_name: string): void { +function reserveRecording(func_name: string): number { if (globalVars.activePlayCount > 0) { throw new EvaluatorRuntimeError(`${func_name}: Cannot record while another sound is playing!`); } @@ -369,10 +370,14 @@ function reserveRecording(func_name: string): void { } assertMicPermission(func_name); globalVars.recordingInProgress = true; + recordingGeneration += 1; + return recordingGeneration; } -function releaseRecording(): void { - globalVars.recordingInProgress = false; +function releaseRecording(generation: number): void { + if (recordingGeneration === generation) { + globalVars.recordingInProgress = false; + } } /** @@ -396,7 +401,7 @@ function releaseRecording(): void { */ export function record(buffer: number): () => () => Promise { validateDuration('record', buffer); - reserveRecording(record.name); + const generation = reserveRecording(record.name); const started = (async () => { await delay(pre_recording_signal_pause_ms + buffer * 1000); @@ -407,7 +412,7 @@ export function record(buffer: number): () => () => Promise { let recordingDone: Promise | undefined; void started.catch(() => { if (!recordingDone) { - releaseRecording(); + releaseRecording(generation); } }); @@ -416,7 +421,7 @@ export function record(buffer: number): () => () => Promise { recordingDone = started .then(() => io().stopRecording()) .then(({ left, right, sampleRate }) => samplesToSound(left, right, sampleRate)) - .finally(releaseRecording); + .finally(() => releaseRecording(generation)); void play_recording_signal(); } return () => recordingDone!; @@ -447,7 +452,7 @@ export function record(buffer: number): () => () => Promise { export function record_for(duration: number, buffer: number): () => Promise { validateDuration('record_for', duration); validateDuration('record_for', buffer); - reserveRecording(record_for.name); + const generation = reserveRecording(record_for.name); // order of events for record_for: // pre-recording-signal pause | recording signal | @@ -463,7 +468,7 @@ export function record_for(duration: number, buffer: number): () => Promise