fix(runtime): keep the animation scheduler alive when a tick callback throws (#194)
onTick set `isDispatching = true`, ran the subscriber callbacks, then reset the flag, flushed `pending`, and rescheduled with no try/finally. A throwing callback skipped all three, leaving `isDispatching` stuck true forever: every later subscribe/unsubscribe queued into `pending` and never ran, and no timer was ever rescheduled. One bad tick permanently killed every `useAnimation` instance sharing the (process-wide) scheduler — a non-recoverable wedge. Wrap the dispatch loop in try/finally so the scheduler invariants are always restored and the error still propagates (restore-then-rethrow, mirroring scheduler.ts `doCommit`). Also advance each subscriber's `nextDueTime` BEFORE invoking its callback, so a thrower can't leave it in the past and make the post-throw schedule() re-arm a 0ms tight re-throw loop. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -152,6 +152,39 @@ describe.sequential("createAnimationScheduler", () => {
|
|||||||
s.dispose();
|
s.dispose();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("a throwing subscriber callback does not wedge the shared scheduler", () => {
|
||||||
|
// Regression: onTick set `isDispatching = true`, ran the subscriber
|
||||||
|
// callbacks, then reset the flag / flushed `pending` / rescheduled with NO
|
||||||
|
// try/finally. A throwing callback skipped all three, leaving isDispatching
|
||||||
|
// stuck true forever — every later subscribe/unsubscribe queued into
|
||||||
|
// `pending` and never ran, and no timer was ever rescheduled. One bad tick
|
||||||
|
// permanently killed EVERY useAnimation instance sharing this scheduler.
|
||||||
|
const s = createAnimationScheduler();
|
||||||
|
const boom = vi.fn(() => {
|
||||||
|
throw new Error("boom in tick");
|
||||||
|
});
|
||||||
|
const boomHandle = s.subscribe(boom, 50);
|
||||||
|
|
||||||
|
// The throw propagates out of the timer callback (house idiom: restore the
|
||||||
|
// scheduler invariants, then rethrow — mirrors scheduler.ts doCommit). That
|
||||||
|
// is expected; what must NOT happen is the scheduler wedging.
|
||||||
|
expect(() => vi.advanceTimersByTime(50)).toThrow("boom in tick");
|
||||||
|
expect(boom).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// Remove the thrower (must run synchronously — proves isDispatching was
|
||||||
|
// reset; on the buggy code this op was queued into `pending` and never ran).
|
||||||
|
boomHandle.unsubscribe();
|
||||||
|
|
||||||
|
// A subscriber added after the throw must still tick. On the buggy code its
|
||||||
|
// add() was queued into `pending` (isDispatching never reset) and no timer
|
||||||
|
// was ever scheduled, so it never fired.
|
||||||
|
const recovered = vi.fn();
|
||||||
|
s.subscribe(recovered, 50);
|
||||||
|
vi.advanceTimersByTime(50);
|
||||||
|
expect(recovered).toHaveBeenCalledTimes(1);
|
||||||
|
s.dispose();
|
||||||
|
});
|
||||||
|
|
||||||
test("dispose clears everything", () => {
|
test("dispose clears everything", () => {
|
||||||
const s = createAnimationScheduler();
|
const s = createAnimationScheduler();
|
||||||
s.subscribe(vi.fn(), 50);
|
s.subscribe(vi.fn(), 50);
|
||||||
|
|||||||
@@ -68,19 +68,33 @@ export function createAnimationScheduler(renderThrottleMs = 0): AnimationSchedul
|
|||||||
scheduledDueTime = Number.POSITIVE_INFINITY;
|
scheduledDueTime = Number.POSITIVE_INFINITY;
|
||||||
const now = performance.now();
|
const now = performance.now();
|
||||||
isDispatching = true;
|
isDispatching = true;
|
||||||
|
try {
|
||||||
for (const s of subscribers) {
|
for (const s of subscribers) {
|
||||||
if (s.cancelled || now < s.nextDueTime) continue;
|
if (s.cancelled || now < s.nextDueTime) continue;
|
||||||
s.callback(now);
|
// Advance nextDueTime BEFORE invoking the callback. If the callback
|
||||||
if (s.cancelled) continue;
|
// throws, leaving nextDueTime in the past would make the post-throw
|
||||||
|
// schedule() re-arm a 0ms timer that re-fires and re-throws on a tight
|
||||||
|
// loop. A subscriber that cancels itself inside the callback is filtered
|
||||||
|
// by the cancelled checks here and in schedule(), so pre-advancing a
|
||||||
|
// then-cancelled sub is harmless (matches the old post-callback advance).
|
||||||
const elapsedFrames = Math.floor((now - s.startTime) / s.interval) + 1;
|
const elapsedFrames = Math.floor((now - s.startTime) / s.interval) + 1;
|
||||||
s.nextDueTime = s.startTime + elapsedFrames * s.interval;
|
s.nextDueTime = s.startTime + elapsedFrames * s.interval;
|
||||||
|
s.callback(now);
|
||||||
}
|
}
|
||||||
|
} finally {
|
||||||
|
// A throwing subscriber callback must never wedge the SHARED scheduler.
|
||||||
|
// Without this finally, isDispatching stays true forever: every later
|
||||||
|
// subscribe/unsubscribe queues into `pending` and never runs, and no timer
|
||||||
|
// is rescheduled — one bad tick silently kills every useAnimation instance
|
||||||
|
// app-wide. Restore the invariants here (mirroring scheduler.ts doCommit's
|
||||||
|
// try/finally) and let the error propagate — restore, then rethrow.
|
||||||
isDispatching = false;
|
isDispatching = false;
|
||||||
if (pending.length > 0) {
|
if (pending.length > 0) {
|
||||||
for (const op of pending.splice(0)) op();
|
for (const op of pending.splice(0)) op();
|
||||||
}
|
}
|
||||||
schedule();
|
schedule();
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function subscribe(callback: (currentTime: number) => void, intervalRaw: number) {
|
function subscribe(callback: (currentTime: number) => void, intervalRaw: number) {
|
||||||
const interval = normalizeInterval(intervalRaw);
|
const interval = normalizeInterval(intervalRaw);
|
||||||
|
|||||||
Reference in New Issue
Block a user