Skip to content

Fix deadlock caused by import 'vscode' from modules loaded via require(esm) #285417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mizdra wants to merge 1 commit into
base: main
Choose a base branch
from
+27 −66
Open

Fix deadlock caused by import 'vscode' from modules loaded via require(esm) #285417

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
93 changes: 27 additions & 66 deletions src/vs/workbench/api/node/extHostExtensionService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import nodeModule from 'node:module';
import { assertType } from '../../../base/common/types.js';
import { generateUuid } from '../../../base/common/uuid.js';
import { BidirectionalMap } from '../../../base/common/map.js';
import { DisposableStore, toDisposable } from '../../../base/common/lifecycle.js';
import { DisposableStore } from '../../../base/common/lifecycle.js';

const require = nodeModule.createRequire(import.meta.url);

Expand Down Expand Up @@ -75,43 +75,12 @@ class NodeModuleRequireInterceptor extends RequireInterceptor {
}
}

class NodeModuleESMInterceptor extends RequireInterceptor {
class NodeModuleInterceptor extends RequireInterceptor {

private static _createDataUri(scriptContent: string): string {
return `data:text/javascript;base64,${Buffer.from(scriptContent).toString('base64')}`;
}

// This string is a script that runs in the loader thread of NodeJS.
private static _loaderScript = `
let lookup;
export const initialize = async (context) => {
let requestIds = 0;
const { port } = context;
const pendingRequests = new Map();
port.onmessage = (event) => {
const { id, url } = event.data;
pendingRequests.get(id)?.(url);
};
lookup = url => {
// debugger;
const myId = requestIds++;
return new Promise((resolve) => {
pendingRequests.set(myId, resolve);
port.postMessage({ id: myId, url, });
});
};
};
export const resolve = async (specifier, context, nextResolve) => {
if (specifier !== 'vscode' || !context.parentURL) {
return nextResolve(specifier, context);
}
const otherUrl = await lookup(context.parentURL);
return {
url: otherUrl,
shortCircuit: true,
};
};`;

private static _vscodeImportFnName = `_VSCODE_IMPORT_VSCODE_API`;

private readonly _store = new DisposableStore();
Expand All @@ -121,14 +90,11 @@ class NodeModuleESMInterceptor extends RequireInterceptor {
}

protected override _installInterceptor(): void {

type Message = { id: string; url: string };

const apiInstances = new BidirectionalMap<typeof vscode, string>();
const apiImportDataUrl = new Map<string, string>();

// define a global function that can be used to get API instances given a random key
Object.defineProperty(globalThis, NodeModuleESMInterceptor._vscodeImportFnName, {
Object.defineProperty(globalThis, NodeModuleInterceptor._vscodeImportFnName, {
enumerable: false,
configurable: false,
writable: false,
Expand All @@ -137,24 +103,16 @@ class NodeModuleESMInterceptor extends RequireInterceptor {
}
});

const { port1, port2 } = new MessageChannel();

let apiModuleFactory: INodeModuleFactory | undefined;

// this is a workaround for the fact that the layer checker does not understand
// that onmessage is NodeJS API here
const port1LayerCheckerWorkaround: any = port1;

port1LayerCheckerWorkaround.onmessage = (e: { data: Message }) => {

const lookup = (url: string): string => {
// Get the vscode-module factory - which is the same logic that's also used by
// the CommonJS require interceptor
if (!apiModuleFactory) {
apiModuleFactory = this._factories.get('vscode');
assertType(apiModuleFactory);
}

const { id, url } = e.data;
const uri = URI.parse(url);

// Get or create the API instance. The interface is per extension and extensions are
Expand All @@ -169,27 +127,25 @@ class NodeModuleESMInterceptor extends RequireInterceptor {
// Create and cache a data-url which is the import script for the API instance
let scriptDataUrlSrc = apiImportDataUrl.get(key);
if (!scriptDataUrlSrc) {
const jsCode = `const _vscodeInstance = globalThis.${NodeModuleESMInterceptor._vscodeImportFnName}('${key}');\n\n${Object.keys(apiInstance).map((name => `export const ${name} = _vscodeInstance['${name}'];`)).join('\n')}`;
scriptDataUrlSrc = NodeModuleESMInterceptor._createDataUri(jsCode);
const jsCode = `const _vscodeInstance = globalThis.${NodeModuleInterceptor._vscodeImportFnName}('${key}');\n\n${Object.keys(apiInstance).map((name => `export const ${name} = _vscodeInstance['${name}'];`)).join('\n')}`;
scriptDataUrlSrc = NodeModuleInterceptor._createDataUri(jsCode);
apiImportDataUrl.set(key, scriptDataUrlSrc);
}

port1.postMessage({
id,
url: scriptDataUrlSrc
});
return scriptDataUrlSrc;
};

nodeModule.register(NodeModuleESMInterceptor._createDataUri(NodeModuleESMInterceptor._loaderScript), {
parentURL: import.meta.url,
data: { port: port2 },
transferList: [port2],
nodeModule.registerHooks({
resolve: (specifier, context, nextResolve) => {
if (specifier !== 'vscode' || !context.parentURL) {
return nextResolve(specifier, context);
}
console.log('ESM resolve', specifier, context);
const otherUrl = lookup(context.parentURL);
return {
url: otherUrl,
shortCircuit: true,
};
},
});

this._store.add(toDisposable(() => {
port1.close();
port2.close();
}));
}
}

Expand All @@ -216,12 +172,17 @@ export class ExtHostExtensionService extends AbstractExtHostExtensionService {
// Register local file system shortcut
this._instaService.createInstance(ExtHostDiskFileSystemProvider);

// Module loading tricks
// Module loading tricks based on `module._load`.
// `module._load` intercepts `require(...)`.
await this._instaService.createInstance(NodeModuleRequireInterceptor, extensionApiFactory, { mine: this._myRegistry, all: this._globalRegistry })
.install();

// ESM loading tricks
await this._store.add(this._instaService.createInstance(NodeModuleESMInterceptor, extensionApiFactory, { mine: this._myRegistry, all: this._globalRegistry }))
// Module loading tricks based on `module.registerHooks`.
// `module.registerHooks` is a generic interceptor that intercepts `require(...)`, `import ...`, and `import(...)`.
// However, at this time, `NodeModuleInterceptor` only intercepts `import 'vscode'` and `import('vscode')`.
// This can also intercept `require('vscode')`, but interception by `NodeModuleRequireInterceptor` takes precedence.
// In the future, we can consider migrating all interception logic to `NodeModuleInterceptor` and removing `NodeModuleRequireInterceptor`.
await this._store.add(this._instaService.createInstance(NodeModuleInterceptor, extensionApiFactory, { mine: this._myRegistry, all: this._globalRegistry }))
.install();

performance.mark('code/extHost/didInitAPI');
Expand Down