fix(display): stop captures when the recording's output file cannot be created (#290)

recordAudioToFile and recordScreenAndAudioToFiles start their captures before
opening the output file. M4aFileWriter.create awaits the write stream's 'open',
so an audio path whose parent directory does not exist rejects once the device
is already streaming, and nothing unwinds it: the try/finally that stops the
captures does not begin until several statements later.

A file descriptor is the least of what leaks:

- the device keeps encoding and sending RTP, because stopAllMediaStreams is
  never called
- the UDP receivers stay bound and their RTCP keepalives keep firing, which is
  exactly what tells the device to keep the session alive past its 20s timeout
- UdpMediaReceiver queues datagrams into an unbounded array whenever no consumer
  is waiting, and after this failure there is no consumer, so inbound RTP
  accumulates in memory for the lifetime of the process

Neither capture escapes its function, so a caller cannot release them either.
The leaked receiver also keeps the event loop alive, so a process that hits this
never exits on its own.

Both functions now release what they have already acquired before rethrowing. A
failed stop is logged at debug rather than swallowed silently, since that case
means the device is still streaming, which is the leak itself; the redundant
second stop and the file close stay quiet. The original error is what
propagates in every case.

Covered by unit tests that drive the real capture path against a stub service
and assert the teardown ran. Reverting either guard fails them with 0 stops
instead of 1 and 2, and also hangs the runner on the leaked receiver.

Author: harsha509

src/services/ios/display/audio/audio-stream-capture.ts

@@ -242,7 +242,25 @@ export async function recordAudioToFile(
   const timer = setTimeout(() => controller.abort(), durationMs);
   timer.unref?.();
 
-  const writer = await M4aFileWriter.create(outputPath);
+  let writer: M4aFileWriter;
+  try {
+    writer = await M4aFileWriter.create(outputPath);
+  } catch (error) {
+    // The capture is already streaming and never escapes this function, so a
+    // caller cannot stop it. Left running, the device keeps sending into a
+    // receiver whose queue nobody drains.
+    clearTimeout(timer);
+    // Swallowed so it cannot mask the error being thrown, but logged: a failed
+    // stop means the device is still streaming, which is the leak itself.
+    await capture.stop().catch((stopError: unknown) => {
+      log.debug(
+        `Failed to stop the audio stream after the output file could not be created: ${
+          stopError instanceof Error ? stopError.message : String(stopError)
+        }`,
+      );
+    });
+    throw error;
+  }
   let written;
   try {
     for await (const unit of capture.accessUnits(controller.signal)) {

src/services/ios/display/recording/av-capture.ts

@@ -125,7 +125,29 @@ export async function recordScreenAndAudioToFiles(
   }
 
   const videoOut = new AnnexBFileWriter(videoPath);
-  const audioOut = await M4aFileWriter.create(audioPath);
+  let audioOut: M4aFileWriter;
+  try {
+    audioOut = await M4aFileWriter.create(audioPath);
+  } catch (error) {
+    // Both captures are already streaming by now, and neither they nor the
+    // video writer escape this function, so a caller cannot release them. Left
+    // running, the device keeps encoding into a receiver whose queue nobody
+    // drains, and its RTCP keepalive keeps the session alive indefinitely.
+    // Each failure is swallowed so it cannot mask the error being thrown. The
+    // stop is still logged: if it fails the device is left streaming, which is
+    // the leak itself. One stop tears down both streams, so the second call and
+    // the file close are quiet.
+    await videoCapture.stop().catch((stopError: unknown) => {
+      log.debug(
+        `Failed to stop cleanly after the audio file could not be created: ${
+          stopError instanceof Error ? stopError.message : String(stopError)
+        }`,
+      );
+    });
+    await audioCapture.stop().catch((): void => undefined);
+    await videoOut.close().catch((): void => undefined);
+    throw error;
+  }
   let framesWritten = 0;
   let videoBytes = 0;
   let sawKeyFrame = false;

test/unit/display/audio/audio-stream-capture.spec.ts

@@ -0,0 +1,51 @@
+import assert from 'node:assert/strict';
+import {mkdtemp, rm} from 'node:fs/promises';
+import {tmpdir} from 'node:os';
+import {join} from 'node:path';
+import {after, before, describe, it} from 'node:test';
+
+import {recordAudioToFile} from '../../../../src/services/ios/display/audio/audio-stream-capture.js';
+import type {DisplayService, MediaStreamAnswer} from '../../../../src/services/ios/display/index.js';
+
+/** A stub service that negotiates without a device and counts teardowns. */
+function makeStubService(): {service: DisplayService; stopCalls: () => number} {
+  let stopCalls = 0;
+  const service = {
+    getTunnelLocalAddress: async (): Promise<string> => '::1',
+    getDeviceAddress: async (): Promise<string> => '::1',
+    startAudioStream: async (): Promise<MediaStreamAnswer> =>
+      // An empty streamConfig means no RTCP identity, so no keepalive timer is
+      // started — the negotiation is all this test needs.
+      ({clientSessionId: undefined, streamConfig: {}, connection: {}, raw: {}}) as unknown as MediaStreamAnswer,
+    stopAllMediaStreams: async (): Promise<number[]> => {
+      stopCalls += 1;
+      return [];
+    },
+  } as unknown as DisplayService;
+  return {service, stopCalls: () => stopCalls};
+}
+
+describe('recordAudioToFile', function () {
+  let directory: string;
+
+  before(async function () {
+    directory = await mkdtemp(join(tmpdir(), 'record-audio-'));
+  });
+
+  after(async function () {
+    await rm(directory, {force: true, recursive: true});
+  });
+
+  it('stops the capture when the output file cannot be created', async function () {
+    // The capture is started before the writer, so a writer that fails to open
+    // used to strand a live device stream: nothing stopped it, its UDP receiver
+    // stayed bound, and its queue grew with every packet nobody read. The
+    // capture never escapes the function, so no caller could clean it up.
+    const {service, stopCalls} = makeStubService();
+    const unwritablePath = join(directory, 'no', 'such', 'dir', 'audio.m4a');
+
+    await assert.rejects(() => recordAudioToFile(service, unwritablePath), /ENOENT/);
+
+    assert.strictEqual(stopCalls(), 1, 'the capture must be stopped before the error propagates');
+  });
+});

test/unit/display/recording/av-capture.spec.ts

@@ -0,0 +1,63 @@
+import assert from 'node:assert/strict';
+import {mkdtemp, rm} from 'node:fs/promises';
+import {tmpdir} from 'node:os';
+import {join} from 'node:path';
+import {after, before, describe, it} from 'node:test';
+
+import type {DisplayService, MediaStreamAnswer} from '../../../../src/services/ios/display/index.js';
+import {recordScreenAndAudioToFiles} from '../../../../src/services/ios/display/recording/av-capture.js';
+
+/** A stub service that negotiates without a device and counts teardowns. */
+function makeStubService(): {service: DisplayService; stopCalls: () => number} {
+  let stopCalls = 0;
+  // An empty streamConfig means no RTCP identity, so no keepalive timer is
+  // started — the negotiation is all this test needs.
+  const answer = {
+    clientSessionId: undefined,
+    streamConfig: {},
+    connection: {},
+    raw: {},
+  } as unknown as MediaStreamAnswer;
+  const service = {
+    getTunnelLocalAddress: async (): Promise<string> => '::1',
+    getDeviceAddress: async (): Promise<string> => '::1',
+    startVideoStream: async (): Promise<MediaStreamAnswer> => answer,
+    startAudioStream: async (): Promise<MediaStreamAnswer> => answer,
+    stopAllMediaStreams: async (): Promise<number[]> => {
+      stopCalls += 1;
+      return [];
+    },
+  } as unknown as DisplayService;
+  return {service, stopCalls: () => stopCalls};
+}
+
+describe('recordScreenAndAudioToFiles', function () {
+  let directory: string;
+
+  before(async function () {
+    directory = await mkdtemp(join(tmpdir(), 'record-av-'));
+  });
+
+  after(async function () {
+    await rm(directory, {force: true, recursive: true});
+  });
+
+  it('stops both captures when the audio file cannot be created', async function () {
+    // Both captures are streaming before either writer is opened, so a failing
+    // audio writer used to strand two live device streams plus the video file
+    // descriptor. None of them escape the function, so no caller could release
+    // them.
+    const {service, stopCalls} = makeStubService();
+
+    await assert.rejects(
+      () =>
+        recordScreenAndAudioToFiles(service, {
+          videoPath: join(directory, 'screen.h265'),
+          audioPath: join(directory, 'no', 'such', 'dir', 'audio.m4a'),
+        }),
+      /ENOENT/,
+    );
+
+    assert.strictEqual(stopCalls(), 2, 'both captures must be stopped before the error propagates');
+  });
+});