From 0471bbbc2eac912b15da291ac235ffdce71e36b8 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sat, 15 Aug 2026 08:41:51 -0700 Subject: [PATCH] refactor([issue-4283]): share mic capture re-entrancy guard --- .changelog/next/fixed-issue-4283.md | 1 + client/src/hooks/README.md | 1 + client/src/hooks/index.js | 1 + client/src/hooks/useAsyncCaptureGuard.js | 49 +++++++++++++++++++ client/src/hooks/useAsyncCaptureGuard.test.js | 41 ++++++++++++++++ client/src/hooks/useSingToScore.js | 44 +++++++++-------- client/src/hooks/useSingToVerify.js | 38 +++++++------- 7 files changed, 136 insertions(+), 39 deletions(-) create mode 100644 .changelog/next/fixed-issue-4283.md create mode 100644 client/src/hooks/useAsyncCaptureGuard.js create mode 100644 client/src/hooks/useAsyncCaptureGuard.test.js diff --git a/.changelog/next/fixed-issue-4283.md b/.changelog/next/fixed-issue-4283.md new file mode 100644 index 0000000000..80f7632509 --- /dev/null +++ b/.changelog/next/fixed-issue-4283.md @@ -0,0 +1 @@ +- **[issue-4283] Microphone capture now cleans up cancelled permission requests consistently across singing tools.** diff --git a/client/src/hooks/README.md b/client/src/hooks/README.md index 6c58bd523d..3bf7ac3205 100644 --- a/client/src/hooks/README.md +++ b/client/src/hooks/README.md @@ -126,6 +126,7 @@ grep -i "what you want to do" client/src/hooks/README.md | `useAsyncAction` | `running` state + toast-on-error. | Buttons that await an async action. | | `useAutoscroll` | rAF autoscroll for a scrollable container: advances `scrollTop` by `pxPerSec`, auto-stops at the bottom, pauses on user wheel/touchmove, live speed changes via ref. `fitToDuration(sec)` solves the speed for a target run time from the measured scroll travel (returns the applied px/s, or `null` when there is nothing to scroll). Returns `{ playing, toggle, stop, pxPerSec, setPxPerSec, fitToDuration }`. | SongBook play view; any teleprompter-style surface. | | `useAudioSessionClaim` | `useAudioSessionClaim(type)` → `{ claim, release }` — one arbitrated `acquireAudioSession(type)` slot with a React lifecycle: `claim()` drops this instance's previous claim before taking a fresh one, `release()` is idempotent, and unmount releases whatever is still held. | A surface owning its OWN `AudioContext` (MorseTrainer, the Security monitor) or `getUserMedia` stream (`useSingToScore`/`useSingToVerify`) needs the iOS session. An output-only player on the shared transport passes `audioSession: 'playback'` to `createLookaheadTransport` instead — don't hand-roll either. | +| `useAsyncCaptureGuard` | Shared pending-request and generation guard for microphone permission prompts; returns `{ tryStart, settleStart, isCurrent, cancel }` and delegates resource teardown plus caller-owned idle reset. | Any capture hook that must reject concurrent `getUserMedia` starts and dispose a stream resolved after cancellation. | | `useWakeLock` | Holds a screen wake lock (`navigator.wakeLock`) while `active` is true; releases on unmount/inactive, re-acquires on visibilitychange; no-op where unsupported. | Hands-free surfaces that must keep the screen on (SongBook autoscroll). | ## Storage & persistence diff --git a/client/src/hooks/index.js b/client/src/hooks/index.js index 564f93e0d1..a228eec6ef 100644 --- a/client/src/hooks/index.js +++ b/client/src/hooks/index.js @@ -10,6 +10,7 @@ // === Default-exporting hooks (re-exported as named) === export { default as useAnchorReveal } from './useAnchorReveal.js'; export { default as useAudioSessionClaim } from './useAudioSessionClaim.js'; +export { default as useAsyncCaptureGuard } from './useAsyncCaptureGuard.js'; export { default as useAutoscroll } from './useAutoscroll.js'; export { default as useCityAudio } from './useCityAudio.js'; export { default as useClonedGltf } from './useClonedGltf.jsx'; diff --git a/client/src/hooks/useAsyncCaptureGuard.js b/client/src/hooks/useAsyncCaptureGuard.js new file mode 100644 index 0000000000..c1f32366df --- /dev/null +++ b/client/src/hooks/useAsyncCaptureGuard.js @@ -0,0 +1,49 @@ +import { useCallback, useRef } from 'react'; + +const noop = () => {}; + +/** + * Coordinate a capture start that may remain pending during a permission + * prompt. The generation token lets callers discard continuations from a + * cancelled request without releasing a newer request's resources. + * + * @param {object} options + * @param {() => void} options.teardown — release the caller's live resources. + * @param {() => void} [options.onCancel] — reset caller-owned state after teardown. + * @returns {{ + * tryStart: () => number|null, + * settleStart: (generation: number) => boolean, + * isCurrent: (generation: number) => boolean, + * cancel: () => void, + * }} + */ +export default function useAsyncCaptureGuard({ teardown, onCancel = noop }) { + const pendingRef = useRef(false); + const generationRef = useRef(0); + + const isCurrent = useCallback( + (generation) => generation === generationRef.current, + [], + ); + + const tryStart = useCallback(() => { + if (pendingRef.current) return null; + pendingRef.current = true; + return ++generationRef.current; + }, []); + + const settleStart = useCallback((generation) => { + if (!isCurrent(generation)) return false; + pendingRef.current = false; + return true; + }, [isCurrent]); + + const cancel = useCallback(() => { + generationRef.current += 1; + pendingRef.current = false; + teardown(); + onCancel(); + }, [onCancel, teardown]); + + return { tryStart, settleStart, isCurrent, cancel }; +} diff --git a/client/src/hooks/useAsyncCaptureGuard.test.js b/client/src/hooks/useAsyncCaptureGuard.test.js new file mode 100644 index 0000000000..450f147218 --- /dev/null +++ b/client/src/hooks/useAsyncCaptureGuard.test.js @@ -0,0 +1,41 @@ +import { act, renderHook } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import useAsyncCaptureGuard from './useAsyncCaptureGuard.js'; + +describe('useAsyncCaptureGuard', () => { + it('allows one pending start and settles the current generation', () => { + const teardown = vi.fn(); + const onCancel = vi.fn(); + const { result } = renderHook(() => useAsyncCaptureGuard({ teardown, onCancel })); + + let firstGeneration; + let secondGeneration; + act(() => { + firstGeneration = result.current.tryStart(); + secondGeneration = result.current.tryStart(); + }); + + expect(firstGeneration).toBe(1); + expect(secondGeneration).toBeNull(); + expect(result.current.isCurrent(firstGeneration)).toBe(true); + + act(() => { result.current.settleStart(firstGeneration); }); + expect(result.current.tryStart()).toBe(2); + }); + + it('invalidates stale generations and delegates cancellation cleanup', () => { + const teardown = vi.fn(); + const onCancel = vi.fn(); + const { result } = renderHook(() => useAsyncCaptureGuard({ teardown, onCancel })); + + const firstGeneration = result.current.tryStart(); + act(() => { result.current.cancel(); }); + + expect(result.current.isCurrent(firstGeneration)).toBe(false); + expect(result.current.settleStart(firstGeneration)).toBe(false); + expect(teardown).toHaveBeenCalledOnce(); + expect(onCancel).toHaveBeenCalledOnce(); + + expect(result.current.tryStart()).toBe(3); + }); +}); diff --git a/client/src/hooks/useSingToScore.js b/client/src/hooks/useSingToScore.js index 0e11c17072..945ba4920c 100644 --- a/client/src/hooks/useSingToScore.js +++ b/client/src/hooks/useSingToScore.js @@ -18,6 +18,7 @@ import { createPitchTracker } from '../lib/pitchDetect.js'; import { createMetronome, clampBpm, timeSignatureFromScore, DEFAULT_BPM } from '../lib/metronome.js'; import { transcribePitchTrack } from '../lib/singToScore.js'; import useAudioSessionClaim from './useAudioSessionClaim.js'; +import useAsyncCaptureGuard from './useAsyncCaptureGuard.js'; import useMounted from './useMounted.js'; // Phases the UI renders distinct states for. @@ -61,8 +62,6 @@ export default function useSingToScore({ tempo, score = '', musicKey = 'C' } = { const trackRef = useRef([]); // accumulated { tMs, hz, clarity } frames const captureStartRef = useRef(0); // performance.now() at first music beat const capturingRef = useRef(false); // gate frames until the count-in completes - const startPendingRef = useRef(false); - const requestGenerationRef = useRef(0); // Held for exactly the window our own mic stream is open, symmetric with // `voiceClient` / `audioRecorder`. Sing-to-score is safe on `auto` only @@ -84,18 +83,22 @@ export default function useSingToScore({ tempo, score = '', musicKey = 'C' } = { capturingRef.current = false; }, [releaseSession]); - // Invalidate an in-flight permission request before tearing down. A browser - // can resolve getUserMedia after this hook unmounts; that continuation owns - // its stream until it sees this generation change and stops it itself. - const cancel = useCallback(() => { - requestGenerationRef.current += 1; - startPendingRef.current = false; - teardown(); + const resetAfterCancel = useCallback(() => { trackRef.current = []; if (!mountedRef.current) return; setPhase(SING_IDLE); setBeat(null); - }, [teardown, mountedRef]); + }, [mountedRef]); + + // Invalidate an in-flight permission request before tearing down. A browser + // can resolve getUserMedia after this hook unmounts; that continuation owns + // its stream until it sees this generation change and stops it itself. + const { + tryStart, + settleStart, + isCurrent, + cancel, + } = useAsyncCaptureGuard({ teardown, onCancel: resetAfterCancel }); // Finalize: stop everything, run the transcription over the captured track. const finish = useCallback(() => { @@ -119,9 +122,9 @@ export default function useSingToScore({ tempo, score = '', musicKey = 'C' } = { }, [phase, finish]); const start = useCallback(async () => { - if (phase !== SING_IDLE || startPendingRef.current) return; - startPendingRef.current = true; - const requestGeneration = ++requestGenerationRef.current; + if (phase !== SING_IDLE) return; + const requestGeneration = tryStart(); + if (requestGeneration === null) return; setError(null); setResult(null); trackRef.current = []; @@ -132,7 +135,7 @@ export default function useSingToScore({ tempo, score = '', musicKey = 'C' } = { // record-capable with no release left to call. Mirrors useSingToVerify. const getUserMedia = navigator.mediaDevices?.getUserMedia?.bind(navigator.mediaDevices); if (!getUserMedia) { - startPendingRef.current = false; + settleStart(requestGeneration); if (mountedRef.current) setError('Microphone access requires a secure browser connection'); return; } @@ -143,20 +146,19 @@ export default function useSingToScore({ tempo, score = '', musicKey = 'C' } = { // below is belt-and-suspenders: the hook already releases on unmount). claimSession(); const src = await getUserMedia({ audio: true }).catch((err) => { - if (requestGeneration === requestGenerationRef.current) { - startPendingRef.current = false; + if (settleStart(requestGeneration)) { releaseSession(); if (mountedRef.current) setError(err?.message || 'Microphone access denied'); } return null; }); if (!src) return; - if (!mountedRef.current || requestGeneration !== requestGenerationRef.current) { + if (!mountedRef.current || !isCurrent(requestGeneration)) { src.getTracks().forEach((track) => track.stop()); - if (requestGeneration === requestGenerationRef.current) releaseSession(); + if (isCurrent(requestGeneration)) releaseSession(); return; } - startPendingRef.current = false; + settleStart(requestGeneration); streamRef.current = src; const graph = createStreamAnalyser(src); @@ -202,12 +204,12 @@ export default function useSingToScore({ tempo, score = '', musicKey = 'C' } = { await metro.start().catch((err) => { // A cancellation or fresh request may have already torn this capture // down. Do not let its late failure tear down a newer session. - if (requestGeneration !== requestGenerationRef.current) return; + if (!isCurrent(requestGeneration)) return; if (mountedRef.current) setError(err?.message || 'Could not start audio'); teardown(); if (mountedRef.current) setPhase(SING_IDLE); }); - }, [phase, bpm, timeSig.beats, timeSig.beatValue, teardown, mountedRef, claimSession, releaseSession]); + }, [phase, bpm, timeSig.beats, timeSig.beatValue, teardown, mountedRef, claimSession, releaseSession, tryStart, settleStart, isCurrent]); // Clear a produced result (after the user inserts or discards it). const reset = useCallback(() => { diff --git a/client/src/hooks/useSingToVerify.js b/client/src/hooks/useSingToVerify.js index 56fb678383..a923a94004 100644 --- a/client/src/hooks/useSingToVerify.js +++ b/client/src/hooks/useSingToVerify.js @@ -8,6 +8,7 @@ import { createPitchTracker } from '../lib/pitchDetect.js'; import { parseScore } from '../lib/scoreNotation.js'; import { alignSingToVerify } from '../lib/singToVerify.js'; import useAudioSessionClaim from './useAudioSessionClaim.js'; +import useAsyncCaptureGuard from './useAsyncCaptureGuard.js'; import useMounted from './useMounted.js'; export const VERIFY_IDLE = 'idle'; @@ -33,8 +34,6 @@ export default function useSingToVerify({ score: scoreText = '', tempo } = {}) { const captureStartRef = useRef(0); const startBarRef = useRef(1); const capturingRef = useRef(false); - const startPendingRef = useRef(false); - const requestGenerationRef = useRef(0); const bpm = clampBpm(tempo ?? score.tempo) ?? DEFAULT_BPM; @@ -55,15 +54,19 @@ export default function useSingToVerify({ score: scoreText = '', tempo } = {}) { capturingRef.current = false; }, [releaseSession]); - const cancel = useCallback(() => { - requestGenerationRef.current += 1; - startPendingRef.current = false; - teardown(); + const resetAfterCancel = useCallback(() => { trackRef.current = []; if (!mountedRef.current) return; setPhase(VERIFY_IDLE); setBeat(null); - }, [teardown, mountedRef]); + }, [mountedRef]); + + const { + tryStart, + settleStart, + isCurrent, + cancel, + } = useAsyncCaptureGuard({ teardown, onCancel: resetAfterCancel }); const stop = useCallback(() => { if (phase === VERIFY_IDLE) return; @@ -83,9 +86,9 @@ export default function useSingToVerify({ score: scoreText = '', tempo } = {}) { }, [phase, score, bpm, teardown, mountedRef]); const start = useCallback(async (startBar = 1) => { - if (phase !== VERIFY_IDLE || startPendingRef.current) return; - startPendingRef.current = true; - const requestGeneration = ++requestGenerationRef.current; + if (phase !== VERIFY_IDLE) return; + const requestGeneration = tryStart(); + if (requestGeneration === null) return; setError(null); setRows([]); trackRef.current = []; @@ -93,7 +96,7 @@ export default function useSingToVerify({ score: scoreText = '', tempo } = {}) { const getUserMedia = navigator.mediaDevices?.getUserMedia?.bind(navigator.mediaDevices); if (!getUserMedia) { - startPendingRef.current = false; + settleStart(requestGeneration); if (mountedRef.current) setError('Microphone access requires a secure browser connection'); return; } @@ -106,20 +109,19 @@ export default function useSingToVerify({ score: scoreText = '', tempo } = {}) { // superseded request would drop THAT claim instead of ours. claimSession(); const stream = await getUserMedia({ audio: true }).catch((err) => { - if (requestGeneration === requestGenerationRef.current) { - startPendingRef.current = false; + if (settleStart(requestGeneration)) { releaseSession(); if (mountedRef.current) setError(err?.message || 'Microphone access denied'); } return null; }); if (!stream) return; - if (!mountedRef.current || requestGeneration !== requestGenerationRef.current) { + if (!mountedRef.current || !isCurrent(requestGeneration)) { stream.getTracks().forEach((track) => track.stop()); - if (requestGeneration === requestGenerationRef.current) releaseSession(); + if (isCurrent(requestGeneration)) releaseSession(); return; } - startPendingRef.current = false; + settleStart(requestGeneration); streamRef.current = stream; const graph = createStreamAnalyser(stream); @@ -160,12 +162,12 @@ export default function useSingToVerify({ score: scoreText = '', tempo } = {}) { // reason: a cancel()/restart during this await already tore THIS request // down, so an ungated teardown() here would stop the newer request's mic // stream and release the session claim it now holds. - if (requestGeneration !== requestGenerationRef.current) return; + if (!isCurrent(requestGeneration)) return; if (mountedRef.current) setError(err?.message || 'Could not start audio'); teardown(); if (mountedRef.current) setPhase(VERIFY_IDLE); }); - }, [phase, bpm, score.time.beats, score.time.beatValue, teardown, mountedRef, claimSession, releaseSession]); + }, [phase, bpm, score.time.beats, score.time.beatValue, teardown, mountedRef, claimSession, releaseSession, tryStart, settleStart, isCurrent]); const reset = useCallback(() => { setRows([]);