From 644c7175527c3401967887329956a73085b80fab Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Wed, 6 May 2026 10:48:29 -0700 Subject: [PATCH] Guard terminal resize/dispose race against xterm.js dimension getters After bumping xterm.js to beta.213 (#313817), three error buckets keep firing the same Cannot read properties of undefined (reading 'dimensions') TypeError on the latest insider builds (1.120.0-insider 79d6cea3 / 0aed0a9b): - aaa283a2 - resize -> _resizeYCallback -> _updatePtyDimensions, triggered by config-change -> setVisible -> _resize. - 4826565b - debounced _debounceResizeX timer firing after the renderer is gone. - 1e83a096 - _onProcessExit -> dispose -> setVisible(false) -> _resize -> _resizeBothCallback. All three reach RenderService.get dimensions() in xterm.js, which dereferences this._renderer.value (undefined post-dispose) and throws synchronously. The optional chaining on rawXterm.dimensions doesn't help because the getter itself throws. The upstream beta.213 fix added isDisposed guards at the OverviewRuler call sites, not inside the getter, so the correct symmetrical fix here is to guard the call sites that VS Code owns: * terminalInstance.ts: bail out of the resize closures and _updatePtyDimensions when the instance is already disposed; defensively wrap the dimensions read in try/catch so a future renderer-dispose race fails silently instead of bubbling as an unhandled error. * terminalResizeDebouncer.ts: bail out of resize/flush and the runWhenWindowIdle / @debounce-scheduled callbacks when the debouncer's store is disposed - the @debounce decorator schedules a setTimeout that isn't tied to the disposable store. Refs microsoft/vscode#303546, microsoft/vscode#313817 --- .../terminal/browser/terminalInstance.ts | 27 +++++++++++++++++-- .../browser/terminalResizeDebouncer.ts | 18 +++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index a14f0adcfe9e9..1499006feaac8 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -816,14 +816,23 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { () => this._isVisible, () => xterm, async (cols, rows) => { + if (this.isDisposed) { + return; + } xterm.resize(cols, rows); await this._updatePtyDimensions(xterm.raw); }, async (cols) => { + if (this.isDisposed) { + return; + } xterm.resize(cols, xterm.raw.rows); await this._updatePtyDimensions(xterm.raw); }, async (rows) => { + if (this.isDisposed) { + return; + } xterm.resize(xterm.raw.cols, rows); await this._updatePtyDimensions(xterm.raw); } @@ -2047,8 +2056,22 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { } private async _updatePtyDimensions(rawXterm: XTermTerminal): Promise { - const pixelWidth = rawXterm.dimensions?.css.canvas.width; - const pixelHeight = rawXterm.dimensions?.css.canvas.height; + if (this.isDisposed) { + return; + } + // `rawXterm.dimensions` proxies to xterm.js' RenderService which throws + // `Cannot read properties of undefined (reading 'dimensions')` if the + // renderer was disposed between scheduling and invocation (debounced or + // idle resize callbacks racing with terminal teardown). Guard against + // that here so the optional chaining short-circuits to undefined. + let pixelWidth: number | undefined; + let pixelHeight: number | undefined; + try { + pixelWidth = rawXterm.dimensions?.css.canvas.width; + pixelHeight = rawXterm.dimensions?.css.canvas.height; + } catch { + // Renderer disposed mid-flight; fall through with undefined dimensions. + } const roundedPixelWidth = pixelWidth ? Math.round(pixelWidth) : undefined; const roundedPixelHeight = pixelHeight ? Math.round(pixelHeight) : undefined; await this._processManager.setDimensions(rawXterm.cols, rawXterm.rows, undefined, roundedPixelWidth, roundedPixelHeight); diff --git a/src/vs/workbench/contrib/terminal/browser/terminalResizeDebouncer.ts b/src/vs/workbench/contrib/terminal/browser/terminalResizeDebouncer.ts index 8afe11348cc2b..88062ff24fb7f 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalResizeDebouncer.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalResizeDebouncer.ts @@ -33,6 +33,9 @@ export class TerminalResizeDebouncer extends Disposable { } async resize(cols: number, rows: number, immediate: boolean): Promise { + if (this._store.isDisposed) { + return; + } this._latestX = cols; this._latestY = rows; @@ -49,12 +52,18 @@ export class TerminalResizeDebouncer extends Disposable { if (win && !this._isVisible()) { if (!this._resizeXJob.value) { this._resizeXJob.value = runWhenWindowIdle(win, async () => { + if (this._store.isDisposed) { + return; + } this._resizeXCallback(this._latestX); this._resizeXJob.clear(); }); } if (!this._resizeYJob.value) { this._resizeYJob.value = runWhenWindowIdle(win, async () => { + if (this._store.isDisposed) { + return; + } this._resizeYCallback(this._latestY); this._resizeYJob.clear(); }); @@ -70,6 +79,9 @@ export class TerminalResizeDebouncer extends Disposable { } flush(): void { + if (this._store.isDisposed) { + return; + } if (this._resizeXJob.value || this._resizeYJob.value) { this._resizeXJob.clear(); this._resizeYJob.clear(); @@ -79,6 +91,12 @@ export class TerminalResizeDebouncer extends Disposable { @debounce(100) private _debounceResizeX(cols: number) { + // The @debounce decorator schedules a setTimeout that is not tied to the + // disposable store, so this can fire after the terminal/xterm renderer is + // disposed. Bail out to avoid throwing from xterm.js dimension getters. + if (this._store.isDisposed) { + return; + } this._resizeXCallback(cols); } }