Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/vs/code/electron-main/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
19 changes: 19 additions & 0 deletions src/vs/platform/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,25 @@ export interface IPtyService {
}
export const IPtyService = createDecorator<IPtyService>('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
*/
Comment on lines +399 to +407
export const ptyServiceEvents = Object.keys({
onProcessData: true,
onProcessReady: true,
onProcessReplay: true,
onProcessOrphanQuestion: true,
onDidRequestDetach: true,
onDidChangeProperty: true,
onProcessExit: true,
} satisfies Record<Extract<keyof IPtyService, `on${string}`>, true>) as readonly Extract<keyof IPtyService, `on${string}`>[];

export interface IPtyServiceContribution {
handleProcessReady(persistentProcessId: number, process: ITerminalChildProcess): void;
handleProcessDispose(persistentProcessId: number): void;
Expand Down
18 changes: 16 additions & 2 deletions src/vs/platform/terminal/node/ptyHostService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
*/
Comment on lines +434 to +443
export function createLocalPtyChannel<TContext>(service: IPtyHostService, disposables: DisposableStore): IServerChannel<TContext> {
return ProxyChannel.fromService<TContext>(service, disposables, { unbufferedEvents: ptyServiceEvents });
}
61 changes: 59 additions & 2 deletions src/vs/platform/terminal/test/node/ptyHostService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Comment on lines +75 to +79
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<string, Emitter<unknown>>();
const service: { [event: string]: Event<unknown> } = {};
for (const name of [...ptyServiceEvents, ...ptyHostControllerEvents]) {
const emitter = store.add(new Emitter<unknown>());
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']);
});
});