Fix setInterval starving the JS dispatch queue

[bkaradzic-microsoft]

Problem

TimeoutDispatcher::ThreadFunction re-arms a repeating timeout on the timer thread, immediately, before its callback has run on the JS thread:

const auto id = m_timeMap.begin()->second->id;
m_timeMap.erase(m_timeMap.begin());
const auto repeat = m_idMap[id]->interval.has_value();
if (repeat)
{
    const auto timeout = std::move(m_idMap.extract(id).mapped());
    DispatchImpl(std::move(timeout->function), *timeout->interval, true, timeout->id);
}
CallFunction(id);   // -> m_runtime.Dispatch(...), i.e. enqueue onto the JS thread

With delay == 0 the re-inserted entry gets time = Now(), which is always already due, so the timer thread's wait_until never blocks. The thread spins, enqueueing callbacks onto the JS dispatch queue far faster than the JS thread can drain them. That queue is unbounded, so it grows without limit and every other item on it is starved arbitrarily long — other timers, and native async completions posted through JsRuntimeScheduler.

This is easy to hit by accident. setInterval(fn) with no delay argument produces a delay of 0, and driving a render loop that way is a common pattern.

Reproducing with no framework involved at all — a 0-delay setInterval and a setTimeout(…, 100) racing each other:

	ticks	setTimeout(100) fired	elapsed
before	2000	no	759ms
after	52	yes, at 111ms	114ms

The callback was never lost, only buried: if the interval is stopped with clearInterval, the overdue setTimeout fires ~40ms later — 2.4s late.

Downstream impact

In Babylon Native this made NativeEngine::CreateProgramAsync's shader-compile continuation (dispatched via JsRuntimeScheduler) never arrive, so effects stayed isReady === false with an empty compilationError, scene readiness callbacks never fired, and the app hung silently with no error. See BabylonJS/BabylonNative#1814.

The signature that isolates it: an identical scene reaches ready in ~13 ticks when driven by engine.runRenderLoop, and never when driven by setInterval + beginFrame/render/endFrame. The frame driver is the only variable.

Fix

Re-arm a repeating timeout on the JS thread, after its callback has actually returned, so an interval can have at most one invocation pending at a time — matching browser behaviour.

Details worth calling out:

• Cadence is preserved. The next deadline is anchored to the previous scheduled time (scheduledTime + interval), not the completion time, so a slow callback does not make the interval drift. It is clamped to not be in the past. The existing test asserting elapsed >= tickCount * 10 for a 10ms interval still passes.
• Id reuse is handled. A monotonic sequence on Timeout distinguishes a timeout from a later one that happens to reuse the same id, so an in-flight callback can never re-arm its replacement, and a clearInterval issued from inside a callback is honoured.
• The condition variable is notified on re-arm. While a lone interval is in flight m_timeMap is empty, so the timer thread is parked in m_condVariable.wait.
• Clear() stays correct. While in flight the entry is absent from m_timeMap, so its equal_range finds nothing and it simply erases from m_idMap; the re-arm check then skips. Re-arm updates timeout->time before re-inserting, which Clear() matches on.
• Fixes a latent null dereference. The old code read m_idMap[id]->interval with operator[], which default-constructs a null unique_ptr when the id is absent and then dereferences it.
• A throwing tick does not stop the interval. Since NAPI_CPP_EXCEPTIONS is enabled whenever the compiler has exceptions on, function->Call throws a C++ Napi::Error. Re-arming after the callback means that exception would unwind past the re-arm, so the interval is re-armed from a catch and the error is then re-raised via ThrowAsJavaScriptException() — leaving JsRuntime::Dispatch's existing pending-exception handling untouched. Browsers behave the same way, as did this dispatcher before the change.

Zero-delay intervals still run at full speed — they just no longer build a backlog. After the fix a 0-delay interval reached 38,357 ticks in the 100ms before the competing setTimeout fired.

Tests

Two tests added to the setInterval suite in Tests/UnitTests/Scripts/tests.ts:

• should not starve other queued work when the interval has no delay — the regression test.
• should stop when cleared from within its own callback — covers the new re-arm path.

One native test added to Tests/UnitTests/Shared/Shared.cpp:

• Scheduling.IntervalSurvivesThrowingCallback — an interval that throws on the first two ticks must still reach the third, and both errors must still reach UnhandledExceptionHandler. This one has to be native, because the unit test host's UnhandledExceptionHandler sets the exit code to -1 and so a throwing timer in tests.ts would fail the entire JavaScript suite.

218 JavaScript tests + 7 native tests passing, 0 failing.

Both regression tests were negative-controlled by reverting only the corresponding production change and keeping the test:

Test	Without the fix
should not starve other queued work…	Suite hangs permanently and must be killed. Mocha's own timeout is itself a setTimeout, so it cannot fire either — a fair illustration of how total the starvation is.
Scheduling.IntervalSurvivesThrowingCallback	Fails after a 10s timeout: the interval stopped after a tick threw.

Also validated against Babylon Native (BabylonNative#1805 branch, 301-test Playground validation suite):

• 301/301 passing, unchanged, no timing regressions.
• The gizmo readiness matrix {setInterval, runRenderLoop} × {parallelShaderCompile on, off} goes from 2 of 4 cells hanging to 4 of 4 ready.
• Validation test idx 170 (Nested BBG), previously an unbounded hang, now passes in 2s.

[Copilot]

Pull request overview

This PR fixes a starvation/backlog issue caused by 0-delay setInterval repeatedly re-arming on the timer thread, which could spin the timer thread and flood the JS dispatch queue, indefinitely delaying unrelated queued work.

Changes:

• Re-arms repeating timeouts on the JS thread only after the callback completes, ensuring at most one pending invocation per interval.
• Adds a monotonic sequence to distinguish reused timeout IDs and avoid re-arming the wrong timer.
• Adds two unit tests covering the starvation regression and clearing an interval from within its own callback.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
Tests/UnitTests/Scripts/tests.ts	Adds regression and behavioral tests for 0-delay interval starvation and clear-within-callback semantics.
Polyfills/Scheduling/Source/TimeoutDispatcher.h	Extends dispatcher state/signature to support per-timeout sequencing.
Polyfills/Scheduling/Source/TimeoutDispatcher.cpp	Moves interval re-arm to post-callback execution on the JS thread; introduces sequence handling and re-arm notification.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.