From e71cd9e492026e4cf673492ebd51cff729fae240 Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Sun, 6 Sep 2026 00:07:48 -0700 Subject: [PATCH 1/2] Cache packaged ESM ASAR resolutions Reuse successful archive resolutions for identical ESM contexts while preserving first-resolution precedence for closer dependencies. Cover cache hits and parent isolation with a packaged ASAR fixture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/bootstrap-esm.ts | 23 +++++- src/vs/code/test/node/bootstrapESM.test.ts | 94 +++++++++++++++++++++- 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/bootstrap-esm.ts b/src/bootstrap-esm.ts index 3d529345763721..1ce3205fc506fa 100644 --- a/src/bootstrap-esm.ts +++ b/src/bootstrap-esm.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as fs from 'node:fs'; -import { createRequire, isBuiltin, registerHooks } from 'node:module'; +import { createRequire, isBuiltin, registerHooks, type Module } from 'node:module'; import { dirname, join } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import { product, pkg } from './bootstrap-meta.js'; @@ -85,6 +85,15 @@ function enableASARSupport(): void { return slash === -1 ? specifier : specifier.slice(0, slash); }; + const resolutionCache = new Map(); + const resolutionCacheKey = (specifier: string, parentPath: string, context: Module.ResolveHookContext): string => { + let key = `${specifier}\0${parentPath}\0${context.conditions.join('\0')}`; + for (const [name, value] of Object.entries(context.importAttributes ?? {}).sort(([a], [b]) => a.localeCompare(b))) { + key += `\0${name}\0${value}`; + } + return key; + }; + const appRoot = dirname(import.meta.dirname); const resourcesPath = process.env['VSCODE_DEV'] ? undefined : normalizeDriveLetter(appRoot); // Root require.resolve() inside the archive; the leading './' below avoids a node_modules walk. @@ -126,6 +135,13 @@ function enableASARSupport(): void { try { parentPath = normalizeDriveLetter(fileURLToPath(context.parentURL)); } catch { parentPath = undefined; } if (parentPath && parentPath.startsWith(resourcesPath)) { trace?.(`resolve "${specifier}" from "${context.parentURL}"`); + const cacheKey = resolutionCacheKey(specifier, parentPath, context); + const cached = resolutionCache.get(cacheKey); + if (cached) { + trace?.(` cache -> ${cached.url} (ACCEPT)`); + return cached; + } + let defaultResult; let defaultError: Error | undefined; // A closer dependency bundled by the importer takes precedence over the application archive. @@ -165,6 +181,7 @@ function enableASARSupport(): void { try { selfRefPath = normalizeDriveLetter(fileURLToPath(selfRef.url)); } catch { selfRefPath = undefined; } if (selfRefPath && selfRefPath.startsWith(resourcesPath)) { trace?.(` self-ref -> ${selfRef.url} (in app, ACCEPT)`); + resolutionCache.set(cacheKey, { ...selfRef, shortCircuit: true }); return selfRef; } trace?.(` self-ref -> ${selfRef.url} (escaped app, reject)`); @@ -175,7 +192,9 @@ function enableASARSupport(): void { const resolved = asarRequire.resolve(`./${specifier}`); const url = pathToFileURL(resolved).href; trace?.(` direct -> ${url} (ACCEPT)`); - return { url, shortCircuit: true }; + const result = { url, shortCircuit: true }; + resolutionCache.set(cacheKey, result); + return result; } trace?.(`defer "${specifier}" (parent outside app resources: ${context.parentURL})`); } diff --git a/src/vs/code/test/node/bootstrapESM.test.ts b/src/vs/code/test/node/bootstrapESM.test.ts index 8e0b4a32d39b1f..3c9d1ce2d7bc2f 100644 --- a/src/vs/code/test/node/bootstrapESM.test.ts +++ b/src/vs/code/test/node/bootstrapESM.test.ts @@ -5,7 +5,8 @@ import assert from 'assert'; import { execFile } from 'child_process'; -import { mkdtemp, rm, writeFile } from 'fs/promises'; +import { copyFile, mkdir, mkdtemp, readFile, rm, writeFile } from 'fs/promises'; +import { createRequire } from 'module'; import { tmpdir } from 'os'; import { promisify } from 'util'; import { fileURLToPath, pathToFileURL } from 'url'; @@ -13,6 +14,11 @@ import { dirname, join } from '../../../base/common/path.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../base/test/common/utils.js'; const execFileAsync = promisify(execFile); +const nodeRequire = createRequire(import.meta.url); +const { createPackage, uncache } = nodeRequire('asar') as { + createPackage(source: string, destination: string): Promise; + uncache(archive: string): boolean; +}; (process.versions['electron'] ? suite : suite.skip)('bootstrap ESM', () => { ensureNoDisposablesAreLeakedInTestSuite(); @@ -20,6 +26,10 @@ const execFileAsync = promisify(execFile); let fixtureDirectory: string; let fixturePath: string; let reentrantHookPath: string; + let packagedFixturePath: string; + let packagedBootstrapPath: string; + let packagedTracePath: string; + let packagedArchivePath: string; suiteSetup(async () => { fixtureDirectory = await mkdtemp(join(tmpdir(), 'vscode-bootstrap-esm-')); @@ -62,10 +72,61 @@ const execFileAsync = promisify(execFile); requiredESMUsesOriginalFs: requiredESM.usesOriginalFs })); `); + + const outRoot = join(dirname(fileURLToPath(import.meta.url)), '../../../../'); + const packagedAppRoot = join(fixtureDirectory, 'resources', 'app'); + const packagedOutRoot = join(packagedAppRoot, 'out'); + await mkdir(join(packagedOutRoot, 'vs', 'base', 'common'), { recursive: true }); + await Promise.all([ + copyFile(join(outRoot, 'bootstrap-esm.js'), join(packagedOutRoot, 'bootstrap-esm.js')), + copyFile(join(outRoot, 'bootstrap-meta.js'), join(packagedOutRoot, 'bootstrap-meta.js')), + copyFile(join(outRoot, 'bootstrap-node.js'), join(packagedOutRoot, 'bootstrap-node.js')), + copyFile(join(outRoot, 'vs', 'base', 'common', 'performance.js'), join(packagedOutRoot, 'vs', 'base', 'common', 'performance.js')), + writeFile(join(packagedAppRoot, 'product.json'), '{}'), + writeFile(join(packagedAppRoot, 'package.json'), '{"type":"module"}'), + ]); + + const archiveSource = join(fixtureDirectory, 'archive-source'); + const packageRoot = join(archiveSource, 'cache-test'); + await mkdir(packageRoot, { recursive: true }); + await writeFile(join(packageRoot, 'package.json'), '{"name":"cache-test","type":"module","exports":"./index.js"}'); + await writeFile(join(packageRoot, 'index.js'), 'export const value = 1;'); + packagedArchivePath = join(packagedAppRoot, 'node_modules.asar'); + await createPackage(archiveSource, packagedArchivePath); + + packagedFixturePath = join(packagedOutRoot, 'cache-fixture.mjs'); + packagedBootstrapPath = join(packagedOutRoot, 'bootstrap-esm.js'); + packagedTracePath = join(fixtureDirectory, 'asar-trace.log'); + await writeFile(join(packagedOutRoot, 'cache-parent-a.mjs'), ` + export const first = () => import('cache-test'); + export const second = () => import('cache-test'); + `); + await writeFile(join(packagedOutRoot, 'cache-parent-b.mjs'), ` + export const first = () => import('cache-test'); + export const second = () => import('cache-test'); + `); + await writeFile(packagedFixturePath, ` + const firstParent = await import('./cache-parent-a.mjs'); + const secondParent = await import('./cache-parent-b.mjs'); + const modules = await Promise.all([ + firstParent.first(), + firstParent.second(), + secondParent.first(), + secondParent.second() + ]); + process.stdout.write(JSON.stringify(modules.map(module => module.value))); + `); }); suiteTeardown(async () => { - await rm(fixtureDirectory, { recursive: true, force: true }); + uncache(packagedArchivePath); + const previousNoAsar = process.noAsar; + process.noAsar = true; + try { + await rm(fixtureDirectory, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }); + } finally { + process.noAsar = previousNoAsar; + } }); for (const condition of [undefined, 'require', 'import']) { @@ -98,4 +159,33 @@ const execFileAsync = promisify(execFile); }); }); } + + test('caches identical packaged ESM archive resolutions', async () => { + const env: NodeJS.ProcessEnv = { + ...process.env, + ELECTRON_RUN_AS_NODE: '1', + VSCODE_ASAR_TRACE: packagedTracePath + }; + delete env['NODE_OPTIONS']; + delete env['VSCODE_DEV']; + + const { stdout } = await execFileAsync(process.execPath, [ + '--import', + pathToFileURL(packagedBootstrapPath).href, + packagedFixturePath + ], { env }); + const trace = await readFile(packagedTracePath, 'utf8'); + + assert.deepStrictEqual({ + values: JSON.parse(stdout), + resolveCount: trace.match(/resolve "cache-test"/g)?.length, + archiveLookupCount: trace.match(/archive pkg\.json/g)?.length, + cacheHitCount: trace.match(/cache ->/g)?.length, + }, { + values: [1, 1, 1, 1], + resolveCount: 4, + archiveLookupCount: 2, + cacheHitCount: 2, + }); + }); }); From b4a0bfc933e6c6e7193d4780cb0d0a42b1b6542e Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Sun, 6 Sep 2026 00:17:43 -0700 Subject: [PATCH 2/2] Fix ASAR resolution cache key collisions Encode conditions and import attributes as separate structural tuple fields and cover the boundary collision with conditional exports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/bootstrap-esm.ts | 7 +-- src/vs/code/test/node/bootstrapESM.test.ts | 68 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/bootstrap-esm.ts b/src/bootstrap-esm.ts index 1ce3205fc506fa..555f0066b11bbd 100644 --- a/src/bootstrap-esm.ts +++ b/src/bootstrap-esm.ts @@ -87,11 +87,8 @@ function enableASARSupport(): void { const resolutionCache = new Map(); const resolutionCacheKey = (specifier: string, parentPath: string, context: Module.ResolveHookContext): string => { - let key = `${specifier}\0${parentPath}\0${context.conditions.join('\0')}`; - for (const [name, value] of Object.entries(context.importAttributes ?? {}).sort(([a], [b]) => a.localeCompare(b))) { - key += `\0${name}\0${value}`; - } - return key; + const importAttributes = Object.entries(context.importAttributes ?? {}).sort(([a], [b]) => a.localeCompare(b)); + return JSON.stringify([specifier, parentPath, context.conditions, importAttributes]); }; const appRoot = dirname(import.meta.dirname); diff --git a/src/vs/code/test/node/bootstrapESM.test.ts b/src/vs/code/test/node/bootstrapESM.test.ts index 3c9d1ce2d7bc2f..bc32465b385b22 100644 --- a/src/vs/code/test/node/bootstrapESM.test.ts +++ b/src/vs/code/test/node/bootstrapESM.test.ts @@ -27,8 +27,10 @@ const { createPackage, uncache } = nodeRequire('asar') as { let fixturePath: string; let reentrantHookPath: string; let packagedFixturePath: string; + let packagedCollisionFixturePath: string; let packagedBootstrapPath: string; let packagedTracePath: string; + let packagedCollisionTracePath: string; let packagedArchivePath: string; suiteSetup(async () => { @@ -91,12 +93,28 @@ const { createPackage, uncache } = nodeRequire('asar') as { await mkdir(packageRoot, { recursive: true }); await writeFile(join(packageRoot, 'package.json'), '{"name":"cache-test","type":"module","exports":"./index.js"}'); await writeFile(join(packageRoot, 'index.js'), 'export const value = 1;'); + + const collisionPackageRoot = join(archiveSource, 'cache-collision'); + await mkdir(collisionPackageRoot, { recursive: true }); + await writeFile(join(collisionPackageRoot, 'package.json'), JSON.stringify({ + name: 'cache-collision', + type: 'module', + exports: { + type: './condition.js', + default: './attribute.json' + } + })); + await writeFile(join(collisionPackageRoot, 'condition.js'), 'export const value = "condition";'); + await writeFile(join(collisionPackageRoot, 'attribute.json'), '{"value":"attribute"}'); + packagedArchivePath = join(packagedAppRoot, 'node_modules.asar'); await createPackage(archiveSource, packagedArchivePath); packagedFixturePath = join(packagedOutRoot, 'cache-fixture.mjs'); + packagedCollisionFixturePath = join(packagedOutRoot, 'cache-collision-fixture.mjs'); packagedBootstrapPath = join(packagedOutRoot, 'bootstrap-esm.js'); packagedTracePath = join(fixtureDirectory, 'asar-trace.log'); + packagedCollisionTracePath = join(fixtureDirectory, 'asar-collision-trace.log'); await writeFile(join(packagedOutRoot, 'cache-parent-a.mjs'), ` export const first = () => import('cache-test'); export const second = () => import('cache-test'); @@ -116,6 +134,27 @@ const { createPackage, uncache } = nodeRequire('asar') as { ]); process.stdout.write(JSON.stringify(modules.map(module => module.value))); `); + await writeFile(packagedCollisionFixturePath, ` + import { registerHooks } from 'node:module'; + + let first = true; + registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier === 'cache-collision' && first) { + first = false; + return nextResolve(specifier, { + ...context, + conditions: [...context.conditions, 'type', 'json'] + }); + } + return nextResolve(specifier, context); + } + }); + + const conditional = await import('cache-collision'); + const attributed = await import('cache-collision', { with: { type: 'json' } }); + process.stdout.write(JSON.stringify([conditional.value, attributed.default.value])); + `); }); suiteTeardown(async () => { @@ -188,4 +227,33 @@ const { createPackage, uncache } = nodeRequire('asar') as { cacheHitCount: 2, }); }); + + test('distinguishes conditions from import attributes in the resolution cache', async () => { + const env: NodeJS.ProcessEnv = { + ...process.env, + ELECTRON_RUN_AS_NODE: '1', + VSCODE_ASAR_TRACE: packagedCollisionTracePath + }; + delete env['NODE_OPTIONS']; + delete env['VSCODE_DEV']; + + const { stdout } = await execFileAsync(process.execPath, [ + '--import', + pathToFileURL(packagedBootstrapPath).href, + packagedCollisionFixturePath + ], { env }); + const trace = await readFile(packagedCollisionTracePath, 'utf8'); + + assert.deepStrictEqual({ + values: JSON.parse(stdout), + resolveCount: trace.match(/resolve "cache-collision"/g)?.length, + archiveLookupCount: trace.match(/archive pkg\.json/g)?.length, + cacheHitCount: trace.match(/cache ->/g)?.length ?? 0, + }, { + values: ['condition', 'attribute'], + resolveCount: 2, + archiveLookupCount: 2, + cacheHitCount: 0, + }); + }); });