JS step debugger: stepInto/Over/Out can freeze the tab on heavy steps #74

Closed
opened 2026-05-06 04:53:05 +00:00 by spikerj · 1 comment
Owner

Problem

JsStepDebuggerService.runUntil() (used by stepInto, stepOver, and stepOut) is a fully synchronous while loop bounded only by STEP_BUDGET = 50_000 micro-steps. While the budget prevents the truly-pathological hang, a single "Step Over" across a function that calls arr.forEach(...) on a sizeable array can block the main thread for 100–500ms with no visual feedback. The user clicks once and the tab “hangs” — in an educational sandbox that reads as a broken tool.

Contrast: the tick() run loop already chunks into batches of STEPS_PER_TICK = 200 and yields via setTimeout(0) between batches, so long Run sessions stay responsive. The step-action path skipped that pattern presumably for simplicity.

Repro

  1. Open /tools/javascript/lessons in the playground.
  2. Paste:
    function spin(n) {
      var s = 0;
      for (var i = 0; i < n; i++) s += i;
      return s;
    }
    spin(20000);
    
  3. Set a breakpoint on the spin(20000) line, click Step Into to descend, then click Step Over on the spin(20000) invocation.
  4. Observe the tab is unresponsive for ~hundreds of ms; the status chip never visibly updates to “Running…”.

Suggested fix (cheap)

Make runUntil iterative-with-yield:

  • Run an inner batch of STEPS_PER_TICK micro-steps.
  • After each batch, schedule the next batch via setTimeout(0) and return.
  • Stop conditions (predicate / breakpoint / completion / budget) stay the same.

The public surface (stepInto/Over/Out) becomes fire-and-forget instead of synchronous, but already-async UI consumers don't care; the only adjustment is in tests that assert state synchronously after a step call.

Suggested fix (deferred / cheaper)

Lower STEP_BUDGET to ~5,000 and surface "Step took too long; click Step again to continue" in the chip. Less invasive, uglier UX, but unblocks shipping.

Owner pointers

  • libraries/tools/src/services/js-step-debugger/js-step-debugger.service.tsrunUntil() lines 661–707, tick() lines 596–652 for the chunk-yield pattern to mirror.
  • Tests live at services/js-step-debugger/js-step-debugger.service.spec.ts and currently invoke stepInto/Over/Out synchronously — they’ll need await after the migration.

Related

Found during code review of the Monaco migration + JS debugger feature wave.

## Problem `JsStepDebuggerService.runUntil()` (used by `stepInto`, `stepOver`, and `stepOut`) is a fully synchronous `while` loop bounded only by `STEP_BUDGET = 50_000` micro-steps. While the budget prevents the truly-pathological hang, a single "Step Over" across a function that calls `arr.forEach(...)` on a sizeable array can block the main thread for 100–500ms with no visual feedback. The user clicks once and the tab “hangs” — in an educational sandbox that reads as a broken tool. Contrast: the `tick()` run loop already chunks into batches of `STEPS_PER_TICK = 200` and yields via `setTimeout(0)` between batches, so long Run sessions stay responsive. The step-action path skipped that pattern presumably for simplicity. ## Repro 1. Open `/tools/javascript/lessons` in the playground. 2. Paste: ```js function spin(n) { var s = 0; for (var i = 0; i < n; i++) s += i; return s; } spin(20000); ``` 3. Set a breakpoint on the `spin(20000)` line, click **Step Into** to descend, then click **Step Over** on the `spin(20000)` invocation. 4. Observe the tab is unresponsive for ~hundreds of ms; the status chip never visibly updates to “Running…”. ## Suggested fix (cheap) Make `runUntil` iterative-with-yield: - Run an inner batch of `STEPS_PER_TICK` micro-steps. - After each batch, schedule the next batch via `setTimeout(0)` and return. - Stop conditions (predicate / breakpoint / completion / budget) stay the same. The public surface (`stepInto/Over/Out`) becomes fire-and-forget instead of synchronous, but already-async UI consumers don't care; the only adjustment is in tests that assert state synchronously after a step call. ## Suggested fix (deferred / cheaper) Lower `STEP_BUDGET` to ~5,000 and surface "Step took too long; click Step again to continue" in the chip. Less invasive, uglier UX, but unblocks shipping. ## Owner pointers - `libraries/tools/src/services/js-step-debugger/js-step-debugger.service.ts` — `runUntil()` lines 661–707, `tick()` lines 596–652 for the chunk-yield pattern to mirror. - Tests live at `services/js-step-debugger/js-step-debugger.service.spec.ts` and currently invoke `stepInto/Over/Out` synchronously — they’ll need `await` after the migration. ## Related Found during code review of the Monaco migration + JS debugger feature wave.
Author
Owner

Fixed.

Summary of changes

libraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts — went with the cheap fix from the ticket. runUntil(predicate) now mirrors tick()'s chunked-yield pattern: an inner batch of STEPS_PER_TICK = 200 micro-steps, then re-enter via setTimeout(0). STEP_BUDGET = 50_000 is now enforced across the whole call rather than spinning a synchronous while, so a Step Over across spin(20000) (the repro) never blocks the main thread for more than a single batch (~1–2ms). _state.set("running") happens synchronously at entry so the toolbar status chip flips to “Running…” the moment the user clicks — that's the user-visible payoff.

API surfacestepInto / stepOver / stepOut now return Promise<void>. Fire-and-forget callers (Angular templates) need no change; consumers driving multiple steps in a loop must await.

Cancellation — added a private runUntilResolver field plus a finishRunUntil() helper. pause(), stop(), and prepare() all drain it, so an await stepInto() parked between batches resolves cleanly when the user clicks Pause instead of dangling forever. The pause-mid-step interrupt is the second user-visible win (used to be a no-op because the synchronous loop never gave the event loop a chance to deliver the click).

libraries/features/dev-tools-blockly/src/lib/interpreters/javascript-interpreter.service.ts — the only consumer that drove the loop synchronously. Its step() method's inner for loop now awaits this.core.stepInto() before reading state(), otherwise the loop would break on the first iteration with state still "running".

Tests

  • Migrated js-step-debugger.service.spec.ts — every stepInto/stepOver/stepOut is now awaited and the wrapping it() callbacks are async, exactly as the owner pointers in the ticket warned.
  • New describe("issue #74 -- step actions yield to the browser") block with three regressions:
    1. stepInto() returns a Promise (API contract guard).
    2. Step Over across spin(2000) (~12k internal micro-steps, ≫ STEPS_PER_TICK) settles in "paused" on the next line without exhausting the budget.
    3. pause() mid-step resolves the awaiter to "paused" — pre-fix this would hang.
  • All 49 tests pass (46 prior + 3 new). Typecheck clean on platform-js-step-debugger, feature-dev-tools-blockly, and feature-dev-tools-javascript-runner.

Note on stale paths

The owner pointers in the ticket cited libraries/tools/src/services/js-step-debugger/...; the service has since been extracted into its own library at libraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts. Line numbers had also drifted by ~8 (runUntil 669–715, tick 604–660 pre-fix). Structure was unchanged so the fix dropped in cleanly.

Closing.

Fixed. ## Summary of changes **`libraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts`** — went with the cheap fix from the ticket. `runUntil(predicate)` now mirrors `tick()`'s chunked-yield pattern: an inner batch of `STEPS_PER_TICK = 200` micro-steps, then re-enter via `setTimeout(0)`. `STEP_BUDGET = 50_000` is now enforced across the whole call rather than spinning a synchronous `while`, so a Step Over across `spin(20000)` (the repro) never blocks the main thread for more than a single batch (~1–2ms). `_state.set("running")` happens synchronously at entry so the toolbar status chip flips to “Running…” the moment the user clicks — that's the user-visible payoff. **API surface** — `stepInto` / `stepOver` / `stepOut` now return `Promise<void>`. Fire-and-forget callers (Angular templates) need no change; consumers driving multiple steps in a loop must `await`. **Cancellation** — added a private `runUntilResolver` field plus a `finishRunUntil()` helper. `pause()`, `stop()`, and `prepare()` all drain it, so an `await stepInto()` parked between batches resolves cleanly when the user clicks Pause instead of dangling forever. The pause-mid-step interrupt is the second user-visible win (used to be a no-op because the synchronous loop never gave the event loop a chance to deliver the click). **`libraries/features/dev-tools-blockly/src/lib/interpreters/javascript-interpreter.service.ts`** — the only consumer that drove the loop synchronously. Its `step()` method's inner `for` loop now `await`s `this.core.stepInto()` before reading `state()`, otherwise the loop would break on the first iteration with state still `"running"`. ## Tests - Migrated `js-step-debugger.service.spec.ts` — every `stepInto/stepOver/stepOut` is now `await`ed and the wrapping `it()` callbacks are `async`, exactly as the owner pointers in the ticket warned. - New `describe("issue #74 -- step actions yield to the browser")` block with three regressions: 1. `stepInto()` returns a `Promise` (API contract guard). 2. Step Over across `spin(2000)` (~12k internal micro-steps, ≫ `STEPS_PER_TICK`) settles in `"paused"` on the next line without exhausting the budget. 3. `pause()` mid-step resolves the awaiter to `"paused"` — pre-fix this would hang. - All 49 tests pass (46 prior + 3 new). Typecheck clean on `platform-js-step-debugger`, `feature-dev-tools-blockly`, and `feature-dev-tools-javascript-runner`. ## Note on stale paths The owner pointers in the ticket cited `libraries/tools/src/services/js-step-debugger/...`; the service has since been extracted into its own library at `libraries/platform/js-step-debugger/src/lib/js-step-debugger.service.ts`. Line numbers had also drifted by ~8 (`runUntil` 669–715, `tick` 604–660 pre-fix). Structure was unchanged so the fix dropped in cleanly. Closing.
Sign in to join this conversation.