diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index c9736ff355fa22..cee5f81ec289fc 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -128,7 +128,7 @@ import { IUtilityProcessWorkerMainService, UtilityProcessWorkerMainService } fro import { ipcUtilityProcessWorkerChannelName } from '../../platform/utilityProcess/common/utilityProcessWorkerService.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,8 +1444,7 @@ export class CodeApplication extends Disposable { sharedProcessClient.then(client => client.registerChannel('profileStorageListener', profileStorageListener)); // Terminal - const ptyHostChannel = ProxyChannel.fromService(accessor.get(ILocalPtyService), disposables); - 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/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/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 5e714085db777b..23b5317e58c2cb 100644 --- a/src/vs/platform/terminal/test/node/ptyHostService.test.ts +++ b/src/vs/platform/terminal/test/node/ptyHostService.test.ts @@ -4,18 +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, 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. @@ -64,4 +70,55 @@ 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 `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: () => { } + }; + 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, ...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']); + }); });