fix: memory leak in pty host service

[SimonSiefke]

Fixes a memory leak in pty host service.

Details

When starting pty host, various events get registered using this._register:

private _startPtyHost(): [IPtyHostConnection, IPtyService] {
  this._register(proxy.onProcessData(e => this._onProcessData.fire(e)));
  this._register(proxy.onProcessReady(e => this._onProcessReady.fire(e)));
  this._register(proxy.onProcessExit(e => this._onProcessExit.fire(e)));
  this._register(proxy.onDidChangeProperty(e => this._onDidChangeProperty.fire(e)));
  this._register(proxy.onProcessReplay(e => this._onProcessReplay.fire(e)));
}

But since there is only one instance of ptyHostService, each time when restarting the pty host, new event listeners get registered, and the number of registered disposables in ptyHostService seems to grow each time.

Change

The change uses a MutableDisposable so that when the pty host is restarted, the previous event listener disposables are disposed.

private readonly ptyHostStore = this._register(new MutableDisposable());

private _startPtyHost(): [IPtyHostConnection, IPtyService, DisposableStore] {
  const store = new DisposableStore();
  store.add(connection.store);

  store.add(proxy.onProcessData(e => this._onProcessData.fire(e)));
  store.add(proxy.onProcessReady(e => this._onProcessReady.fire(e)));
  store.add(proxy.onProcessExit(e => this._onProcessExit.fire(e)));
  store.add(proxy.onDidChangeProperty(e => this._onDidChangeProperty.fire(e)));
  store.add(proxy.onProcessReplay(e => this._onProcessReplay.fire(e)));
  store.add(proxy.onProcessOrphanQuestion(e => this._onProcessOrphanQuestion.fire(e)));
  store.add(proxy.onDidRequestDetach(e => this._onDidRequestDetach.fire(e)));

  /*...*/
  return [connection, proxy, store];
}

async restartPtyHost(): Promise<void> {
	this._disposePtyHost();
	this._isResponsive = true;
	const [_, _2, store] = this._startPtyHost();
	this.ptyHostStore.value = store;
}

Before

When restarting the pty host 37 times, the number of various functions seems to grow by one each time:

After

No more leak is detected.

[vs-code-engineering]

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@anthonykim1

Matched files:

• src/vs/platform/terminal/node/ptyHostService.ts

[Copilot]

Pull request overview

Fixes a memory leak in the terminal pty host controller by ensuring event/listener disposables created during pty host startup are cleaned up when the pty host is restarted.

Changes:

• Introduces a per-pty-host DisposableStore returned from _startPtyHost and tracked via a MutableDisposable.
• Moves pty-host-scoped event registrations (connection exit, proxy events, logger client, config listener) from this._register to the per-pty-host store.
• Updates _ensurePtyHost, _startPtyHost, restartPtyHost, and _disposePtyHost to wire disposal through ptyHostStore.

---

You can also share your feedback on Copilot code review. Take the survey.

[dmitrivMS]

@SimonSiefke Thank you!

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 3

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 0 new