fix(renderer): WebGL longtask fallback hardening (#89)#91
Merged
Conversation
Three follow-ups from #89 on top of #83's auto-fallback. Splits cleanly along the three sub-items so cherry-picking individual fixes stays possible. 1) Observer disconnect on context loss (the leak) `_installWebGLLongTaskGuard()`'s PerformanceObserver was only disconnected on the trip path. If the GPU context was lost before the trip threshold, the observer outlived the disposed addon and kept firing for every longtask the page emits — minor in absolute terms but matters for the 24+ hour sessions this repo cares about. Extracted `_disposeWebGLObserver()` (idempotent, swallows throws) and called from both the trip path and the `onContextLoss` handler. Documented as the call site for any future terminal-disposal path. 2) Hoisted thresholds to a single constant block `LONGTASK_MS` (200), `LONGTASK_COUNT` (3), `WINDOW_MS` (30000), `GRACE_MS` (5000), and `STICKY_EXPIRY_MS` (7d) are now in `WEBGL_FALLBACK` in constants.js. Replaces inline literals in `app.js:_installWebGLLongTaskGuard` and `terminal-ui.js`'s sticky-expiry check. 3) Tests for trip logic and observer cleanup Split the rolling-window arithmetic into a pure helper `evaluateWebGLLongTaskTrip(recent, entries, now)` so the threshold math is testable without driving a real PerformanceObserver (which can't be invoked deterministically from JS — entries arrive from the platform). `test/webgl-fallback.test.ts` (port 3166) covers: - WEBGL_FALLBACK exposes the documented thresholds - 3 longtasks inside 30s → trip - 3 longtasks spread over 60s → no trip (oldest pruned) - <200ms entries ignored - empty batches still prune stale entries - counts accumulate across batches - `_disposeWebGLObserver` is idempotent, calls disconnect once, swallows throws (driver-quirk defence) Files ----- - src/web/public/constants.js: +WEBGL_FALLBACK, +evaluateWebGLLongTaskTrip (exposed via window.X for tests). - src/web/public/app.js: _initWebGL.onContextLoss now disconnects the observer; _installWebGLLongTaskGuard delegates the trip math to the new helper and uses the hoisted constants; new _disposeWebGLObserver helper. - src/web/public/terminal-ui.js: STICKY_EXPIRY_MS replaces the inline 7-day literal. - test/webgl-fallback.test.ts: 9 Playwright-driven tests on port 3166. Closes #89. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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 #89.
Three follow-ups from #89 on top of the #83 auto-fallback. One commit, but the changes split cleanly along the three sub-items so they can be reviewed (or split) independently.
Summary
_installWebGLLongTaskGuard()'sPerformanceObserverwas onlydisconnect()ed on the trip path. If the GPU context was lost before the threshold (or any future terminal-teardown path runs first), the observer outlived its disposed addon and kept firing on every longtask. Extracted_disposeWebGLObserver()(idempotent, swallows throws) and called it from both the trip path and theonContextLosshandler.200ms / 3 / 30s / 5s grace / 7d stickyare nowWEBGL_FALLBACK.{LONGTASK_MS, LONGTASK_COUNT, WINDOW_MS, GRACE_MS, STICKY_EXPIRY_MS}inconstants.js. Replaces inline literals inapp.jsandterminal-ui.js.evaluateWebGLLongTaskTrip(recent, entries, now).test/webgl-fallback.test.ts(9 tests, port 3166) covers trip ≥ threshold inside window, no-trip when spread > window, sub-threshold entries ignored, stale-entry pruning, cumulative counting across batches, and_disposeWebGLObserverbeing idempotent / swallowing throws.Files
src/web/public/constants.js—+WEBGL_FALLBACK,+evaluateWebGLLongTaskTrip(both attached towindowfor test visibility —constdeclarations in non-module scripts aren't otherwise onwindow).src/web/public/app.js—_initWebGL.onContextLossnow disconnects the observer;_installWebGLLongTaskGuarddelegates trip math to the helper and uses the hoisted constants; new_disposeWebGLObserver.src/web/public/terminal-ui.js—STICKY_EXPIRY_MSreplaces the inline 7-day literal in the sticky-disable check.test/webgl-fallback.test.ts— new (port 3166).Test plan
npx tsc --noEmit— cleannpm run lint— cleannpm run format:check— cleannpm test -- test/webgl-fallback.test.ts— 9/9 passnpm test -- test/inline-rename.test.ts— still 7/7 (sanity-check that the constants.js + app.js touches don't break adjacent browser tests)?webgl=force, paste some heavy output, confirm WebGL stays up under normal loadcanvas.getContext('webgl2').getExtension('WEBGL_lose_context').loseContext()) and confirm the observer reference in DevTools is nulled afterward🤖 Generated with Claude Code