fix: return cached dimensions after renderer dispose - #6064
Open
openwong2kim wants to merge 1 commit into
Open
Conversation
The dimensions getter dereferenced _renderer.value with a non-null assertion (_renderer.value!.dimensions). After dispose, the MutableDisposable clears its value, so the getter throws Cannot read properties of undefined (reading 'dimensions') on any post-dispose access. xtermjs#5586 (mouse event path) and xtermjs#5826 (decoration refresh path) were both fixed per-callsite with isDisposed / hasRenderer() guards. That leaves 20+ remaining unguarded .dimensions accesses across CoreBrowserTerminal, AccessibilityManager, BufferDecorationRenderer and MouseCoordsService. Any new caller or async callback that races dispose will hit the same crash. Cache the last known IRenderDimensions on every read. After dispose the getter returns the cached copy instead of dereferencing the undefined renderer. Stale values are harmless: the terminal is mid-teardown and the data goes nowhere.
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.
Both #5586 (mouse event path) and #5826 (decoration refresh path) crash on the same line:
After
dispose(), theMutableDisposableclears its value, so_renderer.valueisundefinedand the non-null assertion throwsTypeError: Cannot read properties of undefined (reading 'dimensions').#5591 and #5827 fixed the two known crash paths with per-callsite guards (
isDisposed/hasRenderer()checks). That works, but there are 20+ remaining unguarded.dimensionsaccesses across:CoreBrowserTerminal.ts— cursor positioning, overview ruler sizing (10+)AccessibilityManager.ts— live region sizing (4)BufferDecorationRenderer.ts— decoration element sizing (6+)MouseCoordsService.ts— mouse coordinate math (4)Any async callback (rAF, setTimeout, intersection observer) that races
dispose()on one of those paths will hit the same crash. Adding a guard at every callsite is whack-a-mole.This PR fixes the getter itself — cache the last known
IRenderDimensionson every read and return it after dispose. Stale values are harmless since the terminal is mid-teardown.The per-callsite guards from #5591/#5827 can stay (they prevent unnecessary work on a disposed terminal) but are no longer load-bearing for crash safety.
Builds clean with
tsgo -b ./tsconfig.all.json.