Fix race condition when emitting implicitly dependent entries

src/ModuleLoader.ts

@@ -245,17 +245,23 @@ export class ModuleLoader {
 				async entryModule => {
 					addChunkNamesToModule(entryModule, unresolvedModule, false, chunkNamePriority);
 					if (!entryModule.info.isEntry) {
-						this.implicitEntryModules.add(entryModule);
 						const implicitlyLoadedAfterModules = await Promise.all(
 							implicitlyLoadedAfter.map(id =>
 								this.loadEntryModule(id, false, unresolvedModule.importer, entryModule.id)
 							)
 						);
-						for (const module of implicitlyLoadedAfterModules) {
-							entryModule.implicitlyLoadedAfter.add(module);
-						}
-						for (const dependant of entryModule.implicitlyLoadedAfter) {
-							dependant.implicitlyLoadedBefore.add(entryModule);
+						// We need to check again if this is still an entry module as these
+						// changes need to be performed atomically to avoid race conditions
+						// if the same module is re-emitted as an entry module.
+						// The inverse changes happen in "handleExistingModule"
+						if (!entryModule.info.isEntry) {
+							this.implicitEntryModules.add(entryModule);
+							for (const module of implicitlyLoadedAfterModules) {
+								entryModule.implicitlyLoadedAfter.add(module);
+							}
+							for (const dependant of entryModule.implicitlyLoadedAfter) {
+								dependant.implicitlyLoadedBefore.add(entryModule);
+							}
 						}
 					}
 					return entryModule;
@@ -615,6 +621,8 @@ export class ModuleLoader {
 				: loadPromise;
 		}
 		if (isEntry) {
+			// This reverts the changes in addEntryWithImplicitDependants and needs to
+			// be performed atomically
 			module.info.isEntry = true;
 			this.implicitEntryModules.delete(module);
 			for (const dependant of module.implicitlyLoadedAfter) {

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry/_config.js

@@ -194,6 +194,12 @@ module.exports = defineTest({
 				assert.strictEqual(dep.isEntry, true, 'dep.isEntry');
 				assert.strictEqual(dep.isDynamicEntry, false, 'dep.isDynamicEntry');
 				assert.strictEqual(dep.isImplicitEntry, false, 'dep.isImplicitEntry');
+			},
+			async transform(code, id) {
+				if (id.endsWith('main.js')) {
+					// This delay used to cause a race condition
+					await new Promise(resolve => setTimeout(resolve, 200));
+				}
 			}
 		}
 	}

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_config.js

@@ -0,0 +1,200 @@
+const assert = require('node:assert');
+const path = require('node:path');
+
+const ID_MAIN = path.join(__dirname, 'main.js');
+const ID_LIB = path.join(__dirname, 'lib.js');
+const ID_DEP = path.join(__dirname, 'dep.js');
+
+module.exports = defineTest({
+	description: 'makes sure emitted entry points are never implicit dependencies',
+	options: {
+		preserveEntrySignatures: 'allow-extension',
+		plugins: {
+			name: 'test-plugin',
+			async buildStart() {
+				this.emitFile({
+					type: 'chunk',
+					id: 'dep.js',
+					implicitlyLoadedAfterOneOf: [ID_MAIN]
+				});
+				await new Promise(resolve => setTimeout(resolve, 200));
+				this.emitFile({
+					type: 'chunk',
+					id: 'dep.js'
+				});
+			},
+			buildEnd() {
+				assert.deepStrictEqual(JSON.parse(JSON.stringify(this.getModuleInfo(ID_MAIN))), {
+					id: ID_MAIN,
+					attributes: {},
+					ast: {
+						type: 'Program',
+						start: 0,
+						end: 51,
+						body: [
+							{
+								type: 'ImportDeclaration',
+								start: 0,
+								end: 30,
+								source: { type: 'Literal', start: 22, end: 29, raw: "'./lib'", value: './lib' },
+								specifiers: [
+									{
+										type: 'ImportSpecifier',
+										start: 9,
+										end: 14,
+										imported: { type: 'Identifier', start: 9, end: 14, name: 'value' },
+										local: { type: 'Identifier', start: 9, end: 14, name: 'value' }
+									}
+								],
+								attributes: []
+							},
+							{
+								type: 'ExpressionStatement',
+								start: 31,
+								end: 50,
+								expression: {
+									type: 'CallExpression',
+									start: 31,
+									end: 49,
+									arguments: [{ type: 'Identifier', start: 43, end: 48, name: 'value' }],
+									callee: {
+										type: 'MemberExpression',
+										start: 31,
+										end: 42,
+										computed: false,
+										object: { type: 'Identifier', start: 31, end: 38, name: 'console' },
+										optional: false,
+										property: { type: 'Identifier', start: 39, end: 42, name: 'log' }
+									},
+									optional: false
+								}
+							}
+						],
+						sourceType: 'module'
+					},
+					code: "import { value } from './lib';\nconsole.log(value);\n",
+					dynamicallyImportedIdResolutions: [],
+					dynamicallyImportedIds: [],
+					dynamicImporters: [],
+					exportedBindings: {
+						'.': []
+					},
+					exports: [],
+					hasDefaultExport: false,
+					moduleSideEffects: true,
+					implicitlyLoadedAfterOneOf: [],
+					implicitlyLoadedBefore: [],
+					importedIdResolutions: [
+						{
+							attributes: {},
+							external: false,
+							id: ID_LIB,
+							meta: {},
+							moduleSideEffects: true,
+							resolvedBy: 'rollup',
+							syntheticNamedExports: false
+						}
+					],
+					importedIds: [ID_LIB],
+					importers: [],
+					isEntry: true,
+					isExternal: false,
+					isIncluded: true,
+					meta: {},
+					syntheticNamedExports: false
+				});
+				assert.deepStrictEqual(JSON.parse(JSON.stringify(this.getModuleInfo(ID_DEP))), {
+					id: ID_DEP,
+					attributes: {},
+					ast: {
+						type: 'Program',
+						start: 0,
+						end: 51,
+						body: [
+							{
+								type: 'ImportDeclaration',
+								start: 0,
+								end: 30,
+								source: { type: 'Literal', start: 22, end: 29, raw: "'./lib'", value: './lib' },
+								specifiers: [
+									{
+										type: 'ImportSpecifier',
+										start: 9,
+										end: 14,
+										imported: { type: 'Identifier', start: 9, end: 14, name: 'value' },
+										local: { type: 'Identifier', start: 9, end: 14, name: 'value' }
+									}
+								],
+								attributes: []
+							},
+							{
+								type: 'ExpressionStatement',
+								start: 31,
+								end: 50,
+								expression: {
+									type: 'CallExpression',
+									start: 31,
+									end: 49,
+									arguments: [{ type: 'Identifier', start: 43, end: 48, name: 'value' }],
+									callee: {
+										type: 'MemberExpression',
+										start: 31,
+										end: 42,
+										computed: false,
+										object: { type: 'Identifier', start: 31, end: 38, name: 'console' },
+										optional: false,
+										property: { type: 'Identifier', start: 39, end: 42, name: 'log' }
+									},
+									optional: false
+								}
+							}
+						],
+						sourceType: 'module'
+					},
+					code: "import { value } from './lib';\nconsole.log(value);\n",
+					dynamicallyImportedIdResolutions: [],
+					dynamicallyImportedIds: [],
+					dynamicImporters: [],
+					exportedBindings: {
+						'.': []
+					},
+					exports: [],
+					hasDefaultExport: false,
+					moduleSideEffects: true,
+					implicitlyLoadedAfterOneOf: [],
+					implicitlyLoadedBefore: [],
+					importedIdResolutions: [
+						{
+							attributes: {},
+							external: false,
+							id: ID_LIB,
+							meta: {},
+							moduleSideEffects: true,
+							resolvedBy: 'rollup',
+							syntheticNamedExports: false
+						}
+					],
+					importedIds: [ID_LIB],
+					importers: [],
+					isEntry: true,
+					isExternal: false,
+					isIncluded: true,
+					meta: {},
+					syntheticNamedExports: false
+				});
+			},
+			generateBundle(options, bundle) {
+				const main = bundle['main.js'];
+				assert.deepStrictEqual(main.implicitlyLoadedBefore, [], 'main.implicitlyLoadedBefore');
+				assert.strictEqual(main.isEntry, true, 'main.isEntry');
+				assert.strictEqual(main.isDynamicEntry, false, 'main.isDynamicEntry');
+				assert.strictEqual(main.isImplicitEntry, false, 'main.isImplicitEntry');
+				const dep = bundle['generated-dep.js'];
+				assert.deepStrictEqual(dep.implicitlyLoadedBefore, [], 'dep.implicitlyLoadedBefore');
+				assert.strictEqual(dep.isEntry, true, 'dep.isEntry');
+				assert.strictEqual(dep.isDynamicEntry, false, 'dep.isDynamicEntry');
+				assert.strictEqual(dep.isImplicitEntry, false, 'dep.isImplicitEntry');
+			}
+		}
+	}
+});

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/amd/generated-dep.js

@@ -0,0 +1,5 @@
+define(['./generated-lib'], (function (lib) { 'use strict';
+
+	console.log(lib.value);
+
+}));

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/amd/generated-lib.js

@@ -0,0 +1,7 @@
+define(['exports'], (function (exports) { 'use strict';
+
+	const value = 42;
+
+	exports.value = value;
+
+}));

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/amd/main.js

@@ -0,0 +1,5 @@
+define(['./generated-lib'], (function (lib) { 'use strict';
+
+	console.log(lib.value);
+
+}));

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/cjs/generated-dep.js

@@ -0,0 +1,5 @@
+'use strict';
+
+var lib = require('./generated-lib.js');
+
+console.log(lib.value);

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/cjs/generated-lib.js

@@ -0,0 +1,5 @@
+'use strict';
+
+const value = 42;
+
+exports.value = value;

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/cjs/main.js

@@ -0,0 +1,5 @@
+'use strict';
+
+var lib = require('./generated-lib.js');
+
+console.log(lib.value);

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/es/generated-dep.js

@@ -0,0 +1,3 @@
+import { v as value } from './generated-lib.js';
+
+console.log(value);

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/es/generated-lib.js

@@ -0,0 +1,3 @@
+const value = 42;
+
+export { value as v };

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/es/main.js

@@ -0,0 +1,3 @@
+import { v as value } from './generated-lib.js';
+
+console.log(value);

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/system/generated-dep.js

@@ -0,0 +1,14 @@
+System.register(['./generated-lib.js'], (function () {
+	'use strict';
+	var value;
+	return {
+		setters: [function (module) {
+			value = module.v;
+		}],
+		execute: (function () {
+
+			console.log(value);
+
+		})
+	};
+}));

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/system/generated-lib.js

@@ -0,0 +1,10 @@
+System.register([], (function (exports) {
+	'use strict';
+	return {
+		execute: (function () {
+
+			const value = exports('v', 42);
+
+		})
+	};
+}));

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/_expected/system/main.js

@@ -0,0 +1,14 @@
+System.register(['./generated-lib.js'], (function () {
+	'use strict';
+	var value;
+	return {
+		setters: [function (module) {
+			value = module.v;
+		}],
+		execute: (function () {
+
+			console.log(value);
+
+		})
+	};
+}));

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/dep.js

@@ -0,0 +1,2 @@
+import { value } from './lib';
+console.log(value);

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/lib.js

@@ -0,0 +1 @@
+export const value = 42;

test/chunking-form/samples/implicit-dependencies/implicitly-dependent-emitted-entry2/main.js

@@ -0,0 +1,2 @@
+import { value } from './lib';
+console.log(value);

test/watch/index.js

@@ -1302,7 +1302,7 @@ describe('rollup.watch', () => {
 			],
 			0
 		);
-	});
+	}).retries(1);
 
 	it('observes configured build delays', async () => {
 		await copy('test/watch/samples/basic', 'test/_tmp/input');