JSStreamer: playerMap is a Map but accessed via bracket indexing, so stopStreaming() never closes peer connections

[mcottontensor]

Summary

In Extras/JSStreamer/src/streamer.ts, playerMap is declared and constructed as a Map, but every insert, lookup and delete uses bracket indexing rather than the Map API. Bracket access sets plain JS properties on the Map object instead of creating Map entries, so the map itself is always empty.

playerMap: Map<string, PlayerPeer>;          // L63
this.playerMap = new Map<string, PlayerPeer>();  // L70

Property-style access (never touches Map storage):

Line	Code
291	this.playerMap[playerId] = newPlayer;
298	const playerPeer = this.playerMap[playerId];
302	delete this.playerMap[playerId];
308, 309	if (playerId && this.playerMap[playerId]) { const playerPeer = this.playerMap[playerId];
317, 318	(same pattern)
334	const playerPeer = this.playerMap[playerId];
368	const playerPeer = this.playerMap[playerId];

Genuine Map API access — the only one:

Line	Code
139	for (const peer of this.playerMap.values()) { peer.peerConnection.close(); }

Impact

The bracket-access sites are mutually consistent, so per-player connect, disconnect, SDP and ICE handling all work as intended. The breakage is confined to line 139 in stopStreaming():

stopStreaming() {
    this.transport.disconnect(1000, 'Normal shutdown by calling stopStreaming');
    for (const peer of this.playerMap.values()) {
        peer.peerConnection.close();
    }
}

Because nothing is ever stored via .set(), .values() yields nothing. stopStreaming() closes zero peer connections, leaking every RTCPeerConnection and leaving each player's 1-second statsTimer interval running. The loop looks correct and fails silently.

Fix

Either switch the access sites to the Map API:

this.playerMap.set(playerId, newPlayer);
const playerPeer = this.playerMap.get(playerId);
this.playerMap.delete(playerId);

or redeclare playerMap as a plain record and iterate with Object.values(). The Map API is preferable given the declared type and the existing .values() call site.

stopStreaming() should probably also clear each peer's statsTimer while tearing down, since that interval is only cleared on the player-disconnected path today.

Why this went unnoticed

eslint flags this loudly — 39 @typescript-eslint/no-unsafe-* errors in streamer.ts, all stemming from these lookups evaluating to an error type:

298:15  error  Unsafe assignment of an error typed value        @typescript-eslint/no-unsafe-assignment
299:38  error  Unsafe member access .statsTimer on a type that cannot be resolved
311:24  error  Unsafe member access .peerConnection on a type that cannot be resolved
...

But .github/workflows/healthcheck-libraries.yml only lints Common, Signalling, SignallingWebServer, Frontend/library and Frontend/ui-library. Extras/JSStreamer has a working npm run lint script that CI never invokes, so the failure has never surfaced. Worth considering adding it to the healthcheck workflow as part of the fix.

Found incidentally while working on #943.

[github-actions]

Issues go stale after 30 days of inactivity. Please comment or re-open the issue if you are still interested in getting this issue fixed.