Skip to content

Commit

Permalink
detect cycles during this.loadModule from loader
Browse files Browse the repository at this point in the history
  • Loading branch information
sokra committed Dec 20, 2019
1 parent dda3279 commit d16abb3
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 2 deletions.
56 changes: 55 additions & 1 deletion lib/Compilation.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const RuntimeTemplate = require("./RuntimeTemplate");
const Stats = require("./Stats");
const WebpackError = require("./WebpackError");
const buildChunkGraph = require("./buildChunkGraph");
const BuildCycleError = require("./errors/BuildCycleError");
const { Logger, LogType } = require("./logging/Logger");
const StatsFactory = require("./stats/StatsFactory");
const StatsPrinter = require("./stats/StatsPrinter");
Expand Down Expand Up @@ -581,6 +582,14 @@ class Compilation {
processor: this._processModuleDependencies.bind(this)
});

/**
* Modules in value are building during the build of Module in key.
* Means value blocking key from finishing.
* Needed to detect build cycles.
* @type {WeakMap<Module, Set<Module>>}
*/
this.creatingModuleDuringBuild = new WeakMap();

/** @type {Map<string, EntryDependency[]>} */
this.entryDependencies = new Map();
/** @type {Map<string, Entrypoint>} */
Expand Down Expand Up @@ -1088,6 +1097,7 @@ class Compilation {
* @property {Dependency[]} dependencies
* @property {Module | null} originModule
* @property {string=} context
* @property {boolean=} recursive recurse into dependencies of the created module
*/

/**
Expand All @@ -1096,7 +1106,7 @@ class Compilation {
* @returns {void}
*/
handleModuleCreation(
{ factory, dependencies, originModule, context },
{ factory, dependencies, originModule, context, recursive = true },
callback
) {
const moduleGraph = this.moduleGraph;
Expand Down Expand Up @@ -1153,7 +1163,46 @@ class Compilation {
}
}

// Check for cycles when build is trigger inside another build
let creatingModuleDuringBuildSet = undefined;
if (!recursive && this.buildQueue.isProcessing(originModule)) {
// Track build dependency
creatingModuleDuringBuildSet = this.creatingModuleDuringBuild.get(
originModule
);
if (creatingModuleDuringBuildSet === undefined) {
creatingModuleDuringBuildSet = new Set();
this.creatingModuleDuringBuild.set(
originModule,
creatingModuleDuringBuildSet
);
}
creatingModuleDuringBuildSet.add(originModule);

// When building is blocked by another module
// search for a cycle, cancel the cycle by throwing
// an error (otherwise this would deadlock)
const blockReasons = this.creatingModuleDuringBuild.get(module);
if (blockReasons !== undefined) {
const set = new Set(blockReasons);
for (const item of set) {
const blockReasons = this.creatingModuleDuringBuild.get(item);
if (blockReasons !== undefined) {
for (const m of blockReasons) {
if (m === module) {
return callback(new BuildCycleError(module));
}
set.add(m);
}
}
}
}
}

this.buildModule(module, err => {
if (creatingModuleDuringBuildSet !== undefined) {
creatingModuleDuringBuildSet.delete(module);
}
if (err) {
if (!err.module) {
err.module = module;
Expand All @@ -1163,6 +1212,11 @@ class Compilation {
return callback(err);
}

if (!recursive) {
callback(null, module);
return;
}

// This avoids deadlocks for circular dependencies
if (this.processDependenciesQueue.isProcessing(module)) {
return callback();
Expand Down
3 changes: 2 additions & 1 deletion lib/dependencies/LoaderPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ class LoaderPlugin {
factory,
dependencies: [dep],
originModule: loaderContext._module,
context: loaderContext.context
context: loaderContext.context,
recursive: false
},
err => {
compilation.buildQueue.decreaseParallelism();
Expand Down
28 changes: 28 additions & 0 deletions lib/errors/BuildCycleError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
MIT License http://www.opensource.org/licenses/mit-license.php
Author Tobias Koppers @sokra
*/

"use strict";

const WebpackError = require("../WebpackError");

/** @typedef {import("../Module")} Module */

class BuildCycleError extends WebpackError {
/**
* Creates an instance of ModuleDependencyError.
* @param {Module} module the module starting the cycle
*/
constructor(module) {
super(
"There is a circular build dependency, which makes it impossible to create this module"
);

this.name = "BuildCycleError";
this.module = module;
Error.captureStackTrace(this, this.constructor);
}
}

module.exports = BuildCycleError;
1 change: 1 addition & 0 deletions test/cases/errors/load-module-cycle/1/a.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"./a.json"
1 change: 1 addition & 0 deletions test/cases/errors/load-module-cycle/2/a.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"./b.json"
1 change: 1 addition & 0 deletions test/cases/errors/load-module-cycle/2/b.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"./a.json"
1 change: 1 addition & 0 deletions test/cases/errors/load-module-cycle/3/a.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"./b.json"
1 change: 1 addition & 0 deletions test/cases/errors/load-module-cycle/3/b.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"./c.json"
1 change: 1 addition & 0 deletions test/cases/errors/load-module-cycle/3/c.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"./a.json"
15 changes: 15 additions & 0 deletions test/cases/errors/load-module-cycle/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
it("should error loadModule when a cycle with 2 modules is requested", () => {
expect(require("./loader!./2/a")).toMatch(
/^source: err: There is a circular build dependency/
);
});
it("should error loadModule when a cycle with 3 modules is requested", () => {
expect(require("./loader!./3/a")).toMatch(
/^source: source: err: There is a circular build dependency/
);
});
it("should error loadModule when requesting itself", () => {
expect(require("./loader!./1/a")).toMatch(
/^err: There is a circular build dependency/
);
});
11 changes: 11 additions & 0 deletions test/cases/errors/load-module-cycle/loader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
exports.default = function(source) {
const ref = JSON.parse(source);
const callback = this.async();
this.loadModule("../loader!" + ref, (err, source, sourceMap, module) => {
if (err) {
callback(null, JSON.stringify(`err: ${err && err.message}`));
} else {
callback(null, JSON.stringify(`source: ${JSON.parse(source)}`));
}
});
};

0 comments on commit d16abb3

Please sign in to comment.