JS step debugger: expandHandle() doesn't validate handle freshness across prepare() #75

New Issue

2026-05-06T04:53:22Z

spikerj commented

2026-05-06 04:53:22 +00:00

Problem

JsStepDebuggerService.expandHandle(handle) returns the children of a pseudo-object reference but does not verify the handle was produced by the current interpreter:

expandHandle(handle: unknown): ScopeBinding[] {
  if (!this.interpreter || handle == null) return [];
  return snapshotChildren(handle, this.interpreter);
}

Workflow that exposes the issue:

User starts debug session, expands myObj in the Variables panel.
User clicks Run again — prepare() builds a brand-new interpreter.
The Variables panel still holds a handle reference into the OLD pseudo-object graph.
The next refresh / expansion calls snapshotChildren(staleHandle, newInterpreter). The old object isn’t GC-collected (the panel holds it), so staleHandle.properties still resolves — but interp.pseudoToNative(...) is invoked on the NEW interpreter against values it didn’t allocate.

In practice the panel re-renders on currentScope change (which clears expansions), so this self-heals — but a future panel change that caches expansion state would silently start producing stale data.

Suggested fix

Track a per-interpreter epoch and refuse to expand stale handles:

Bump a private interpreterEpoch = signal<number>(0) inside prepare() and stepBack() (anywhere this.interpreter is rebuilt).
Have snapshotScope and snapshotChildren tag every emitted ScopeBinding with the epoch the handle is valid for.
expandHandle checks the epoch matches interpreterEpoch() and returns [] otherwise.

Alternative: use a WeakMap<handle, interpreter> so the panel can detect staleness on its own end without changing ScopeBinding's shape.

Owner pointers

libraries/tools/src/services/js-step-debugger/js-step-debugger.service.ts:579-582 (expandHandle)
libraries/tools/src/services/js-step-debugger/scope-snapshot.ts (where ScopeBinding is defined)

## Problem `JsStepDebuggerService.expandHandle(handle)` returns the children of a pseudo-object reference but does not verify the handle was produced by the *current* interpreter: ```ts expandHandle(handle: unknown): ScopeBinding[] { if (!this.interpreter || handle == null) return []; return snapshotChildren(handle, this.interpreter); } ``` Workflow that exposes the issue: 1. User starts debug session, expands `myObj` in the Variables panel. 2. User clicks **Run** again — `prepare()` builds a brand-new interpreter. 3. The Variables panel still holds a `handle` reference into the OLD pseudo-object graph. 4. The next refresh / expansion calls `snapshotChildren(staleHandle, newInterpreter)`. The old object isn’t GC-collected (the panel holds it), so `staleHandle.properties` still resolves — but `interp.pseudoToNative(...)` is invoked on the NEW interpreter against values it didn’t allocate. In practice the panel re-renders on `currentScope` change (which clears expansions), so this self-heals — but a future panel change that caches expansion state would silently start producing stale data. ## Suggested fix Track a per-interpreter epoch and refuse to expand stale handles: - Bump a `private interpreterEpoch = signal<number>(0)` inside `prepare()` and `stepBack()` (anywhere `this.interpreter` is rebuilt). - Have `snapshotScope` and `snapshotChildren` tag every emitted `ScopeBinding` with the epoch the handle is valid for. - `expandHandle` checks the epoch matches `interpreterEpoch()` and returns `[]` otherwise. Alternative: use a `WeakMap<handle, interpreter>` so the panel can detect staleness on its own end without changing `ScopeBinding`'s shape. ## Owner pointers - `libraries/tools/src/services/js-step-debugger/js-step-debugger.service.ts:579-582` (`expandHandle`) - `libraries/tools/src/services/js-step-debugger/scope-snapshot.ts` (where `ScopeBinding` is defined) ## Related Found during code review of the Monaco migration + JS debugger feature wave.

spikerj commented

2026-06-15 14:48:41 +00:00

Fixed in spikersoft-angular PR #53. Implemented the WeakMap-style alternative from the suggested fix: a per-interpreter WeakSet<object> of live handles (reset on prepare/stepBack/stop, populated as bindings are emitted), and expandHandle() now rejects handles that aren't part of the current interpreter. Also closed the concrete leak in stepBack(), which restored frame.currentScope whose cached handles pointed at the pre-serialize graph — it now re-derives scope from the rebuilt interpreter. ScopeBinding's shape is unchanged. Added regression tests (live-handle expansion + stale-handle rejection); 51/51 step-debugger tests pass. Will close once PR #53 merges.

Fixed in `spikersoft-angular` PR #53. Implemented the WeakMap-style alternative from the suggested fix: a per-interpreter `WeakSet<object>` of live handles (reset on `prepare`/`stepBack`/`stop`, populated as bindings are emitted), and `expandHandle()` now rejects handles that aren't part of the current interpreter. Also closed the concrete leak in `stepBack()`, which restored `frame.currentScope` whose cached handles pointed at the pre-serialize graph — it now re-derives scope from the rebuilt interpreter. `ScopeBinding`'s shape is unchanged. Added regression tests (live-handle expansion + stale-handle rejection); 51/51 step-debugger tests pass. Will close once PR #53 merges.

spikerj commented

2026-06-15 15:33:32 +00:00