Skip to content

Commit 99f2863

Browse files
authored
fix: wire ext host restart through utility process api (microsoft#302633)
1 parent bc015ef commit 99f2863

5 files changed

Lines changed: 141 additions & 22 deletions

File tree

src/vs/platform/extensions/common/extensionHostStarter.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { createDecorator } from '../../instantiation/common/instantiation.js';
99
export const IExtensionHostStarter = createDecorator<IExtensionHostStarter>('extensionHostStarter');
1010

1111
export const ipcExtensionHostStarterChannelName = 'extensionHostStarter';
12+
export const extensionHostGraceTimeMs = 6000;
1213

1314
export interface IExtensionHostProcessOptions {
1415
responseWindowId: number;
@@ -31,6 +32,7 @@ export interface IExtensionHostStarter {
3132
createExtensionHost(): Promise<{ id: string }>;
3233
start(id: string, opts: IExtensionHostProcessOptions): Promise<{ pid: number | undefined }>;
3334
enableInspectPort(id: string): Promise<boolean>;
35+
waitForExit(id: string, maxWaitTimeMs: number): Promise<void>;
3436
kill(id: string): Promise<void>;
3537

3638
}

src/vs/platform/extensions/electron-main/extensionHostStarter.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { Promises } from '../../../base/common/async.js';
77
import { canceled } from '../../../base/common/errors.js';
88
import { Event } from '../../../base/common/event.js';
99
import { Disposable, IDisposable } from '../../../base/common/lifecycle.js';
10-
import { IExtensionHostProcessOptions, IExtensionHostStarter } from '../common/extensionHostStarter.js';
10+
import { extensionHostGraceTimeMs, IExtensionHostProcessOptions, IExtensionHostStarter } from '../common/extensionHostStarter.js';
1111
import { ILifecycleMainService } from '../../lifecycle/electron-main/lifecycleMainService.js';
1212
import { ILogService } from '../../log/common/log.js';
1313
import { ITelemetryService } from '../../telemetry/common/telemetry.js';
@@ -121,7 +121,7 @@ export class ExtensionHostStarter extends Disposable implements IDisposable, IEx
121121
allowLoadingUnsignedLibraries: true,
122122
respondToAuthRequestsFromMainProcess: true,
123123
windowLifecycleBound: true,
124-
windowLifecycleGraceTime: 6000,
124+
windowLifecycleGraceTime: extensionHostGraceTimeMs,
125125
correlationId: id
126126
});
127127
const pid = await Event.toPromise(extHost.onSpawn);
@@ -151,6 +151,17 @@ export class ExtensionHostStarter extends Disposable implements IDisposable, IEx
151151
extHostProcess.kill();
152152
}
153153

154+
async waitForExit(id: string, maxWaitTimeMs: number): Promise<void> {
155+
if (this._shutdown) {
156+
throw canceled();
157+
}
158+
const extHostProcess = this._extHosts.get(id);
159+
if (!extHostProcess) {
160+
return;
161+
}
162+
await extHostProcess.waitForExit(maxWaitTimeMs);
163+
}
164+
154165
async _killAllNow(): Promise<void> {
155166
for (const [, extHost] of this._extHosts) {
156167
extHost.kill();

src/vs/workbench/services/extensions/electron-browser/localProcessExtensionHost.ts

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { BufferedEmitter } from '../../../../base/parts/ipc/common/ipc.net.js';
1919
import { acquirePort } from '../../../../base/parts/ipc/electron-browser/ipc.mp.js';
2020
import * as nls from '../../../../nls.js';
2121
import { IExtensionHostDebugService } from '../../../../platform/debug/common/extensionHostDebug.js';
22-
import { IExtensionHostProcessOptions, IExtensionHostStarter } from '../../../../platform/extensions/common/extensionHostStarter.js';
22+
import { extensionHostGraceTimeMs, IExtensionHostProcessOptions, IExtensionHostStarter } from '../../../../platform/extensions/common/extensionHostStarter.js';
2323
import { ILabelService } from '../../../../platform/label/common/label.js';
2424
import { ILogService, ILoggerService } from '../../../../platform/log/common/log.js';
2525
import { INativeHostService } from '../../../../platform/native/common/native.js';
@@ -32,7 +32,7 @@ import { IWorkspaceContextService, WorkbenchState, isUntitledWorkspace } from '.
3232
import { INativeWorkbenchEnvironmentService } from '../../environment/electron-browser/environmentService.js';
3333
import { IShellEnvironmentService } from '../../environment/electron-browser/shellEnvironmentService.js';
3434
import { MessagePortExtHostConnection, writeExtHostConnection } from '../common/extensionHostEnv.js';
35-
import { IExtensionHostInitData, MessageType, NativeLogMarkers, UIKind, isMessageOfType } from '../common/extensionHostProtocol.js';
35+
import { createMessageOfType, IExtensionHostInitData, MessageType, NativeLogMarkers, UIKind, isMessageOfType } from '../common/extensionHostProtocol.js';
3636
import { LocalProcessRunningLocation } from '../common/extensionRunningLocation.js';
3737
import { ExtensionHostExtensions, ExtensionHostStartup, IExtensionHost, IExtensionInspectInfo } from '../common/extensions.js';
3838
import { IHostService } from '../../host/browser/host.js';
@@ -83,6 +83,10 @@ export class ExtensionHostProcess {
8383
return this._extensionHostStarter.enableInspectPort(this._id);
8484
}
8585

86+
public waitForExit(maxWaitTimeMs: number): Promise<void> {
87+
return this._extensionHostStarter.waitForExit(this._id, maxWaitTimeMs);
88+
}
89+
8690
public kill(): Promise<void> {
8791
return this._extensionHostStarter.kill(this._id);
8892
}
@@ -161,14 +165,39 @@ export class NativeLocalProcessExtensionHost extends Disposable implements IExte
161165
}
162166

163167
public override dispose(): void {
164-
if (this._terminating) {
165-
return;
168+
if (!this._terminating) {
169+
this._terminating = true;
166170
}
167-
this._terminating = true;
168171
super.dispose();
169172
this._messageProtocol = null;
170173
}
171174

175+
public async disconnect(): Promise<void> {
176+
this._terminating = true;
177+
178+
if (this._messageProtocol) {
179+
try {
180+
const protocol = await Promise.race([
181+
this._messageProtocol.then(protocol => protocol, () => undefined),
182+
timeout(1000).then(() => undefined)
183+
]);
184+
protocol?.send(createMessageOfType(MessageType.Terminate));
185+
} catch {
186+
// ignore - extension host may have already exited
187+
}
188+
}
189+
190+
if (this._extensionHostProcess) {
191+
try {
192+
await this._extensionHostProcess.waitForExit(extensionHostGraceTimeMs);
193+
} catch {
194+
// best-effort: waitForExit may reject with canceled() if the main side is already shutting down
195+
}
196+
}
197+
198+
this._messageProtocol = null;
199+
}
200+
172201
public start(): Promise<IMessagePassingProtocol> {
173202
if (this._terminating) {
174203
// .terminate() was called

test/smoke/extensions/vscode-smoketest-ext-host/extension.js

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,26 @@ let deactivateMarkerFile;
1616
* @param {vscode.ExtensionContext} context
1717
*/
1818
function activate(context) {
19+
// Record extension host pid on every activation so smoke tests can validate
20+
// that a new extension host process was started after a restart action.
21+
try {
22+
const pid = String(process.pid);
23+
const activationPidFile = path.join(os.tmpdir(), 'vscode-ext-host-pid-on-activate.txt');
24+
fs.writeFileSync(activationPidFile, pid, 'utf-8');
25+
} catch {
26+
// Ignore errors in smoke helper setup.
27+
}
28+
1929
// This is used to verify that the extension host process is properly killed
2030
// when window reloads even if the extension host is blocked
2131
// Refs: https://github.com/microsoft/vscode/issues/291346
2232
context.subscriptions.push(
2333
vscode.commands.registerCommand('smoketest.getExtensionHostPidAndBlock', (delayMs = 100, durationMs = 60000) => {
2434
const pid = process.pid;
2535

26-
// Write PID file to workspace folder if available, otherwise temp dir
36+
// Write PID file to temp dir to avoid polluting workspace search results
2737
// Note: filename must match name in extension-host-restart.test.ts
28-
const workspaceFolder = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath;
29-
const pidFile = workspaceFolder
30-
? path.join(workspaceFolder, 'vscode-ext-host-pid.txt')
31-
: path.join(os.tmpdir(), 'vscode-ext-host-pid.txt');
38+
const pidFile = path.join(os.tmpdir(), 'vscode-ext-host-pid.txt');
3239
setTimeout(() => {
3340
fs.writeFileSync(pidFile, String(pid), 'utf-8');
3441

@@ -57,13 +64,8 @@ function activate(context) {
5764
context.subscriptions.push(
5865
vscode.commands.registerCommand('smoketest.setupGracefulDeactivation', () => {
5966
const pid = process.pid;
60-
const workspaceFolder = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath;
61-
const pidFile = workspaceFolder
62-
? path.join(workspaceFolder, 'vscode-ext-host-pid-graceful.txt')
63-
: path.join(os.tmpdir(), 'vscode-ext-host-pid-graceful.txt');
64-
deactivateMarkerFile = workspaceFolder
65-
? path.join(workspaceFolder, 'vscode-ext-host-deactivated.txt')
66-
: path.join(os.tmpdir(), 'vscode-ext-host-deactivated.txt');
67+
const pidFile = path.join(os.tmpdir(), 'vscode-ext-host-pid-graceful.txt');
68+
deactivateMarkerFile = path.join(os.tmpdir(), 'vscode-ext-host-deactivated.txt');
6769

6870
// Write PID file immediately so test knows the extension is ready
6971
fs.writeFileSync(pidFile, String(pid), 'utf-8');

test/smoke/src/areas/extensions/extension-host-restart.test.ts

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as fs from 'fs';
7+
import * as os from 'os';
78
import * as path from 'path';
89
import { Application, Logger } from '../../../../automation';
910
import { installAllHandlers, timeout } from '../../utils';
@@ -30,7 +31,7 @@ export function setup(logger: Logger) {
3031
this.timeout(60_000);
3132

3233
const app = this.app as Application;
33-
const pidFile = path.join(app.workspacePathOrFolder, 'vscode-ext-host-pid.txt');
34+
const pidFile = path.join(os.tmpdir(), 'vscode-ext-host-pid.txt');
3435

3536
if (fs.existsSync(pidFile)) {
3637
fs.unlinkSync(pidFile);
@@ -78,8 +79,8 @@ export function setup(logger: Logger) {
7879
this.timeout(60_000);
7980

8081
const app = this.app as Application;
81-
const pidFile = path.join(app.workspacePathOrFolder, 'vscode-ext-host-pid-graceful.txt');
82-
const markerFile = path.join(app.workspacePathOrFolder, 'vscode-ext-host-deactivated.txt');
82+
const pidFile = path.join(os.tmpdir(), 'vscode-ext-host-pid-graceful.txt');
83+
const markerFile = path.join(os.tmpdir(), 'vscode-ext-host-deactivated.txt');
8384

8485
// Clean up any existing files
8586
if (fs.existsSync(pidFile)) {
@@ -139,5 +140,79 @@ export function setup(logger: Logger) {
139140

140141
logger.log('Extension host was properly terminated after graceful deactivation');
141142
});
143+
144+
it('kills blocked extension host on restart extension host (issue #296681)', async function () {
145+
this.timeout(90_000);
146+
147+
const app = this.app as Application;
148+
const pidFile = path.join(os.tmpdir(), 'vscode-ext-host-pid.txt');
149+
const activationPidFile = path.join(os.tmpdir(), 'vscode-ext-host-pid-on-activate.txt');
150+
151+
if (fs.existsSync(pidFile)) {
152+
fs.unlinkSync(pidFile);
153+
}
154+
155+
await app.workbench.quickaccess.runCommand('smoketest.getExtensionHostPidAndBlock');
156+
157+
let retries = 0;
158+
while (!fs.existsSync(pidFile) && retries < 20) {
159+
await timeout(500);
160+
retries++;
161+
}
162+
163+
if (!fs.existsSync(pidFile)) {
164+
throw new Error('PID file was not created - extension may not have activated');
165+
}
166+
167+
const oldPid = parseInt(fs.readFileSync(pidFile, 'utf-8'), 10);
168+
logger.log(`Old extension host PID: ${oldPid}`);
169+
170+
if (fs.existsSync(activationPidFile)) {
171+
fs.unlinkSync(activationPidFile);
172+
}
173+
174+
await app.workbench.quickaccess.runCommand('Developer: Restart Extension Host', { keepOpen: true });
175+
176+
const maxWaitMs = 10_000;
177+
const pollIntervalMs = 500;
178+
let waitedMs = 0;
179+
180+
let newPid: number | undefined;
181+
while (waitedMs < maxWaitMs) {
182+
if (fs.existsSync(activationPidFile)) {
183+
const pidText = fs.readFileSync(activationPidFile, 'utf-8').trim();
184+
const parsedPid = parseInt(pidText, 10);
185+
if (!Number.isNaN(parsedPid) && parsedPid !== oldPid) {
186+
newPid = parsedPid;
187+
break;
188+
}
189+
}
190+
191+
await timeout(pollIntervalMs);
192+
waitedMs += pollIntervalMs;
193+
}
194+
195+
if (!newPid) {
196+
throw new Error(`New extension host PID was not observed after restart (waited ${maxWaitMs}ms)`);
197+
}
198+
199+
if (newPid === oldPid) {
200+
throw new Error(`Extension host PID did not change after restart (pid: ${oldPid})`);
201+
}
202+
203+
logger.log(`New extension host PID observed: ${newPid}`);
204+
205+
waitedMs = 0;
206+
while (processExists(oldPid) && waitedMs < maxWaitMs) {
207+
await timeout(pollIntervalMs);
208+
waitedMs += pollIntervalMs;
209+
}
210+
211+
if (processExists(oldPid)) {
212+
throw new Error(`Old extension host ${oldPid} still running after restart (waited ${maxWaitMs}ms)`);
213+
}
214+
215+
logger.log(`Extension host restarted successfully (old: ${oldPid}, new: ${newPid})`);
216+
});
142217
});
143218
}

0 commit comments

Comments
 (0)