From 1558fd55c2d965f6ef3a12232754e1400d6348ec Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 17 Jun 2018 22:26:38 +0800 Subject: [PATCH] Clear up references causing dispose issues in terminal Part of #46356 --- .../terminal/browser/terminalWidgetManager.ts | 10 +++++++- .../electron-browser/terminalActions.ts | 7 +++--- .../electron-browser/terminalInstance.ts | 23 ++++++++++--------- .../electron-browser/terminalLinkHandler.ts | 1 + .../terminal/node/terminalCommandTracker.ts | 7 +++++- 5 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/vs/workbench/parts/terminal/browser/terminalWidgetManager.ts b/src/vs/workbench/parts/terminal/browser/terminalWidgetManager.ts index e8d18df087e..26db47bc76e 100644 --- a/src/vs/workbench/parts/terminal/browser/terminalWidgetManager.ts +++ b/src/vs/workbench/parts/terminal/browser/terminalWidgetManager.ts @@ -7,7 +7,7 @@ import { IDisposable, dispose } from 'vs/base/common/lifecycle'; const WIDGET_HEIGHT = 29; -export class TerminalWidgetManager { +export class TerminalWidgetManager implements IDisposable { private _container: HTMLElement; private _xtermViewport: HTMLElement; @@ -24,6 +24,14 @@ export class TerminalWidgetManager { this._initTerminalHeightWatcher(terminalWrapper); } + public dispose(): void { + if (this._container) { + this._container.parentElement.removeChild(this._container); + this._container = null; + } + this._xtermViewport = null; + } + private _initTerminalHeightWatcher(terminalWrapper: HTMLElement) { // Watch the xterm.js viewport for style changes and do a layout if it changes this._xtermViewport = terminalWrapper.querySelector('.xterm-viewport'); diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts index d8fc7e37acb..1600743536d 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalActions.ts @@ -71,9 +71,10 @@ export class KillTerminalAction extends Action { } public run(event?: any): TPromise { - const terminalInstance = this.terminalService.getActiveInstance(); - if (terminalInstance) { - this.terminalService.getActiveInstance().dispose(); + console.log('kill'); + const instance = this.terminalService.getActiveInstance(); + if (instance) { + instance.dispose(); if (this.terminalService.terminalInstances.length > 0) { this.terminalService.showPanel(true); } diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts index f924bc1d921..7f7ebf03667 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts @@ -155,14 +155,14 @@ export class TerminalInstance implements ITerminalInstance { } }); - this._configurationService.onDidChangeConfiguration(e => { + this.addDisposable(this._configurationService.onDidChangeConfiguration(e => { if (e.affectsConfiguration('terminal.integrated')) { this.updateConfig(); } if (e.affectsConfiguration('editor.accessibilitySupport')) { this.updateAccessibilitySupport(); } - }); + })); } public addDisposable(disposable: lifecycle.IDisposable): void { @@ -558,18 +558,21 @@ export class TerminalInstance implements ITerminalInstance { public dispose(): void { this._logService.trace(`terminalInstance#dispose (id: ${this.id})`); - if (this._windowsShellHelper) { - this._windowsShellHelper.dispose(); - } - if (this._linkHandler) { - this._linkHandler.dispose(); - } + this._windowsShellHelper = lifecycle.dispose(this._windowsShellHelper); + this._linkHandler = lifecycle.dispose(this._linkHandler); + this._commandTracker = lifecycle.dispose(this._commandTracker); + this._widgetManager = lifecycle.dispose(this._widgetManager); + if (this._xterm && this._xterm.element) { this._hadFocusOnExit = dom.hasClass(this._xterm.element, 'focus'); } if (this._wrapperElement) { + if ((this._wrapperElement).xterm) { + (this._wrapperElement).xterm = null; + } this._container.removeChild(this._wrapperElement); this._wrapperElement = null; + this._xtermElement = null; } if (this._xterm) { const buffer = (this._xterm.buffer); @@ -577,9 +580,7 @@ export class TerminalInstance implements ITerminalInstance { this._xterm.dispose(); this._xterm = null; } - if (this._processManager) { - this._processManager.dispose(); - } + this._processManager = lifecycle.dispose(this._processManager); if (!this._isDisposed) { this._isDisposed = true; this._onDisposed.fire(this); diff --git a/src/vs/workbench/parts/terminal/electron-browser/terminalLinkHandler.ts b/src/vs/workbench/parts/terminal/electron-browser/terminalLinkHandler.ts index 5fda5d268a3..7179711f900 100644 --- a/src/vs/workbench/parts/terminal/electron-browser/terminalLinkHandler.ts +++ b/src/vs/workbench/parts/terminal/electron-browser/terminalLinkHandler.ts @@ -121,6 +121,7 @@ export class TerminalLinkHandler { } public dispose(): void { + this._xterm = null; this._hoverDisposables = dispose(this._hoverDisposables); this._mouseMoveDisposable = dispose(this._mouseMoveDisposable); } diff --git a/src/vs/workbench/parts/terminal/node/terminalCommandTracker.ts b/src/vs/workbench/parts/terminal/node/terminalCommandTracker.ts index 2180347e7b0..bdde9f6e0b9 100644 --- a/src/vs/workbench/parts/terminal/node/terminalCommandTracker.ts +++ b/src/vs/workbench/parts/terminal/node/terminalCommandTracker.ts @@ -5,6 +5,7 @@ import { Terminal, IMarker } from 'vscode-xterm'; import { ITerminalCommandTracker } from 'vs/workbench/parts/terminal/common/terminal'; +import { IDisposable } from 'vs/base/common/lifecycle'; /** * The minimize size of the prompt in which to assume the line is a command. @@ -21,7 +22,7 @@ export enum ScrollPosition { Middle } -export class TerminalCommandTracker implements ITerminalCommandTracker { +export class TerminalCommandTracker implements ITerminalCommandTracker, IDisposable { private _currentMarker: IMarker | Boundary = Boundary.Bottom; private _selectionStart: IMarker | Boundary | null = null; private _isDisposable: boolean = false; @@ -32,6 +33,10 @@ export class TerminalCommandTracker implements ITerminalCommandTracker { this._xterm.on('key', key => this._onKey(key)); } + public dispose(): void { + this._xterm = null; + } + private _onKey(key: string): void { if (key === '\x0d') { this._onEnter(); -- GitLab