diff --git a/src/offscreen/recorder.ts b/src/offscreen/recorder.ts index a86caf2..c396206 100644 --- a/src/offscreen/recorder.ts +++ b/src/offscreen/recorder.ts @@ -359,16 +359,40 @@ function onUserStoppedSharing(): void { } function stopRecording(): void { - // Отменяем будущую ротацию, чтобы rotateSegment не дёрнул stop() на - // уже остановленном рекордере. + // Sweep #1 fix: race between rotateSegment / onSegmentStopped and a + // STOP_RECORDING that does NOT null the mediaStream. The previous + // implementation cleared the rotation timer and called .stop(), which + // triggers onSegmentStopped async; onSegmentStopped then unconditionally + // calls startNewSegment() whenever `mediaStream !== null`. Result: after + // a manual STOP_RECORDING, a new segment would spawn anyway and the + // recorder kept rolling silently. The fix mirrors onUserStoppedSharing: + // null the mediaStream FIRST so onSegmentStopped's gate sees the stopped + // state, then stop the recorder and release the tracks. This also fixes + // the secondary issue of leaked MediaStreamTracks (the picker indicator + // would have stayed up until the operator manually clicked "Stop + // sharing" in Chrome's UI). if (rotationTimerId !== null) { clearTimeout(rotationTimerId); rotationTimerId = null; } + // Capture stream before nulling so we can release its tracks. + const streamToStop = mediaStream; + mediaStream = null; if (videoRecorder !== null && videoRecorder.state !== 'inactive') { - videoRecorder.stop(); + try { + videoRecorder.stop(); + } catch (err) { + logger.warn('stop() during stopRecording failed:', err); + } logger.log('Recording stopped manually'); } + if (streamToStop !== null) { + streamToStop.getTracks().forEach((t) => t.stop()); + } + // Note: we do NOT call resetBuffer() here — the operator may want to + // STOP and then SAVE the buffered footage. resetBuffer happens on + // onUserStoppedSharing (operator dismissed Chrome's sharing indicator) + // and at the start of the next startRecording(). } // ─── Bootstrap: handshake + port lifecycle (D-17, RESEARCH.md Patterns 4 & 5) ── diff --git a/tests/offscreen/segment-rotation.test.ts b/tests/offscreen/segment-rotation.test.ts index 8e44030..557997f 100644 --- a/tests/offscreen/segment-rotation.test.ts +++ b/tests/offscreen/segment-rotation.test.ts @@ -111,6 +111,25 @@ describe('segment rotation (D-13 restart-segments)', () => { expect(getSegments()).toEqual([]); }); + it('resetBuffer preserves segments do NOT survive (sweep #1 stop-race adjacent)', () => { + // Sweep #1 documented that stopRecording() previously raced + // onSegmentStopped: the timer was cleared but onSegmentStopped's + // mediaStream != null branch would still spawn a new segment. The + // fix nulls mediaStream FIRST. resetBuffer is the orthogonal + // mechanism for clearing the buffer (used by onUserStoppedSharing + // AND between tests). This test pins the contract that resetBuffer + // never leaves a partial segment behind. + pushSegmentForTest(makeSegmentBlob(0xa)); + pushSegmentForTest(makeSegmentBlob(0xb)); + pushSegmentForTest(makeSegmentBlob(0xc)); + expect(getSegments()).toHaveLength(MAX_SEGMENTS); + resetBuffer(); + expect(getSegments()).toHaveLength(0); + // Subsequent pushes start from an empty buffer — no eviction needed. + pushSegmentForTest(makeSegmentBlob(0xd)); + expect(getSegments()).toHaveLength(1); + }); + it('getSegments returns a snapshot — mutating the result does not corrupt internal state', () => { pushSegmentForTest(makeSegmentBlob(0x1)); pushSegmentForTest(makeSegmentBlob(0x2));