Skip to content

fix: return cached dimensions after renderer dispose - #6064

Open
openwong2kim wants to merge 1 commit into
xtermjs:masterfrom
openwong2kim:fix/dimensions-getter-post-dispose
Open

fix: return cached dimensions after renderer dispose#6064
openwong2kim wants to merge 1 commit into
xtermjs:masterfrom
openwong2kim:fix/dimensions-getter-post-dispose

Conversation

@openwong2kim

Copy link
Copy Markdown

Both #5586 (mouse event path) and #5826 (decoration refresh path) crash on the same line:

public get dimensions(): IRenderDimensions { return this._renderer.value!.dimensions; }

After dispose(), the MutableDisposable clears its value, so _renderer.value is undefined and the non-null assertion throws TypeError: 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 .dimensions accesses 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 IRenderDimensions on every read and return it after dispose. Stale values are harmless since the terminal is mid-teardown.

- public get dimensions(): IRenderDimensions { return this._renderer.value!.dimensions; }
+ private _cachedDimensions: IRenderDimensions = createRenderDimensions();
+ public get dimensions(): IRenderDimensions {
+   const renderer = this._renderer.value;
+   if (renderer) {
+     this._cachedDimensions = renderer.dimensions;
+   }
+   return this._cachedDimensions;
+ }

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.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants