From ee16721c6919a6596270d988e1851489e59d58ba Mon Sep 17 00:00:00 2001 From: Daniel Garza <7416940+DanielGarzaB@users.noreply.github.com> Date: Fri, 14 Aug 2026 10:51:25 -0600 Subject: [PATCH 1/3] terminal: do not buffer pty host events that have no IPC client `ProxyChannel.fromService` eagerly creates an `Event.buffer` for every event of the service, and such a buffer only drains once a client calls `listen`. The window consumes every `IPtyService` event over a direct message port to the pty host (`TerminalIpcChannels.PtyHostWindow`), never over the localPty channel, so these buffers never drained: the main process archived every chunk of terminal output for the lifetime of the session. Reported at 1,150,631 events / 602 MB, ending in a V8 OOM that closes every window at once, with no warning and no reload prompt. Mark them unbuffered. The five `IPtyHostController` events stay buffered, those do have a client on this channel. Fixes #328885 Co-Authored-By: Claude Opus 5 (1M context) --- src/vs/code/electron-main/app.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index c9736ff355fa22..9b1a15ed8eefa9 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -126,7 +126,7 @@ import { ILoggerMainService } from '../../platform/log/electron-main/loggerServi import { IInitialProtocolUrls, IProtocolUrl } from '../../platform/url/electron-main/url.js'; import { IUtilityProcessWorkerMainService, UtilityProcessWorkerMainService } from '../../platform/utilityProcess/electron-main/utilityProcessWorkerMainService.js'; import { ipcUtilityProcessWorkerChannelName } from '../../platform/utilityProcess/common/utilityProcessWorkerService.js'; -import { ILocalPtyService, LocalReconnectConstants, TerminalIpcChannels, TerminalSettingId } from '../../platform/terminal/common/terminal.js'; +import { ILocalPtyService, IPtyService, LocalReconnectConstants, TerminalIpcChannels, TerminalSettingId } from '../../platform/terminal/common/terminal.js'; import { ElectronPtyHostStarter } from '../../platform/terminal/electron-main/electronPtyHostStarter.js'; import { PtyHostService } from '../../platform/terminal/node/ptyHostService.js'; import { ElectronAgentHostStarter } from '../../platform/agentHost/electron-main/electronAgentHostStarter.js'; @@ -1444,7 +1444,17 @@ export class CodeApplication extends Disposable { sharedProcessClient.then(client => client.registerChannel('profileStorageListener', profileStorageListener)); // Terminal - const ptyHostChannel = ProxyChannel.fromService(accessor.get(ILocalPtyService), disposables); + const ptyHostChannel = ProxyChannel.fromService(accessor.get(ILocalPtyService), disposables, { + // The window consumes every `IPtyService` event over a direct message port to the pty host + // (TerminalIpcChannels.PtyHostWindow), never over this channel, so buffering them here only + // retains: `onProcessData` carries raw terminal output and grows by hundreds of MBs an hour. + // The `IPtyHostController` events are left buffered, those do have a client on this channel. + // See https://github.com/microsoft/vscode/issues/328885 + unbufferedEvents: [ + 'onProcessData', 'onProcessReady', 'onProcessReplay', 'onProcessOrphanQuestion', + 'onDidRequestDetach', 'onDidChangeProperty', 'onProcessExit' + ] satisfies readonly (keyof IPtyService)[] + }); mainProcessElectronServer.registerChannel(TerminalIpcChannels.LocalPty, ptyHostChannel); // External Terminal From 4c365fda2d5e499b4b941b8b69ff2251a6e5dd07 Mon Sep 17 00:00:00 2001 From: Daniel Garza <7416940+DanielGarzaB@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:39:14 -0600 Subject: [PATCH 2/3] terminal: keep the list of unbuffered local pty events exhaustive The local pty channel opted the `IPtyService` events out of buffering by name, and nothing checked that list against the service: an event added to `IPtyService` later would have been buffered again in the main process for a client that never comes, which is exactly how #328885 came to be. Move the list next to the interface as `ptyServiceEvents`, derived from an object that `satisfies Record<..., true>` so that a missing or an extra name does not compile, and add a test that reflects over a `PtyHostService` instance the way `ProxyChannel.fromService` does, so an `on*` property the class gains outside the interface has to be classified too. Co-Authored-By: Claude Fable 5.1 --- src/vs/code/electron-main/app.ts | 7 ++--- src/vs/platform/terminal/common/terminal.ts | 19 ++++++++++++ .../terminal/test/node/ptyHostService.test.ts | 31 +++++++++++++++++++ 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index 9b1a15ed8eefa9..a9f84c4ae1d0eb 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -126,7 +126,7 @@ import { ILoggerMainService } from '../../platform/log/electron-main/loggerServi import { IInitialProtocolUrls, IProtocolUrl } from '../../platform/url/electron-main/url.js'; import { IUtilityProcessWorkerMainService, UtilityProcessWorkerMainService } from '../../platform/utilityProcess/electron-main/utilityProcessWorkerMainService.js'; import { ipcUtilityProcessWorkerChannelName } from '../../platform/utilityProcess/common/utilityProcessWorkerService.js'; -import { ILocalPtyService, IPtyService, LocalReconnectConstants, TerminalIpcChannels, TerminalSettingId } from '../../platform/terminal/common/terminal.js'; +import { ILocalPtyService, LocalReconnectConstants, ptyServiceEvents, TerminalIpcChannels, TerminalSettingId } from '../../platform/terminal/common/terminal.js'; import { ElectronPtyHostStarter } from '../../platform/terminal/electron-main/electronPtyHostStarter.js'; import { PtyHostService } from '../../platform/terminal/node/ptyHostService.js'; import { ElectronAgentHostStarter } from '../../platform/agentHost/electron-main/electronAgentHostStarter.js'; @@ -1450,10 +1450,7 @@ export class CodeApplication extends Disposable { // retains: `onProcessData` carries raw terminal output and grows by hundreds of MBs an hour. // The `IPtyHostController` events are left buffered, those do have a client on this channel. // See https://github.com/microsoft/vscode/issues/328885 - unbufferedEvents: [ - 'onProcessData', 'onProcessReady', 'onProcessReplay', 'onProcessOrphanQuestion', - 'onDidRequestDetach', 'onDidChangeProperty', 'onProcessExit' - ] satisfies readonly (keyof IPtyService)[] + unbufferedEvents: ptyServiceEvents }); mainProcessElectronServer.registerChannel(TerminalIpcChannels.LocalPty, ptyHostChannel); diff --git a/src/vs/platform/terminal/common/terminal.ts b/src/vs/platform/terminal/common/terminal.ts index cca3805245ee08..6c44c1a4e7995e 100644 --- a/src/vs/platform/terminal/common/terminal.ts +++ b/src/vs/platform/terminal/common/terminal.ts @@ -396,6 +396,25 @@ export interface IPtyService { } export const IPtyService = createDecorator('ptyService'); +/** + * The events of {@link IPtyService} by name, kept exhaustive by the type: adding an event to the interface + * without adding it here does not compile. + * + * The local pty channel of the main process needs this list at runtime. A window consumes these events over + * a direct message port to the pty host, never over that channel, so the channel must not buffer them for a + * client that never comes: `onProcessData` carries raw terminal output and grows without bound otherwise. + * See https://github.com/microsoft/vscode/issues/328885 + */ +export const ptyServiceEvents = Object.keys({ + onProcessData: true, + onProcessReady: true, + onProcessReplay: true, + onProcessOrphanQuestion: true, + onDidRequestDetach: true, + onDidChangeProperty: true, + onProcessExit: true, +} satisfies Record, true>) as readonly Extract[]; + export interface IPtyServiceContribution { handleProcessReady(persistentProcessId: number, process: ITerminalChildProcess): void; handleProcessDispose(persistentProcessId: number): void; diff --git a/src/vs/platform/terminal/test/node/ptyHostService.test.ts b/src/vs/platform/terminal/test/node/ptyHostService.test.ts index 5e714085db777b..cced6a0d52d9bf 100644 --- a/src/vs/platform/terminal/test/node/ptyHostService.test.ts +++ b/src/vs/platform/terminal/test/node/ptyHostService.test.ts @@ -10,6 +10,7 @@ import { IChannel, IChannelClient } from '../../../../base/parts/ipc/common/ipc. import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { TestConfigurationService } from '../../../configuration/test/common/testConfigurationService.js'; import { NullLogService, NullLoggerService } from '../../../log/common/log.js'; +import { IPtyHostController, ptyServiceEvents } from '../../common/terminal.js'; import { IPtyHostConnection, IPtyHostStarter } from '../../node/ptyHost.js'; import { PtyHostService } from '../../node/ptyHostService.js'; @@ -64,4 +65,34 @@ suite('PtyHostService', () => { 'listener counts should not grow across pty host restarts' ); }); + + test('every event of the service is accounted for on the local pty channel', () => { + // `ProxyChannel.fromService` buffers every `on*` property of the service it is handed until a client + // listens, and the local pty channel lists the `IPtyService` events as unbuffered because a window + // never listens to them there (see app.ts, #328885). `ptyServiceEvents` is exhaustive for the + // interface; this checks the class, which is what the channel reflects over: an event it gains that is + // in neither list would be buffered in the main process for a client that never comes. + const consumedOverLocalPtyChannel = [ + 'onPtyHostExit', 'onPtyHostStart', 'onPtyHostUnresponsive', 'onPtyHostResponsive', 'onPtyHostRequestResolveVariables' + ] satisfies readonly (keyof IPtyHostController)[]; + + const starter: IPtyHostStarter = { + start: (): IPtyHostConnection => { throw new Error('the pty host is not expected to start'); }, + dispose: () => { } + }; + const service = store.add(new PtyHostService( + starter, + new TestConfigurationService(), + new NullLogService(), + store.add(new NullLoggerService()) + )); + + const events: string[] = []; + for (const key in service) { + if (/^on[A-Z]/.test(key)) { + events.push(key); + } + } + deepStrictEqual(events.sort(), [...ptyServiceEvents, ...consumedOverLocalPtyChannel].sort()); + }); }); From 7d78a5a3b6a748ef230564910f0e618fa89883e9 Mon Sep 17 00:00:00 2001 From: Daniel Garza <7416940+DanielGarzaB@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:58:17 -0600 Subject: [PATCH 3/3] terminal: test the local pty channel itself, not only its list of events The guardian of the previous commit checks that every event of `PtyHostService` is classified, but nothing exercised the channel: dropping `unbufferedEvents` from the registration in app.ts kept every test green while the leak came back. Move the registration into `createLocalPtyChannel`, next to the service, so that a test can build the channel production builds and assert that none of the `IPtyService` events has a listener before a client asks for one, and that a client who does ask gets live events without a backlog. Co-Authored-By: Claude Fable 5.1 --- src/vs/code/electron-main/app.ts | 14 ++---- .../platform/terminal/node/ptyHostService.ts | 18 ++++++- .../terminal/test/node/ptyHostService.test.ts | 50 ++++++++++++++----- 3 files changed, 57 insertions(+), 25 deletions(-) diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index a9f84c4ae1d0eb..cee5f81ec289fc 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -126,9 +126,9 @@ import { ILoggerMainService } from '../../platform/log/electron-main/loggerServi import { IInitialProtocolUrls, IProtocolUrl } from '../../platform/url/electron-main/url.js'; import { IUtilityProcessWorkerMainService, UtilityProcessWorkerMainService } from '../../platform/utilityProcess/electron-main/utilityProcessWorkerMainService.js'; import { ipcUtilityProcessWorkerChannelName } from '../../platform/utilityProcess/common/utilityProcessWorkerService.js'; -import { ILocalPtyService, LocalReconnectConstants, ptyServiceEvents, TerminalIpcChannels, TerminalSettingId } from '../../platform/terminal/common/terminal.js'; +import { ILocalPtyService, LocalReconnectConstants, TerminalIpcChannels, TerminalSettingId } from '../../platform/terminal/common/terminal.js'; import { ElectronPtyHostStarter } from '../../platform/terminal/electron-main/electronPtyHostStarter.js'; -import { PtyHostService } from '../../platform/terminal/node/ptyHostService.js'; +import { createLocalPtyChannel, PtyHostService } from '../../platform/terminal/node/ptyHostService.js'; import { ElectronAgentHostStarter } from '../../platform/agentHost/electron-main/electronAgentHostStarter.js'; import { AgentHostProcessManager } from '../../platform/agentHost/node/agentHostService.js'; import { NODE_REMOTE_RESOURCE_CHANNEL_NAME, NODE_REMOTE_RESOURCE_IPC_METHOD_NAME, NodeRemoteResourceResponse, NodeRemoteResourceRouter } from '../../platform/remote/common/electronRemoteResources.js'; @@ -1444,15 +1444,7 @@ export class CodeApplication extends Disposable { sharedProcessClient.then(client => client.registerChannel('profileStorageListener', profileStorageListener)); // Terminal - const ptyHostChannel = ProxyChannel.fromService(accessor.get(ILocalPtyService), disposables, { - // The window consumes every `IPtyService` event over a direct message port to the pty host - // (TerminalIpcChannels.PtyHostWindow), never over this channel, so buffering them here only - // retains: `onProcessData` carries raw terminal output and grows by hundreds of MBs an hour. - // The `IPtyHostController` events are left buffered, those do have a client on this channel. - // See https://github.com/microsoft/vscode/issues/328885 - unbufferedEvents: ptyServiceEvents - }); - mainProcessElectronServer.registerChannel(TerminalIpcChannels.LocalPty, ptyHostChannel); + mainProcessElectronServer.registerChannel(TerminalIpcChannels.LocalPty, createLocalPtyChannel(accessor.get(ILocalPtyService), disposables)); // External Terminal const externalTerminalChannel = ProxyChannel.fromService(accessor.get(IExternalTerminalMainService), disposables); diff --git a/src/vs/platform/terminal/node/ptyHostService.ts b/src/vs/platform/terminal/node/ptyHostService.ts index 3ac9cdb8e9a149..f3ab777261474b 100644 --- a/src/vs/platform/terminal/node/ptyHostService.ts +++ b/src/vs/platform/terminal/node/ptyHostService.ts @@ -6,14 +6,14 @@ import { Emitter, Event } from '../../../base/common/event.js'; import { Disposable, DisposableStore, toDisposable } from '../../../base/common/lifecycle.js'; import { IProcessEnvironment, OS, OperatingSystem, isWindows } from '../../../base/common/platform.js'; -import { ProxyChannel } from '../../../base/parts/ipc/common/ipc.js'; +import { IServerChannel, ProxyChannel } from '../../../base/parts/ipc/common/ipc.js'; import { IConfigurationService } from '../../configuration/common/configuration.js'; import { ILogService, ILoggerService, LogLevel } from '../../log/common/log.js'; import { RemoteLoggerChannelClient } from '../../log/common/logIpc.js'; import { getResolvedShellEnv } from '../../shell/node/shellEnv.js'; import { IPtyHostProcessReplayEvent } from '../common/capabilities/capabilities.js'; import { RequestStore } from '../common/requestStore.js'; -import { HeartbeatConstants, IHeartbeatService, ITerminalLaunchResult, IProcessDataEvent, IProcessProperty, IProcessPropertyMap, IProcessReadyEvent, IPtyHostLatencyMeasurement, IPtyHostService, IPtyService, IRequestResolveVariablesEvent, ISerializedTerminalState, IShellLaunchConfig, ITerminalLaunchError, ITerminalProcessOptions, ITerminalProfile, ITerminalsLayoutInfo, ProcessPropertyType, TerminalIcon, TerminalIpcChannels, TerminalSettingId, TitleEventSource } from '../common/terminal.js'; +import { HeartbeatConstants, IHeartbeatService, ITerminalLaunchResult, IProcessDataEvent, IProcessProperty, IProcessPropertyMap, IProcessReadyEvent, IPtyHostLatencyMeasurement, IPtyHostService, IPtyService, IRequestResolveVariablesEvent, ISerializedTerminalState, IShellLaunchConfig, ITerminalLaunchError, ITerminalProcessOptions, ITerminalProfile, ITerminalsLayoutInfo, ProcessPropertyType, ptyServiceEvents, TerminalIcon, TerminalIpcChannels, TerminalSettingId, TitleEventSource } from '../common/terminal.js'; import { registerTerminalPlatformConfiguration } from '../common/terminalPlatformConfiguration.js'; import { IGetTerminalLayoutInfoArgs, IProcessDetails, ISetTerminalLayoutInfoArgs } from '../common/terminalProcess.js'; import { IPtyHostConnection, IPtyHostStarter } from './ptyHost.js'; @@ -430,3 +430,17 @@ export class PtyHostService extends Disposable implements IPtyHostService { this._resolveVariablesRequestStore.acceptReply(requestId, resolved); } } + +/** + * The channel a window reaches the {@link PtyHostService} of the main process over, + * {@link TerminalIpcChannels.LocalPty}. + * + * The window consumes every `IPtyService` event over a direct message port to the pty host + * ({@link TerminalIpcChannels.PtyHostWindow}), never over this channel, so buffering them here for a client + * that never comes only retains: `onProcessData` carries raw terminal output and grows by hundreds of MBs an + * hour. The `IPtyHostController` events are left buffered, those do have a client on this channel. + * See https://github.com/microsoft/vscode/issues/328885 + */ +export function createLocalPtyChannel(service: IPtyHostService, disposables: DisposableStore): IServerChannel { + return ProxyChannel.fromService(service, disposables, { unbufferedEvents: ptyServiceEvents }); +} diff --git a/src/vs/platform/terminal/test/node/ptyHostService.test.ts b/src/vs/platform/terminal/test/node/ptyHostService.test.ts index cced6a0d52d9bf..23b5317e58c2cb 100644 --- a/src/vs/platform/terminal/test/node/ptyHostService.test.ts +++ b/src/vs/platform/terminal/test/node/ptyHostService.test.ts @@ -4,19 +4,24 @@ *--------------------------------------------------------------------------------------------*/ import { deepStrictEqual } from 'assert'; -import { Event } from '../../../../base/common/event.js'; +import { Emitter, Event } from '../../../../base/common/event.js'; import { DisposableStore, IDisposable } from '../../../../base/common/lifecycle.js'; import { IChannel, IChannelClient } from '../../../../base/parts/ipc/common/ipc.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { TestConfigurationService } from '../../../configuration/test/common/testConfigurationService.js'; import { NullLogService, NullLoggerService } from '../../../log/common/log.js'; -import { IPtyHostController, ptyServiceEvents } from '../../common/terminal.js'; +import { IPtyHostController, IPtyHostService, ptyServiceEvents } from '../../common/terminal.js'; import { IPtyHostConnection, IPtyHostStarter } from '../../node/ptyHost.js'; -import { PtyHostService } from '../../node/ptyHostService.js'; +import { createLocalPtyChannel, PtyHostService } from '../../node/ptyHostService.js'; suite('PtyHostService', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); + // the events a window does listen to over the local pty channel, unlike those of `IPtyService` + const ptyHostControllerEvents = [ + 'onPtyHostExit', 'onPtyHostStart', 'onPtyHostUnresponsive', 'onPtyHostResponsive', 'onPtyHostRequestResolveVariables' + ] satisfies readonly (keyof IPtyHostController)[]; + test('restartPtyHost disposes listeners registered during pty host startup', async () => { // Track active listener counts per event across pty host restarts. Without the // fix, each restart would leak the listeners registered in _startPtyHost. @@ -68,14 +73,10 @@ suite('PtyHostService', () => { test('every event of the service is accounted for on the local pty channel', () => { // `ProxyChannel.fromService` buffers every `on*` property of the service it is handed until a client - // listens, and the local pty channel lists the `IPtyService` events as unbuffered because a window - // never listens to them there (see app.ts, #328885). `ptyServiceEvents` is exhaustive for the - // interface; this checks the class, which is what the channel reflects over: an event it gains that is - // in neither list would be buffered in the main process for a client that never comes. - const consumedOverLocalPtyChannel = [ - 'onPtyHostExit', 'onPtyHostStart', 'onPtyHostUnresponsive', 'onPtyHostResponsive', 'onPtyHostRequestResolveVariables' - ] satisfies readonly (keyof IPtyHostController)[]; - + // listens, and `createLocalPtyChannel` lists the `IPtyService` events as unbuffered because a window + // never listens to them there (#328885). `ptyServiceEvents` is exhaustive for the interface; this + // checks the class, which is what the channel reflects over: an event it gains that is in neither + // list would be buffered in the main process for a client that never comes. const starter: IPtyHostStarter = { start: (): IPtyHostConnection => { throw new Error('the pty host is not expected to start'); }, dispose: () => { } @@ -93,6 +94,31 @@ suite('PtyHostService', () => { events.push(key); } } - deepStrictEqual(events.sort(), [...ptyServiceEvents, ...consumedOverLocalPtyChannel].sort()); + deepStrictEqual(events.sort(), [...ptyServiceEvents, ...ptyHostControllerEvents].sort()); + }); + + test('the local pty channel does not buffer the events a window never listens to there', () => { + // a buffered event has a listener on the service from the moment the channel is created, before any + // client asked for it: that listener is what retained every chunk of terminal output in the main + // process (#328885), so the events a window never asks for over this channel must not have one + const emitters = new Map>(); + const service: { [event: string]: Event } = {}; + for (const name of [...ptyServiceEvents, ...ptyHostControllerEvents]) { + const emitter = store.add(new Emitter()); + emitters.set(name, emitter); + service[name] = emitter.event; + } + const channelDisposables = store.add(new DisposableStore()); + const channel = createLocalPtyChannel(service as unknown as IPtyHostService, channelDisposables); + + const subscribedEagerly = [...emitters].filter(([, emitter]) => emitter.hasListeners()).map(([name]) => name); + deepStrictEqual(subscribedEagerly.sort(), [...ptyHostControllerEvents].sort()); + + // a client that does ask for one of them gets it live, without a backlog + emitters.get('onProcessData')!.fire('before'); + const received: unknown[] = []; + channelDisposables.add(channel.listen(undefined, 'onProcessData')(e => received.push(e))); + emitters.get('onProcessData')!.fire('after'); + deepStrictEqual(received, ['after']); }); });