Summary
record/record_for in src/bundles/sound/src/functions.ts only gate on globalVars.isPlaying before scheduling a recording. Neither function marks "a recording is pending/in-flight" for the (potentially long) window between being called and actually invoking io().startRecording().
Both functions work by scheduling a chain of setTimeouts: wait for the buffer pause -> play a beep -> wait for the beep to finish -> actually call io().startRecording(). If a student calls record(1) (or record_for) twice in quick succession, both calls pass the isPlaying check (since neither has started recording yet), both schedule independent timeout chains, and both eventually call io().startRecording() - two overlapping recordings that stomp on each other's state (__recordedChunks, __mediaRecorder on the tab side in src/tabs/Sound/src/index.tsx).
Suggested direction
- Reserve recording state synchronously the moment
record/record_for is called (before the first setTimeout even fires), not just at the point io().startRecording() actually resolves.
- Release that state only after
stopRecording() settles, including on failure/rejection paths.
- Make the returned
stop() function (from record) and record_for's promise idempotent, so calling them more than once doesn't double-fire cleanup/signals.
Context
Flagged by CodeRabbit's review on #796 (the sound Conductor migration), correctly labeled a "heavy lift" rather than a quick patch - deferred to this follow-up rather than rushed into that PR. See the review thread: #796 (comment)
Summary
record/record_forinsrc/bundles/sound/src/functions.tsonly gate onglobalVars.isPlayingbefore scheduling a recording. Neither function marks "a recording is pending/in-flight" for the (potentially long) window between being called and actually invokingio().startRecording().Both functions work by scheduling a chain of
setTimeouts: wait for the buffer pause -> play a beep -> wait for the beep to finish -> actually callio().startRecording(). If a student callsrecord(1)(orrecord_for) twice in quick succession, both calls pass theisPlayingcheck (since neither has started recording yet), both schedule independent timeout chains, and both eventually callio().startRecording()- two overlapping recordings that stomp on each other's state (__recordedChunks,__mediaRecorderon the tab side insrc/tabs/Sound/src/index.tsx).Suggested direction
record/record_foris called (before the firstsetTimeouteven fires), not just at the pointio().startRecording()actually resolves.stopRecording()settles, including on failure/rejection paths.stop()function (fromrecord) andrecord_for's promise idempotent, so calling them more than once doesn't double-fire cleanup/signals.Context
Flagged by CodeRabbit's review on #796 (the sound Conductor migration), correctly labeled a "heavy lift" rather than a quick patch - deferred to this follow-up rather than rushed into that PR. See the review thread: #796 (comment)