Review live-video session teardown

Review exercise · TypeScript

Review brief

Nightmare60 min

A live-video gateway owns peer connections, local media tracks, recorders, and signaling subscriptions. Disconnect callbacks, moderator removal, and process shutdown may race, while recorder finalization must preserve the last media segment.

Behavioral contract

A join is registered as provisional before its first await. Shutdown prevents new joins, aborts or joins every provisional handshake, and no participant can register after shutdown completes.
Participant teardown is idempotent: atomically claim removal, detach the disconnect callback, stop and await recorder finalization, stop every local track, close the peer, and unsubscribe signaling.
Failure of one cleanup step is recorded but does not skip later steps; teardown settles after every owned resource has been attempted exactly once.
Service shutdown starts teardown for every session, waits for all outcomes rather than failing fast, and reports aggregate cleanup failures.
The 10-second deadline actively aborts pending handshakes and resource operations, then joins their settlement before shutdown resolves; a timeout race alone is insufficient.

Review instructions

1Trace join, disconnect, moderator removal, recorder finalization, and process shutdown as concurrent paths.
2Identify root lifecycle defects rather than reporting each leaked resource as a separate symptom.
3Prioritize no more than seven findings.

Learning objectives

• Make multi-resource teardown idempotent under concurrent triggers
• Preserve required cleanup ordering while tolerating individual failures
• Include handshakes and deadlines in service-wide shutdown coordination

src/video-session-types.tsTypeScript

@@ -0,0 +1,29 @@

1+export interface MediaTrack {

2+ stop(): void;

3+}

5+export interface PeerConnection {

6+ id: string;

7+ close(signal?: AbortSignal): Promise<void>;

8+ onDisconnected(callback: () => void): () => void;

9+}

10+

11+export interface Recorder {

12+ stop(signal?: AbortSignal): Promise<Blob>;

13+}

14+

15+export interface SignalingSubscription {

16+ unsubscribe(signal?: AbortSignal): Promise<void>;

17+}

18+

19+export interface ParticipantResources {

20+ peer: PeerConnection;

21+ recorder: Recorder;

22+ tracks: readonly MediaTrack[];

23+ signaling: SignalingSubscription;

24+}

25+

26+export interface VideoDependencies {

27+ acceptJoin(sessionId: string, signal?: AbortSignal): Promise<ParticipantResources>;

28+ reportCleanupError(error: unknown): void;

29+}

Reference solution

The findings below are ordered by priority.

Findings

1. An in-progress join can register after shutdown

Location: src/live-video-manager.ts:10-17,19-28

The stopping check occurs before acceptJoin, but the pending handshake is not registered or abortable. Shutdown snapshots existing sessions, resolves, and then the await can complete and add fresh resources to a newly created session.

Track a provisional join and controller before the first await, include it in shutdown, and recheck ownership before registration while ensuring rejected registration tears the resources down.

2. Concurrent removal runs teardown more than once

Location: src/live-video-session.ts:20-31

The participant stays in the map until all awaits finish. A disconnect callback, moderator action, and session close can each retrieve the same resources and concurrently stop or close them multiple times.

Atomically remove or replace the entry with one shared teardown promise before the first await, and have every trigger join that promise.

3. Closing the peer precedes recorder finalization

Location: src/live-video-session.ts:26-28

The peer is closed before recorder.stop finishes the final media segment. Closing its source first can truncate the recording and violates the explicit ordering guarantee.

Detach callbacks, await recorder finalization, stop local tracks, and only then close the peer.

4. One cleanup rejection skips every later resource

Location: src/live-video-session.ts:26-30

The sequential awaits have no failure aggregation. If peer close or recorder stop rejects, tracks remain active, signaling stays subscribed, and the participant remains registered.

Attempt each ordered stage with per-stage error capture and a final ownership release, then report an aggregate after all resources settle.

5. Disconnect listeners remain attached

Location: src/live-video-session.ts:14-18

onDisconnected returns the required unsubscribe function, but it is discarded. The callback retains the session and can fire during or after intentional teardown, adding races and preventing timely collection.

Store the unsubscribe handle in participant state and call it when teardown is claimed, before asynchronous cleanup.

6. Session shutdown fails fast instead of waiting for every participant

Location: src/live-video-session.ts:34-37, src/live-video-manager.ts:21-28

Both levels use Promise.all. A single participant or session rejection makes closing reject immediately, so manager shutdown can return while other cleanups are still running and does not report their later failures together.

Use all-settled aggregation at both levels and surface a combined result only after every teardown settles.

7. The deadline does not cancel or join timed-out work

Location: src/live-video-manager.ts:19-28

The timer merely wins Promise.race. It supplies no abort signal to handshakes or resource methods and shutdown resolves while they retain sockets, tracks, and subscriptions in the background.

At the deadline abort all outstanding controllers, then await their cleanup settlement before resolving and report which resources exceeded graceful time.

Reasonable non-findings

Setting the stopping flag before beginning the shutdown snapshot is correct.
Keeping sessions in a map is appropriate when their participant lifetimes are coordinated.
Individual media APIs may be asynchronous; the contract intentionally requires joining them rather than assuming close is immediate.