Skip to content

Conversation

@SimonSiefke
Copy link
Contributor

@SimonSiefke SimonSiefke commented Jan 11, 2026

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:

pty-restart

After

No more leak is detected.

@vs-code-engineering
Copy link

📬 CODENOTIFY

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

@Tyriar

Matched files:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants