Skip to content

Commit

Permalink
preload - and and use invokeWithRetry (#146785)
Browse files Browse the repository at this point in the history
  • Loading branch information
bpasero committed Apr 13, 2022
1 parent 622d058 commit a7cdd9a
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 36 deletions.
58 changes: 53 additions & 5 deletions src/vs/base/parts/sandbox/electron-browser/preload.js
Expand Up @@ -9,6 +9,10 @@

const { ipcRenderer, webFrame, contextBridge } = require('electron');

/**
* @typedef {import('../common/sandboxTypes').ISandboxConfiguration} ISandboxConfiguration
*/

//#region Utilities

/**
Expand Down Expand Up @@ -49,14 +53,51 @@
return undefined;
}

/**
* @param {string} channel
* @param {number} retryDelay
* @returns {Promise<ISandboxConfiguration>}
*/
async function invokeWithRetry(channel, retryDelay) {
let timeoutHandle;

// A timeout promise that resolves after `retryDelay`
const timeout = new Promise(resolve => {
timeoutHandle = setTimeout(() => {
resolve();
}, retryDelay);
});

// A first `invoke` call that clears the timeout
const firstInvoke = ((async () => {
try {
return await ipcRenderer.invoke(channel);
} finally {
clearTimeout(timeoutHandle);
}
})());

// Race the `invoke` to the `setTimeout`
const result = await Promise.race([
firstInvoke,
timeout
]);

// If we have a result, return immediately
if (result) {
return result;
}

console.warn(`[preload] ipcRenderer.invoke(${channel}) did not return after ${retryDelay}ms. Retrying once...`);

// Otherwise, we retry once on the same channel
return ipcRenderer.invoke(channel);
}

//#endregion

//#region Resolve Configuration

/**
* @typedef {import('../common/sandboxTypes').ISandboxConfiguration} ISandboxConfiguration
*/

/** @type {ISandboxConfiguration | undefined} */
let configuration = undefined;

Expand All @@ -71,7 +112,14 @@
if (validateIPC(windowConfigIpcChannel)) {

// Resolve configuration from electron-main
configuration = await ipcRenderer.invoke(windowConfigIpcChannel);
//
// TODO@electron there seems to be a condition where an early
// `ipcRenderer.invoke` call does not return when running in
// smoke tests where a debugger is attached. The workaround
// here is to retry the call, but the underlying reasons are
// not yet understood.
// (https://github.com/microsoft/vscode/issues/146785)
configuration = await invokeWithRetry(windowConfigIpcChannel, 5000);

// Apply `userEnv` directly
Object.assign(process.env, configuration.userEnv);
Expand Down
6 changes: 3 additions & 3 deletions src/vs/platform/sharedProcess/electron-main/sharedProcess.ts
Expand Up @@ -60,7 +60,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
}

private async onWindowConnection(e: IpcMainEvent, nonce: string): Promise<void> {
this.logService.info('SharedProcess: on vscode:createSharedProcessMessageChannel');
this.logService.trace('SharedProcess: on vscode:createSharedProcessMessageChannel');

// release barrier if this is the first window connection
if (!this.firstWindowConnectionBarrier.isOpen()) {
Expand Down Expand Up @@ -175,7 +175,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {
// Overall signal that the shared process window was loaded and
// all services within have been created.
this._whenReady = new Promise<void>(resolve => validatedIpcMain.once('vscode:shared-process->electron-main=init-done', () => {
this.logService.info('SharedProcess: Overall ready');
this.logService.trace('SharedProcess: Overall ready');

resolve();
}));
Expand All @@ -200,7 +200,7 @@ export class SharedProcess extends Disposable implements ISharedProcess {

// Wait for window indicating that IPC connections are accepted
await new Promise<void>(resolve => validatedIpcMain.once('vscode:shared-process->electron-main=ipc-ready', () => {
this.logService.info('SharedProcess: IPC ready');
this.logService.trace('SharedProcess: IPC ready');

resolve();
}));
Expand Down
Expand Up @@ -42,10 +42,10 @@ export class SharedProcessService extends Disposable implements ISharedProcessSe

// Acquire a message port connected to the shared process
mark('code/willConnectSharedProcess');
this.logService.info('Renderer->SharedProcess#connect: before acquirePort');
this.logService.trace('Renderer->SharedProcess#connect: before acquirePort');
const port = await acquirePort('vscode:createSharedProcessMessageChannel', 'vscode:createSharedProcessMessageChannelResult');
mark('code/didConnectSharedProcess');
this.logService.info('Renderer->SharedProcess#connect: connection established');
this.logService.trace('Renderer->SharedProcess#connect: connection established');

return this._register(new MessagePortClient(port, `window:${this.windowId}`));
}
Expand Down
27 changes: 1 addition & 26 deletions test/automation/src/application.ts
Expand Up @@ -6,7 +6,6 @@
import { Workbench } from './workbench';
import { Code, launch, LaunchOptions } from './code';
import { Logger, measureAndLog } from './logger';
import { PlaywrightDriver } from './playwrightDriver';

export const enum Quality {
Dev,
Expand Down Expand Up @@ -122,7 +121,7 @@ export class Application {
await code.waitForWindowIds(ids => ids.length > 0);

// We need a rendered workbench
await this.checkWorkbenchReady(code);
await measureAndLog(code.waitForElement('.monaco-workbench'), 'Application#checkWindowReady: wait for .monaco-workbench element', this.logger);

// Remote but not web: wait for a remote connection state change
if (this.remote) {
Expand All @@ -141,28 +140,4 @@ export class Application {
}, 300 /* = 30s of retry */), 'Application#checkWindowReady: wait for remote indicator', this.logger);
}
}

private async checkWorkbenchReady(code: Code): Promise<void> {
const driver = code.driver;

// Web / Legacy: just poll for workbench element
if (this.web || !(driver instanceof PlaywrightDriver)) {
await measureAndLog(code.waitForElement('.monaco-workbench'), 'Application#checkWindowReady: wait for .monaco-workbench element', this.logger);
}

// Desktop (playwright): we see hangs, where IPC messages
// are not delivered (https://github.com/microsoft/vscode/issues/146785)
// Workaround is to try to reload the window when that happens
else {
try {
await measureAndLog(code.waitForElement('.monaco-workbench', undefined, 100 /* 10s of retry */), 'Application#checkWindowReady: wait for .monaco-workbench element', this.logger);
} catch (error) {
this.logger.log(`checkWindowReady: giving up after 10s, reloading window and trying again...`);

await driver.reload();

return this.checkWorkbenchReady(code);
}
}
}
}

0 comments on commit a7cdd9a

Please sign in to comment.