Merge pull request #229981 from microsoft/tyriar/191768

Fix memory leak issues in terminal

src/vs/workbench/contrib/terminal/browser/terminal.ts

@@ -7,7 +7,7 @@ import { IDimension } from '../../../../base/browser/dom.js';
 import { Orientation } from '../../../../base/browser/ui/splitview/splitview.js';
 import { Color } from '../../../../base/common/color.js';
 import { Event, IDynamicListEventMultiplexer, type DynamicListEventMultiplexer } from '../../../../base/common/event.js';
-import { DisposableStore, IDisposable } from '../../../../base/common/lifecycle.js';
+import { DisposableStore, IDisposable, type IReference } from '../../../../base/common/lifecycle.js';
 import { OperatingSystem } from '../../../../base/common/platform.js';
 import { URI } from '../../../../base/common/uri.js';
 import { createDecorator } from '../../../../platform/instantiation/common/instantiation.js';
@@ -628,7 +628,8 @@ export interface ITerminalInstance extends IBaseTerminalInstance {
  /**
  * The position of the terminal.
  */
- target?: TerminalLocation;
+ target: TerminalLocation | undefined;
+ targetRef: IReference<TerminalLocation | undefined>;
 
  /**
  * The id of a persistent process. This is defined if this is a terminal created by a pty host

src/vs/workbench/contrib/terminal/browser/terminalEvents.ts

@@ -17,21 +17,31 @@ export function createInstanceCapabilityEventMultiplexer<T extends TerminalCapab
 ): IDynamicListEventMultiplexer<{ instance: ITerminalInstance; data: K }> {
  const store = new DisposableStore();
  const multiplexer = store.add(new EventMultiplexer<{ instance: ITerminalInstance; data: K }>());
- const capabilityListeners = store.add(new DisposableMap<ITerminalCapabilityImplMap[T], IDisposable>());
+ const capabilityListeners = store.add(new DisposableMap<number, DisposableMap<ITerminalCapabilityImplMap[T], IDisposable>>());
 
  function addCapability(instance: ITerminalInstance, capability: ITerminalCapabilityImplMap[T]) {
  const listener = multiplexer.add(Event.map(getEvent(capability), data => ({ instance, data })));
- capabilityListeners.set(capability, listener);
+ let instanceCapabilityListeners = capabilityListeners.get(instance.instanceId);
+ if (!instanceCapabilityListeners) {
+ instanceCapabilityListeners = new DisposableMap<ITerminalCapabilityImplMap[T], IDisposable>();
+ capabilityListeners.set(instance.instanceId, instanceCapabilityListeners);
+ }
+ instanceCapabilityListeners.set(capability, listener);
  }
 
- // Existing capabilities
+ // Existing instances
  for (const instance of currentInstances) {
  const capability = instance.capabilities.get(capabilityId);
  if (capability) {
  addCapability(instance, capability);
  }
  }
 
+ // Removed instances
+ store.add(onRemoveInstance(instance => {
+ capabilityListeners.deleteAndDispose(instance.instanceId);
+ }));
+
  // Added capabilities
  const addCapabilityMultiplexer = store.add(new DynamicListEventMultiplexer(
  currentInstances,
@@ -50,11 +60,11 @@ export function createInstanceCapabilityEventMultiplexer<T extends TerminalCapab
  currentInstances,
  onAddInstance,
  onRemoveInstance,
- instance => instance.capabilities.onDidRemoveCapability
+ instance => Event.map(instance.capabilities.onDidRemoveCapability, changeEvent => ({ instance, changeEvent }))
  ));
  store.add(removeCapabilityMultiplexer.event(e => {
- if (e.id === capabilityId) {
- capabilityListeners.deleteAndDispose(e.capability);
+ if (e.changeEvent.id === capabilityId) {
+ capabilityListeners.get(e.instance.instanceId)?.deleteAndDispose(e.changeEvent.id);
  }
  }));
 

src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

@@ -17,7 +17,7 @@ import { ErrorNoTelemetry, onUnexpectedError } from '../../../../base/common/err
 import { Emitter, Event } from '../../../../base/common/event.js';
 import { KeyCode } from '../../../../base/common/keyCodes.js';
 import { ISeparator, template } from '../../../../base/common/labels.js';
-import { Disposable, DisposableStore, IDisposable, MutableDisposable, dispose, toDisposable } from '../../../../base/common/lifecycle.js';
+import { Disposable, DisposableStore, IDisposable, ImmortalReference, MutableDisposable, dispose, toDisposable, type IReference } from '../../../../base/common/lifecycle.js';
 import { Schemas } from '../../../../base/common/network.js';
 import * as path from '../../../../base/common/path.js';
 import { OS, OperatingSystem, isMacintosh, isWindows } from '../../../../base/common/platform.js';
@@ -188,7 +188,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
  private _labelComputer?: TerminalLabelComputer;
  private _userHome?: string;
  private _hasScrollBar?: boolean;
- private _target?: TerminalLocation | undefined;
  private _usedShellIntegrationInjection: boolean = false;
  get usedShellIntegrationInjection(): boolean { return this._usedShellIntegrationInjection; }
  private _lineDataEventAddon: LineDataEventAddon | undefined;
@@ -216,9 +215,12 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
  this._shellLaunchConfig.waitOnExit = value;
  }
 
- get target(): TerminalLocation | undefined { return this._target; }
+ private _targetRef: ImmortalReference<TerminalLocation | undefined> = new ImmortalReference(undefined);
+ get targetRef(): IReference<TerminalLocation | undefined> { return this._targetRef; }
+
+ get target(): TerminalLocation | undefined { return this._targetRef.object; }
  set target(value: TerminalLocation | undefined) {
- this._target = value;
+ this._targetRef.object = value;
  this._onDidChangeTarget.fire(value);
  }
 
@@ -317,10 +319,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
  readonly onData = this._onData.event;
  private readonly _onBinary = this._register(new Emitter<string>());
  readonly onBinary = this._onBinary.event;
- private readonly _onLineData = this._register(new Emitter<string>({
- onDidAddFirstListener: () => this._onLineDataSetup()
- }));
- readonly onLineData = this._onLineData.event;
  private readonly _onRequestExtHostProcess = this._register(new Emitter<ITerminalInstance>());
  readonly onRequestExtHostProcess = this._onRequestExtHostProcess.event;
  private readonly _onDimensionsChanged = this._register(new Emitter<void>());
@@ -357,6 +355,11 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
  private readonly _onDidPaste = this._register(new Emitter<string>());
  readonly onDidPaste = this._onDidPaste.event;
 
+ private readonly _onLineData = this._register(new Emitter<string>({
+ onDidAddFirstListener: async () => (this.xterm ?? await this._xtermReadyPromise).raw.loadAddon(this._lineDataEventAddon!)
+ }));
+ readonly onLineData = this._onLineData.event;
+
  constructor(
  private readonly _terminalShellTypeContextKey: IContextKey<string>,
  private readonly _terminalInRunCommandPicker: IContextKey<boolean>,
@@ -753,7 +756,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
  Terminal,
  this._cols,
  this._rows,
- this._scopedInstantiationService.createInstance(TerminalInstanceColorProvider, this),
+ this._scopedInstantiationService.createInstance(TerminalInstanceColorProvider, this._targetRef),
  this.capabilities,
  this._processManager.shellIntegrationNonce,
  disableShellIntegrationReporting
@@ -775,6 +778,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
  await this._updatePtyDimensions(xterm.raw);
  }
  ));
+ this._register(toDisposable(() => this._resizeDebouncer = undefined));
  this.updateAccessibilitySupport();
  this._register(this.xterm.onDidRequestRunCommand(e => {
  if (e.copyAsHtml) {
@@ -863,11 +867,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
  return xterm;
  }
 
- private async _onLineDataSetup(): Promise<void> {
- const xterm = this.xterm || await this._xtermReadyPromise;
- xterm.raw.loadAddon(this._lineDataEventAddon!);
- }
-
  async runCommand(commandLine: string, shouldExecute: boolean): Promise<void> {
  let commandDetection = this.capabilities.get(TerminalCapability.CommandDetection);
 
@@ -2676,7 +2675,7 @@ export function parseExitResult(
 
 export class TerminalInstanceColorProvider implements IXtermColorProvider {
  constructor(
- private readonly _instance: ITerminalInstance,
+ private readonly _target: IReference<TerminalLocation | undefined>,
  @IViewDescriptorService private readonly _viewDescriptorService: IViewDescriptorService,
  ) {
  }
@@ -2686,7 +2685,7 @@ export class TerminalInstanceColorProvider implements IXtermColorProvider {
  if (terminalBackground) {
  return terminalBackground;
  }
- if (this._instance.target === TerminalLocation.Editor) {
+ if (this._target.object === TerminalLocation.Editor) {
  return theme.getColor(editorBackground);
  }
  const location = this._viewDescriptorService.getViewLocationById(TERMINAL_VIEW_ID)!;

src/vs/workbench/contrib/terminalContrib/commandGuide/browser/terminal.commandGuide.contribution.ts

@@ -42,11 +42,11 @@ class TerminalCommandGuideContribution extends Disposable implements ITerminalCo
  xtermOpen(xterm: IXtermTerminal & { raw: RawXtermTerminal }): void {
  this._xterm = xterm;
  this._refreshActivatedState();
- this._configurationService.onDidChangeConfiguration(e => {
+ this._register(this._configurationService.onDidChangeConfiguration(e => {
  if (e.affectsConfiguration(TerminalCommandGuideSettingId.ShowCommandGuide)) {
  this._refreshActivatedState();
  }
- });
+ }));
  }
 
  private _refreshActivatedState() {

src/vs/workbench/contrib/terminalContrib/developer/browser/terminal.developer.contribution.ts

@@ -7,7 +7,7 @@ import type { Terminal } from '@xterm/xterm';
 import { Delayer } from '../../../../../base/common/async.js';
 import { VSBuffer } from '../../../../../base/common/buffer.js';
 import { Event } from '../../../../../base/common/event.js';
-import { Disposable, DisposableStore, IDisposable, MutableDisposable, combinedDisposable, dispose } from '../../../../../base/common/lifecycle.js';
+import { Disposable, DisposableMap, DisposableStore, IDisposable, MutableDisposable, combinedDisposable, dispose } from '../../../../../base/common/lifecycle.js';
 import { URI } from '../../../../../base/common/uri.js';
 import './media/developer.css';
 import { localize, localize2 } from '../../../../../nls.js';
@@ -256,8 +256,11 @@ class DevModeContribution extends Disposable implements ITerminalContribution {
  const commandDetection = this._instance.capabilities.get(TerminalCapability.CommandDetection);
  if (devMode) {
  if (commandDetection) {
- const commandDecorations = new Map<ITerminalCommand, IDisposable[]>();
+ const commandDecorations = new DisposableMap<ITerminalCommand, IDisposable>();
+ const otherDisposables = new DisposableStore();
  this._activeDevModeDisposables.value = combinedDisposable(
+ commandDecorations,
+ otherDisposables,
  // Prompt input
  this._instance.onDidBlur(() => this._updateDevMode()),
  this._instance.onDidFocus(() => this._updateDevMode()),
@@ -266,17 +269,17 @@ class DevModeContribution extends Disposable implements ITerminalContribution {
  commandDetection.onCommandFinished(command => {
  const colorClass = `color-${this._currentColor}`;
  const decorations: IDisposable[] = [];
- commandDecorations.set(command, decorations);
+ commandDecorations.set(command, combinedDisposable(...decorations));
  if (command.promptStartMarker) {
  const d = this._instance.xterm!.raw?.registerDecoration({
  marker: command.promptStartMarker
  });
  if (d) {
  decorations.push(d);
- d.onRender(e => {
+ otherDisposables.add(d.onRender(e => {
  e.textContent = 'A';
  e.classList.add('xterm-sequence-decoration', 'top', 'left', colorClass);
- });
+ }));
  }
  }
  if (command.marker) {
@@ -286,10 +289,10 @@ class DevModeContribution extends Disposable implements ITerminalContribution {
  });
  if (d) {
  decorations.push(d);
- d.onRender(e => {
+ otherDisposables.add(d.onRender(e => {
  e.textContent = 'B';
  e.classList.add('xterm-sequence-decoration', 'top', 'right', colorClass);
- });
+ }));
  }
  }
  if (command.executedMarker) {
@@ -299,10 +302,10 @@ class DevModeContribution extends Disposable implements ITerminalContribution {
  });
  if (d) {
  decorations.push(d);
- d.onRender(e => {
+ otherDisposables.add(d.onRender(e => {
  e.textContent = 'C';
  e.classList.add('xterm-sequence-decoration', 'bottom', 'left', colorClass);
- });
+ }));
  }
  }
  if (command.endMarker) {
@@ -311,10 +314,10 @@ class DevModeContribution extends Disposable implements ITerminalContribution {
  });
  if (d) {
  decorations.push(d);
- d.onRender(e => {
+ otherDisposables.add(d.onRender(e => {
  e.textContent = 'D';
  e.classList.add('xterm-sequence-decoration', 'bottom', 'right', colorClass);
- });
+ }));
  }
  }
  this._currentColor = (this._currentColor + 1) % 2;
@@ -325,7 +328,7 @@ class DevModeContribution extends Disposable implements ITerminalContribution {
  if (decorations) {
  dispose(decorations);
  }
- commandDecorations.delete(c);
+ commandDecorations.deleteAndDispose(c);
  }
  })
  );

src/vs/workbench/contrib/terminalContrib/stickyScroll/browser/terminalStickyScrollContribution.ts

@@ -103,7 +103,7 @@ export class TerminalStickyScrollContribution extends Disposable implements ITer
  TerminalStickyScrollOverlay,
  this._instance,
  this._xterm!,
- this._instantiationService.createInstance(TerminalInstanceColorProvider, this._instance),
+ this._instantiationService.createInstance(TerminalInstanceColorProvider, this._instance.targetRef),
  this._instance.capabilities.get(TerminalCapability.CommandDetection)!,
  xtermCtorEventually
  );