Skip to content

Commit b386bae

Browse files
committed
Use a disposable store
1 parent 3876a21 commit b386bae

File tree

5 files changed

+28
-19
lines changed

5 files changed

+28
-19
lines changed

src/vs/platform/terminal/electron-main/electronPtyHostStarter.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import { IPtyHostConnection, IPtyHostStarter } from 'vs/platform/terminal/node/p
1313
import { UtilityProcess } from 'vs/platform/utilityProcess/electron-main/utilityProcess';
1414
import { Client as MessagePortClient } from 'vs/base/parts/ipc/electron-main/ipc.mp';
1515
import { IpcMainEvent } from 'electron';
16-
import { assertIsDefined } from 'vs/base/common/types';
1716
import { validatedIpcMain } from 'vs/base/parts/ipc/electron-main/ipcMain';
17+
import { DisposableStore, toDisposable } from 'vs/base/common/lifecycle';
1818

1919
export class ElectronPtyHostStarter implements IPtyHostStarter {
2020

@@ -55,11 +55,19 @@ export class ElectronPtyHostStarter implements IPtyHostStarter {
5555
// Listen for new windows to establish connection directly to pty host
5656
validatedIpcMain.on('vscode:createPtyHostMessageChannel', (e, nonce) => this._onWindowConnection(e, nonce));
5757

58+
// TODO: Do we need to listen for window close to close the port?
59+
60+
const store = new DisposableStore();
61+
store.add(client);
62+
store.add(this.utilityProcess);
63+
store.add(toDisposable(() => {
64+
validatedIpcMain.removeHandler('vscode:createPtyHostMessageChannel');
65+
this.utilityProcess = undefined;
66+
}));
67+
5868
return {
5969
client,
60-
port,
61-
connect: () => assertIsDefined(this.utilityProcess).connect(),
62-
dispose: client.dispose,
70+
store,
6371
onDidProcessExit: this.utilityProcess.onExit
6472
};
6573
}

src/vs/platform/terminal/node/nodePtyHostStarter.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { DisposableStore } from 'vs/base/common/lifecycle';
67
import { FileAccess } from 'vs/base/common/network';
78
import { Client, IIPCOptions } from 'vs/base/parts/ipc/node/ipc.cp';
89
import { IEnvironmentService, INativeEnvironmentService } from 'vs/platform/environment/common/environment';
@@ -43,9 +44,12 @@ export class NodePtyHostStarter implements IPtyHostStarter {
4344

4445
const client = new Client(FileAccess.asFileUri('bootstrap-fork').fsPath, opts);
4546

47+
const store = new DisposableStore();
48+
store.add(client);
49+
4650
return {
4751
client,
48-
dispose: client.dispose,
52+
store,
4953
onDidProcessExit: client.onDidProcessExit
5054
};
5155
}

src/vs/platform/terminal/node/ptyHost.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,12 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Event } from 'vs/base/common/event';
7-
import { IDisposable } from 'vs/base/common/lifecycle';
7+
import { DisposableStore } from 'vs/base/common/lifecycle';
88
import { IChannelClient } from 'vs/base/parts/ipc/common/ipc';
99

10-
export interface IPtyHostConnection extends IDisposable {
10+
export interface IPtyHostConnection {
1111
readonly client: IChannelClient;
12-
// TODO: Type
13-
readonly port?: any;
14-
connect?(): any;
12+
readonly store: DisposableStore;
1513
readonly onDidProcessExit: Event<{ code: number; signal: string }>;
1614
}
1715

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ export class PtyHostService extends Disposable implements IPtyService {
320320

321321
private _disposePtyHost(): void {
322322
this._proxy.shutdownAll?.();
323-
this._connection.dispose();
323+
this._connection.store.dispose();
324324
}
325325

326326
private _handleHeartbeat() {

src/vs/workbench/contrib/terminal/electron-sandbox/localTerminalBackend.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { INativeWorkbenchEnvironmentService } from 'vs/workbench/services/enviro
3434
import { Client as MessagePortClient } from 'vs/base/parts/ipc/common/ipc.mp';
3535
import { acquirePort } from 'vs/base/parts/ipc/electron-sandbox/ipc.mp';
3636
import { ProxyChannel } from 'vs/base/parts/ipc/common/ipc';
37+
import { mark } from 'vs/base/common/performance';
3738

3839
export class LocalTerminalBackendContribution implements IWorkbenchContribution {
3940
constructor(
@@ -74,26 +75,24 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
7475
) {
7576
super(_localPtyService, logService, notificationService, historyService, _configurationResolverService, workspaceContextService);
7677

77-
78-
79-
78+
// TODO: Use the direct connection
8079
(async () => {
81-
// mark('code/willConnectSharedProcess');
82-
// this.logService.trace('Renderer->SharedProcess#connect: before acquirePort');
80+
mark('code/willConnectPtyHost');
81+
logService.trace('Renderer->PtyHost#connect: before acquirePort');
8382
const port = await acquirePort('vscode:createPtyHostMessageChannel', 'vscode:createPtyHostMessageChannelResult');
84-
// mark('code/didConnectSharedProcess');
85-
// this.logService.trace('Renderer->SharedProcess#connect: connection established');
83+
mark('code/didConnectPtyHost');
84+
logService.trace('Renderer->PtyHost#connect: connection established');
8685

8786
const client = new MessagePortClient(port, `window:${environmentService.window.id}`);
8887
const proxy = ProxyChannel.toService<IPtyService>(client.getChannel(TerminalIpcChannels.PtyHostWindow));
8988

89+
// Testing
9090
logService.info('latency: ', proxy.getLatency(0));
9191
proxy.onProcessData(e => {
9292
logService.info('message port process data: ' + e.event);
9393
});
9494
})();
9595

96-
9796
// Attach process listeners
9897
this._localPtyService.onProcessData(e => this._ptys.get(e.id)?.handleData(e.event));
9998
this._localPtyService.onDidChangeProperty(e => this._ptys.get(e.id)?.handleDidChangeProperty(e.property));

0 commit comments

Comments
 (0)