Merge pull request webpack#7027 from webpack/bugfix/load-module-race

fix race condition when using loadModule

Author: sokra

lib/dependencies/LoaderPlugin.js

@@ -36,6 +36,7 @@ class LoaderPlugin {
 									}`
 								)
 							);
+						compilation.semaphore.release();
 						compilation.addModuleDependencies(
 							module,
 							[
@@ -48,37 +49,39 @@ class LoaderPlugin {
 							"lm",
 							false,
 							err => {
-								if (err) return callback(err);
+								compilation.semaphore.acquire(() => {
+									if (err) return callback(err);
 
-								if (!dep.module)
-									return callback(new Error("Cannot load the module"));
+									if (!dep.module)
+										return callback(new Error("Cannot load the module"));
 
-								if (dep.module.error) return callback(dep.module.error);
-								if (!dep.module._source)
-									throw new Error(
-										"The module created for a LoaderDependency must have a property _source"
-									);
-								let source, map;
-								const moduleSource = dep.module._source;
-								if (moduleSource.sourceAndMap) {
-									const sourceAndMap = moduleSource.sourceAndMap();
-									map = sourceAndMap.map;
-									source = sourceAndMap.source;
-								} else {
-									map = moduleSource.map();
-									source = moduleSource.source();
-								}
-								if (dep.module.buildInfo.fileDependencies) {
-									for (const d of dep.module.buildInfo.fileDependencies) {
-										loaderContext.addDependency(d);
+									if (dep.module.error) return callback(dep.module.error);
+									if (!dep.module._source)
+										throw new Error(
+											"The module created for a LoaderDependency must have a property _source"
+										);
+									let source, map;
+									const moduleSource = dep.module._source;
+									if (moduleSource.sourceAndMap) {
+										const sourceAndMap = moduleSource.sourceAndMap();
+										map = sourceAndMap.map;
+										source = sourceAndMap.source;
+									} else {
+										map = moduleSource.map();
+										source = moduleSource.source();
 									}
-								}
-								if (dep.module.buildInfo.contextDependencies) {
-									for (const d of dep.module.buildInfo.contextDependencies) {
-										loaderContext.addContextDependency(d);
+									if (dep.module.buildInfo.fileDependencies) {
+										for (const d of dep.module.buildInfo.fileDependencies) {
+											loaderContext.addDependency(d);
+										}
 									}
-								}
-								return callback(null, source, map, dep.module);
+									if (dep.module.buildInfo.contextDependencies) {
+										for (const d of dep.module.buildInfo.contextDependencies) {
+											loaderContext.addContextDependency(d);
+										}
+									}
+									return callback(null, source, map, dep.module);
+								});
 							}
 						);
 					};

lib/util/Semaphore.js

@@ -8,6 +8,7 @@ class Semaphore {
 	constructor(available) {
 		this.available = available;
 		this.waiters = [];
+		this._continue = this._continue.bind(this);
 	}
 
 	acquire(callback) {
@@ -20,11 +21,19 @@ class Semaphore {
 	}
 
 	release() {
+		this.available++;
 		if (this.waiters.length > 0) {
-			const callback = this.waiters.pop();
-			process.nextTick(callback);
-		} else {
-			this.available++;
+			process.nextTick(this._continue);
+		}
+	}
+
+	_continue() {
+		if (this.available > 0) {
+			if (this.waiters.length > 0) {
+				this.available--;
+				const callback = this.waiters.pop();
+				callback();
+			}
 		}
 	}
 }

test/configCases/race-conditions/load-module/index.js

@@ -0,0 +1,4 @@
+it("should not deadlock when using loadModule", () => {
+	const result = require("./loader!");
+	result.should.match(/console.log\(42\)/);
+});

test/configCases/race-conditions/load-module/loader.js

@@ -0,0 +1,15 @@
+module.exports = function() {
+	const callback = this.async();
+	let finished = false;
+	this.loadModule("./module.js", (err, result) => {
+		if (err) return callback(err);
+		if (finished) return;
+		finished = true;
+		callback(null, `module.exports = ${JSON.stringify(result)};`);
+	});
+	setTimeout(() => {
+		if (finished) return;
+		finished = true;
+		callback(new Error("loadModule is hanging"));
+	}, 2000);
+};

test/configCases/race-conditions/load-module/module.js

@@ -0,0 +1 @@
+console.log(42);

test/configCases/race-conditions/load-module/webpack.config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	parallelism: 1
+};