Skip to content

[Web] Listeners and timers not removed on unmount leak scenes/handlers (Replay3DLite, useDemoPlayback) #550

Description

@Flegma

3D replay adds window pointerup/pointermove listeners that are never removed, leaking the whole Three.js scene per mount

Location: components/match/Replay3DLite.vue:296 (web)

What: Inside the onMounted setup (lines 168-1751), the free-look handlers are registered with bare addEventListener (which targets window): addEventListener("pointerup", (e) => {...}) at line 296 and addEventListener("pointermove", (e) => {...}) at line 303. Both use anonymous inline closures. The cleanup function (lines 1741-1750, invoked from onBeforeUnmount at 1795) removes only keydown/keyup/blur (1746-1748) and document visibilitychange (1749); it never removes these two window listeners, and since they are anonymous they could not be removed even if attempted. Each closure captures el, camera, controls, followSid, and rl/rlx/rly from the setup scope, so the entire scene graph is retained by window after the component unmounts even though renderer.dispose()/controls.dispose() run.

Impact: Replay3DLite is mounted via v-if="viewMode === '3d' && radarSrc && calibration" in ReplayViewer.vue (line 5260). A user reviewing demos toggles between the 2D radar and 3D view, or navigates from one demo replay to the next; each 3D mount leaks two permanent window listeners whose closures pin a full WebGL scene (camera, controls, geometry references) so it can never be garbage-collected. After a handful of toggles the pointermove handler also runs its orbit-math for every prior instance on every mouse move, so both memory and per-move CPU climb without bound across the session.

Suggested fix: Hoist the pointerup/pointermove handlers into named functions (like onKeyDown/onKeyUp) added with window.addEventListener, and add matching window.removeEventListener calls for them inside the cleanup function alongside the existing keydown/keyup/blur removals.

Verifier evidence

components/match/Replay3DLite.vue:296 addEventListener("pointerup", (e) => { (anonymous, window-targeted); :303 addEventListener("pointermove", (e) => { (anonymous, window-targeted). Cleanup at :1741-1750 removes only: :1746 removeEventListener("keydown", onKeyDown); :1747 removeEventListener("keyup", onKeyUp); :1748 removeEventListener("blur", clearKeys); :1749 document.removeEventListener("visibilitychange", onVis);, no pointerup/pointermove removal. Named keyboard handlers exist (:380 onKeyDown, :388 onKeyUp, :394 clearKeys) proving the anonymous pointer handlers are the exception. onBeforeUnmount at :1795 onBeforeUnmount(() => cleanup?.());. Early-return guard :304 if (!rl) return; bounds stale CPU cost.


Demo attach mode leaves the 1Hz state poll timer, socket handler, and visibilitychange listener running after the page unmounts

Location: composables/useDemoPlayback.ts:178 (web)

What: startStatePoll() (line 136) installs a module-level setInterval (statePollTimer, line 128) that calls control("state") over the websocket every second, registers socket.on("demo-session:state", stateSocketHandler) (line 164), and adds a document visibilitychange listener (line 174). These are only torn down by stopStatePoll() (178-188), which runs from stop() (line 436) or when the subscription reports the session left 'playing'. In pages/demo/[matchMapId].vue the onBeforeUnmount handler skips stop() entirely when in attach mode: if (!attach.value) { void stop(); } (lines 183-185). The store is never reset in that path, so store.matchMapId stays truthy and tickOnce() keeps passing its guard.

Impact: An operator opens the dev demo popup via /demo/dev or ?attach=1 (attach mode), the session reaches status 'playing' so startStatePoll() runs, then navigates away or closes the popup via SPA navigation. Because stop() is skipped in attach mode and the store is not reset, statePollTimer keeps firing control("state") over the websocket once per second indefinitely, and the demo-session:state socket handler plus visibilitychange listener stay attached, until the next demo is opened (which clears the prior timer via stopStatePoll at line 137).

Suggested fix: Call stopStatePoll() unconditionally in the demo page's onBeforeUnmount (independently of the attach-gated stop() mutation), or have the composable register an onScopeDispose that runs stopStatePoll() so the shared timer/handler/listener are always torn down when the last consumer unmounts.

Verifier evidence

composables/useDemoPlayback.ts:15-16 let statePollTimer... let stateSocketHandler... (module-level, shared); :114 let visibilityHandler: (() => void) | null = null; (instance-level, inside useDemoPlayback); :116 if (!store.matchMapId) return; (tickOnce guard defeated because store not reset); :136-137 function startStatePoll() { stopStatePoll(); (only next-open clears module timer); :178-188 stopStatePoll tears down timer+socket handler+visibility listener; :436 stopStatePoll() only inside stop(); pages/demo/[matchMapId].vue onBeforeUnmount: if (!attach.value) { void stop(); } skips stop() in attach mode, with comment In attach mode we DON'T stop; composables/useSubscriptionManager.ts:1-28 module-level Map with no onScopeDispose, so subscription also survives unmount when unsubscribe (called only from stop) is skipped.


Found by the 2026-07 multi-agent code audit; adversarially verified (CONFIRMED, P2/P3). One of a batch of findings from that pass.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2-mediumCode quality & robustnessaudit-2026-07Findings from the 2026-07 code audit / review passmemory-leakMemory or subscription leakresource-leakResource or connection leakservice:web5stackgg/web service

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions