Repository navigation
fix(core): reschedule when requestRender races the loop scheduling - #1086
Closed
PeterPonyu wants to merge 1 commit into
Closed
PeterPonyu wants to merge 1 commit into
PeterPonyu wants to merge 1 commit into
Conversation
…nomalyco#789) If `requestRender()` fires while a frame is in flight (`this.rendering` true), it is demoted to setting `immediateRerenderRequested` and returns. The loop's end-of-frame schedule decision at line 3995 reads that flag — but a request that arrives between that read and `this.rendering = false` in the finally block is dropped: the read already happened, and after the finally the renderer goes idle with no pending timer. Per the reporter's RCA in anomalyco#789, this manifests as a blank screen with the process healthy (`rendering=false scheduled=false running=false`) and modals still rendering correctly on top — the next paint never fires. Re-check `immediateRerenderRequested` inside the loop's finally, clear it, and re-issue. Defer the re-issue with `setImmediate`: on the one-off (paused / idle) scheduling path, `activateFrame` keeps `updateScheduled = true` until its own finally runs, which is after ours. `process.nextTick` would fire before that, before the `!updateScheduled` guard in `requestRender()` would let us through. `setImmediate` runs after the microtask queue drains, by which point `activateFrame`'s finally has cleared the flag. The regression test exercises the one-off path explicitly (paused mode), monkey-patches `clock.clearTimeout` to fire a sync requestRender while `rendering` is still true, and asserts that at least a second frame still happens. Without the fix, only one frame renders before the renderer stalls. Closes anomalyco#789 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #789.
What changed
If
requestRender()fires while a frame is in flight (this.renderingis true), it is demoted to settingimmediateRerenderRequestedand returns. The loop's end-of-frame schedule decision atrenderer.ts:3995reads that flag and reschedules, but a request that arrives between that read andthis.rendering = falsein the finally block is dropped: the read already happened, and after the finally the renderer goes idle with no pending timer.The reporter's RCA in #789 describes the symptom: blank screen with
rendering=false scheduled=false running=false, the process otherwise healthy, modals still rendering on top — the next paint never fires.The fix re-checks
immediateRerenderRequestedinside the loop's finally, clears it, and re-issues the request viasetImmediate.} finally { this.rendering = false if (this._destroyPending) { this.finalizeDestroy() + } else if (this.immediateRerenderRequested) { + this.immediateRerenderRequested = false + setImmediate(() => { + if (!this._isDestroyed) this.requestRender() + }) } this.resolveIdleIfNeeded() }Why
setImmediate(notnextTick)On the one-off (paused / idle) scheduling path,
activateFramekeepsupdateScheduled = trueuntil its own finally runs, and that finally runs afterloop()'s finally.process.nextTickfires before microtasks resume, which means it runs beforeactivateFrame's finally — at which pointrequestRender()is blocked by the!updateScheduledguard at line 1247 and silently no-ops.setImmediateruns after the microtask queue drains, by which pointupdateScheduledis back to false and the request schedules normally.How I verified
cd packages/core && bun test src/tests/renderer.control.test.ts— 29 pass, 0 fail. New test "loop() finally reschedules when requestRender races the schedule decision (#789)" exercises the one-off path:clock.clearTimeout(called in the loop's else branch at line 4004) to fire a syncrequestRender()whilerenderingis still truerenderCount >= 2Confirmed the test fails without the fix (
Received: 1) and passes with it by revertingrenderer.tsand re-running.Also ran the broader
bun test src/tests/renderer*sweep — 408 pass, 0 fail.bunx oxfmtapplied.