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.
Copy link
Contributor Author

@mizdra mizdra Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit confusing that only require('vscode') gets intercepted by NodeModuleRequireInterceptor. So I tried modifying it as follows to have require('vscode') intercepted by NodeModuleInterceptor.

--- a/src/vs/workbench/api/node/extHostExtensionService.ts
+++ b/src/vs/workbench/api/node/extHostExtensionService.ts
@@ -35,7 +35,7 @@ class NodeModuleRequireInterceptor extends RequireInterceptor {
                const originalLoad = node_module._load;
                node_module._load = function load(request: string, parent: { filename: string }, isMain: boolean) {
                        request = applyAlternatives(request);
-                       if (!that._factories.has(request)) {
+                       if (!that._factories.has(request) || request === 'vscode') {
                                return originalLoad.apply(this, arguments);
                        }

However, this causes an ENAMETOOLONG error.

$ cd repro-vscode-test-hang

$ # Edit files
$ vim

$ git diff
diff --git a/test/index.test.js b/test/index.test.js
index 209123b..3b8b882 100644
--- a/test/index.test.js
+++ b/test/index.test.js
@@ -1,6 +1,10 @@
 import assert from 'node:assert/strict';

 // The following will hang the test.
-import * as vscode from 'vscode';
+// import * as vscode from 'vscode';
+
+import { createRequire } from 'node:module';
+const require = createRequire(import.meta.url);
+const vscode = require('vscode');

 assert.equal(1, 2);
diff --git a/test/runTest.js b/test/runTest.js
index 01ac63b..99c3376 100644
--- a/test/runTest.js
+++ b/test/runTest.js
@@ -12,7 +12,7 @@ async function main() {
       extensionDevelopmentPath,
       extensionTestsPath,
       launchArgs: ['--disable-extensions'],
-      // vscodeExecutablePath: '/Users/mizdra/src/github.com/microsoft/vscode/scripts/code.sh',
+      vscodeExecutablePath: '/Users/mizdra/src/github.com/microsoft/vscode/scripts/code.sh',
     });
   } catch (err) {
     console.error(err);


$ npm start
Error: ENAMETOOLONG: name too long, open 'data:text/javascript;base64,Y29uc3QgX3ZzY29kZUluc3RhbmNlID0gZ2xvYmFsVGhpcy5fVlNDT0RFX0lNUE9SVF9WU0NPREVfQVBJKCc5MGMzNTUwMC0yNjY0LTQ2M2...(long text)...107'
	at Object.readFileSync (node:fs:441:20)
	at t.readFileSync (node:electron/js2c/node_init:2:11013)
	at defaultLoadImpl (node:internal/modules/cjs/loader:1112:17)
	at loadSource (node:internal/modules/cjs/loader:1742:20)
	at Module._extensions..js (node:internal/modules/cjs/loader:1841:44)
	at Module.load (node:internal/modules/cjs/loader:1448:32)
	at Module._load (node:internal/modules/cjs/loader:1270:12)
	at c._load (node:electron/js2c/node_init:2:17993)
	at Module._load (file:///Users/mizdra/src/github.com/microsoft/vscode/out/vs/workbench/api/node/extensionHostProcess.js:68:29)
	at Module.load (file:///Users/mizdra/src/github.com/microsoft/vscode/out/vs/workbench/api/node/extHostExtensionService.js:32:37)
	at Module.load [as _load] (file:///Users/mizdra/src/github.com/microsoft/vscode/out/vs/workbench/api/node/proxyResolver.js:290:33)
	at TracingChannel.traceSync (node:diagnostics_channel:328:14)
	at wrapModuleLoad (node:internal/modules/cjs/loader:244:24)
	at Module.require (node:internal/modules/cjs/loader:1470:12)
	at require (node:internal/modules/helpers:147:16)
	at file:///Users/mizdra/src/github.com/mizdra/repro-vscode-test-hang/test/index.test.js:8:16
	at ModuleJobSync.runSync (node:internal/modules/esm/module_job:454:37)
	at ModuleLoader.importSyncForRequire (node:internal/modules/esm/loader:435:47)
	at loadESMFromCJS (node:internal/modules/cjs/loader:1544:24)
	at Module._compile (node:internal/modules/cjs/loader:1695:5)
	at Module._extensions..js (node:internal/modules/cjs/loader:1848:10)
	at Module.load (node:internal/modules/cjs/loader:1448:32)
	at Module._load (node:internal/modules/cjs/loader:1270:12)
	at c._load (node:electron/js2c/node_init:2:17993)
	at Module._load (file:///Users/mizdra/src/github.com/microsoft/vscode/out/vs/workbench/api/node/extensionHostProcess.js:68:29)
	at Module.load (file:///Users/mizdra/src/github.com/microsoft/vscode/out/vs/workbench/api/node/extHostExtensionService.js:32:37)
	at Module.load [as _load] (file:///Users/mizdra/src/github.com/microsoft/vscode/out/vs/workbench/api/node/proxyResolver.js:290:33)
	at TracingChannel.traceSync (node:diagnostics_channel:328:14)
	at wrapModuleLoad (node:internal/modules/cjs/loader:244:24)
	at Module.require (node:internal/modules/cjs/loader:1470:12)
	at require (node:internal/modules/helpers:147:16)
	at Object.run (/Users/mizdra/src/github.com/mizdra/repro-vscode-test-hang/test/runner.cjs:3:3)
	at file:///Users/mizdra/src/github.com/microsoft/vscode/out/vs/workbench/api/common/extHostExtensionService.js:601:42
	at new Promise (<anonymous>)
	at ExtHostExtensionService._doHandleExtensionTests (file:///Users/mizdra/src/github.com/microsoft/vscode/out/vs/workbench/api/common/extHostExtensionService.js:580:16)
	at async ExtHostExtensionService.$extensionTestsExecute (file:///Users/mizdra/src/github.com/microsoft/vscode/out/vs/workbench/api/common/extHostExtensionService.js:558:20)

For some reason, originalLoad.apply() appears to return a base64-encoded path generated by NodeModuleInterceptor instead of the module instance. This seems to be a Node.js bug.

Since I couldn't find a workaround, I decided to abandon intercepting require('vscode') with NodeModuleInterceptor.

// In the future, we can consider migrating all interception logic to `NodeModuleInterceptor` and removing `NodeModuleRequireInterceptor`.
Copy link
Contributor Author

@mizdra mizdra Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module._load was able to return an object from a hook. However, hooks registered via module.registerHooks currently cannot do this. Therefore, NodeModuleInterceptor circumvents this limitation by using base64-encoded source text and NodeModuleInterceptor._vscodeImportFnName. This is a complex workaround.

Incidentally, it appears Node.js plans to implement hooks that can return objects. Once implemented, this complex workaround could be removed. Porting the logic from NodeModuleRequireInterceptor to NodeModuleInterceptor should also become easier.

await this._store.add(this._instaService.createInstance(NodeModuleInterceptor, extensionApiFactory, { mine: this._myRegistry, all: this._globalRegistry }))
.install();

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