From 18967078cb47503284495defaf84b309c212e7c6 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 15 Mar 2021 10:29:51 +0100 Subject: [PATCH] cleanup modules after compilation to avoid leaking references when they are cached --- lib/ChunkGraph.js | 20 +++++++++ lib/Compiler.js | 24 +++++++++- lib/ContextModule.js | 9 +++- lib/DelegatedModule.js | 8 ++++ lib/DllModule.js | 8 ++++ lib/Module.js | 31 +++++++++++++ lib/ModuleGraph.js | 9 ++++ lib/NormalModule.js | 55 ++++++++++++++++++++++- lib/NormalModuleFactory.js | 36 ++++++++++----- lib/optimize/ConcatenatedModule.js | 11 +---- lib/optimize/ModuleConcatenationPlugin.js | 3 ++ types.d.ts | 28 ++++++++++++ 12 files changed, 220 insertions(+), 22 deletions(-) diff --git a/lib/ChunkGraph.js b/lib/ChunkGraph.js index 760fc8397..b9db3f832 100644 --- a/lib/ChunkGraph.js +++ b/lib/ChunkGraph.js @@ -348,6 +348,7 @@ class ChunkGraph { } cgc.modules.clear(); chunk.disconnectFromGroups(); + ChunkGraph.clearChunkGraphForChunk(chunk); } /** @@ -895,6 +896,7 @@ class ChunkGraph { chunkA.addGroup(chunkGroup); chunkB.removeGroup(chunkGroup); } + ChunkGraph.clearChunkGraphForChunk(chunkB); } /** @@ -1500,6 +1502,15 @@ Caller might not support runtime-dependent code generation (opt-out via optimiza chunkGraphForModuleMap.set(module, chunkGraph); } + // TODO remove in webpack 6 + /** + * @param {Module} module the module + * @returns {void} + */ + static clearChunkGraphForModule(module) { + chunkGraphForModuleMap.delete(module); + } + // TODO remove in webpack 6 /** * @param {Chunk} chunk the chunk @@ -1540,6 +1551,15 @@ Caller might not support runtime-dependent code generation (opt-out via optimiza static setChunkGraphForChunk(chunk, chunkGraph) { chunkGraphForChunkMap.set(chunk, chunkGraph); } + + // TODO remove in webpack 6 + /** + * @param {Chunk} chunk the chunk + * @returns {void} + */ + static clearChunkGraphForChunk(chunk) { + chunkGraphForChunkMap.delete(chunk); + } } // TODO remove in webpack 6 diff --git a/lib/Compiler.js b/lib/Compiler.js index 7df7c45d5..cbd7792f5 100644 --- a/lib/Compiler.js +++ b/lib/Compiler.js @@ -17,9 +17,11 @@ const { SizeOnlySource } = require("webpack-sources"); const webpack = require("./"); const Cache = require("./Cache"); const CacheFacade = require("./CacheFacade"); +const ChunkGraph = require("./ChunkGraph"); const Compilation = require("./Compilation"); const ConcurrentCompilationError = require("./ConcurrentCompilationError"); const ContextModuleFactory = require("./ContextModuleFactory"); +const ModuleGraph = require("./ModuleGraph"); const NormalModuleFactory = require("./NormalModuleFactory"); const RequestShortener = require("./RequestShortener"); const ResolverFactory = require("./ResolverFactory"); @@ -254,6 +256,9 @@ class Compiler { /** @type {boolean} */ this.watchMode = false; + /** @type {Compilation} */ + this._lastCompilation = undefined; + /** @private @type {WeakMap }>} */ this._assetEmittingSourceCache = new WeakMap(); /** @private @type {Map} */ @@ -350,6 +355,22 @@ class Compiler { ); } + // TODO webpack 6: solve this in a better way + // e.g. move compilation specific info from Modules into ModuleGraph + _cleanupLastCompilation() { + if (this._lastCompilation !== undefined) { + for (const module of this._lastCompilation.modules) { + ChunkGraph.clearChunkGraphForModule(module); + ModuleGraph.clearModuleGraphForModule(module); + module.cleanupForCache(); + } + for (const chunk of this._lastCompilation.chunks) { + ChunkGraph.clearChunkGraphForChunk(chunk); + } + this._lastCompilation = undefined; + } + } + /** * @param {WatchOptions} watchOptions the watcher's options * @param {Callback} handler signals when the call finishes @@ -978,7 +999,8 @@ ${other}`); } createCompilation() { - return new Compilation(this); + this._cleanupLastCompilation(); + return (this._lastCompilation = new Compilation(this)); } /** diff --git a/lib/ContextModule.js b/lib/ContextModule.js index 38731eeb0..a6f6ed41c 100644 --- a/lib/ContextModule.js +++ b/lib/ContextModule.js @@ -138,7 +138,14 @@ class ContextModule extends Module { const m = /** @type {ContextModule} */ (module); this.resolveDependencies = m.resolveDependencies; this.options = m.options; - this.resolveOptions = m.resolveOptions; + } + + /** + * Assuming this module is in the cache. Remove internal references to allow freeing some memory. + */ + cleanupForCache() { + super.cleanupForCache(); + this.resolveDependencies = undefined; } prettyRegExp(regexString) { diff --git a/lib/DelegatedModule.js b/lib/DelegatedModule.js index e33e2bde4..98e0ca8f4 100644 --- a/lib/DelegatedModule.js +++ b/lib/DelegatedModule.js @@ -224,6 +224,14 @@ class DelegatedModule extends Module { this.originalRequest = m.originalRequest; this.delegateData = m.delegateData; } + + /** + * Assuming this module is in the cache. Remove internal references to allow freeing some memory. + */ + cleanupForCache() { + super.cleanupForCache(); + this.delegateData = undefined; + } } makeSerializable(DelegatedModule, "webpack/lib/DelegatedModule"); diff --git a/lib/DllModule.js b/lib/DllModule.js index 1d10dfff9..ce124e72b 100644 --- a/lib/DllModule.js +++ b/lib/DllModule.js @@ -143,6 +143,14 @@ class DllModule extends Module { super.updateCacheModule(module); this.dependencies = module.dependencies; } + + /** + * Assuming this module is in the cache. Remove internal references to allow freeing some memory. + */ + cleanupForCache() { + super.cleanupForCache(); + this.dependencies = undefined; + } } makeSerializable(DllModule, "webpack/lib/DllModule"); diff --git a/lib/Module.js b/lib/Module.js index 7dd4780aa..d564d5eee 100644 --- a/lib/Module.js +++ b/lib/Module.js @@ -27,6 +27,7 @@ const makeSerializable = require("./util/makeSerializable"); /** @typedef {import("./ExportsInfo").UsageStateType} UsageStateType */ /** @typedef {import("./FileSystemInfo")} FileSystemInfo */ /** @typedef {import("./ModuleGraphConnection").ConnectionState} ConnectionState */ +/** @typedef {import("./NormalModuleFactory")} NormalModuleFactory */ /** @typedef {import("./RequestShortener")} RequestShortener */ /** @typedef {import("./ResolverFactory").ResolverWithOptions} ResolverWithOptions */ /** @typedef {import("./RuntimeTemplate")} RuntimeTemplate */ @@ -897,6 +898,36 @@ class Module extends DependenciesBlock { this.resolveOptions = module.resolveOptions; } + /** + * Module should be unsafe cached. Get data that's needed for that. + * This data will be passed to restoreFromUnsafeCache later. + * @returns {object} cached data + */ + getUnsafeCacheData() { + return { + factoryMeta: this.factoryMeta, + resolveOptions: this.resolveOptions + }; + } + + /** + * restore unsafe cache data + * @param {object} unsafeCacheData data from getUnsafeCacheData + * @param {NormalModuleFactory} normalModuleFactory the normal module factory handling the unsafe caching + */ + _restoreFromUnsafeCache(unsafeCacheData, normalModuleFactory) { + this.factoryMeta = unsafeCacheData.factoryMeta; + this.resolveOptions = unsafeCacheData.resolveOptions; + } + + /** + * Assuming this module is in the cache. Remove internal references to allow freeing some memory. + */ + cleanupForCache() { + this.factoryMeta = undefined; + this.resolveOptions = undefined; + } + /** * @returns {Source | null} the original source for the module before webpack transformation */ diff --git a/lib/ModuleGraph.js b/lib/ModuleGraph.js index 7bc814892..8e2e42d27 100644 --- a/lib/ModuleGraph.js +++ b/lib/ModuleGraph.js @@ -739,6 +739,15 @@ class ModuleGraph { static setModuleGraphForModule(module, moduleGraph) { moduleGraphForModuleMap.set(module, moduleGraph); } + + // TODO remove in webpack 6 + /** + * @param {Module} module the module + * @returns {void} + */ + static clearModuleGraphForModule(module) { + moduleGraphForModuleMap.delete(module); + } } // TODO remove in webpack 6 diff --git a/lib/NormalModule.js b/lib/NormalModule.js index 78a0e059e..1579118ed 100644 --- a/lib/NormalModule.js +++ b/lib/NormalModule.js @@ -55,6 +55,7 @@ const memoize = require("./util/memoize"); /** @typedef {import("./Module").NeedBuildContext} NeedBuildContext */ /** @typedef {import("./ModuleGraph")} ModuleGraph */ /** @typedef {import("./ModuleGraphConnection").ConnectionState} ConnectionState */ +/** @typedef {import("./NormalModuleFactory")} NormalModuleFactory */ /** @typedef {import("./Parser")} Parser */ /** @typedef {import("./RequestShortener")} RequestShortener */ /** @typedef {import("./ResolverFactory").ResolverWithOptions} ResolverWithOptions */ @@ -205,7 +206,9 @@ class NormalModule extends Module { * @param {string} options.resource path + query of the real resource * @param {string | undefined} options.matchResource path + query of the matched resource (virtual) * @param {Parser} options.parser the parser used + * @param {object} options.parserOptions the options of the parser used * @param {Generator} options.generator the generator used + * @param {object} options.generatorOptions the options of the generator used * @param {Object} options.resolveOptions options used for resolving requests from this module */ constructor({ @@ -218,7 +221,9 @@ class NormalModule extends Module { resource, matchResource, parser, + parserOptions, generator, + generatorOptions, resolveOptions }) { super(type, getContext(resource), layer); @@ -234,8 +239,10 @@ class NormalModule extends Module { this.binary = /^(asset|webassembly)\b/.test(type); /** @type {Parser} */ this.parser = parser; + this.parserOptions = parserOptions; /** @type {Generator} */ this.generator = generator; + this.generatorOptions = generatorOptions; /** @type {string} */ this.resource = resource; /** @type {string | undefined} */ @@ -319,12 +326,56 @@ class NormalModule extends Module { this.userRequest = m.userRequest; this.rawRequest = m.rawRequest; this.parser = m.parser; + this.parserOptions = m.parserOptions; this.generator = m.generator; + this.generatorOptions = m.generatorOptions; this.resource = m.resource; this.matchResource = m.matchResource; this.loaders = m.loaders; } + /** + * Assuming this module is in the cache. Remove internal references to allow freeing some memory. + */ + cleanupForCache() { + super.cleanupForCache(); + this.parser = undefined; + this.parserOptions = undefined; + this.generator = undefined; + this.generatorOptions = undefined; + } + + /** + * Module should be unsafe cached. Get data that's needed for that. + * This data will be passed to restoreFromUnsafeCache later. + * @returns {object} cached data + */ + getUnsafeCacheData() { + const data = super.getUnsafeCacheData(); + data.parserOptions = this.parserOptions; + data.generatorOptions = this.generatorOptions; + return data; + } + + restoreFromUnsafeCache(unsafeCacheData, normalModuleFactory) { + this._restoreFromUnsafeCache(unsafeCacheData, normalModuleFactory); + } + + /** + * restore unsafe cache data + * @param {object} unsafeCacheData data from getUnsafeCacheData + * @param {NormalModuleFactory} normalModuleFactory the normal module factory handling the unsafe caching + */ + _restoreFromUnsafeCache(unsafeCacheData, normalModuleFactory) { + this.parserOptions = unsafeCacheData.parserOptions; + this.parser = normalModuleFactory.getParser(this.type, this.parserOptions); + this.generatorOptions = unsafeCacheData.generatorOptions; + this.generator = normalModuleFactory.getGenerator( + this.type, + this.generatorOptions + ); + } + /** * @param {string} context the compilation context * @param {string} name the asset name @@ -668,7 +719,7 @@ class NormalModule extends Module { }, (err, result) => { if (!result) { - processResult( + return processResult( err || new Error("No result from loader-runner processing"), null ); @@ -1158,7 +1209,9 @@ class NormalModule extends Module { loaders: null, matchResource: null, parser: null, + parserOptions: null, generator: null, + generatorOptions: null, resolveOptions: null }); obj.deserialize(context); diff --git a/lib/NormalModuleFactory.js b/lib/NormalModuleFactory.js index 98796411e..eea75220a 100644 --- a/lib/NormalModuleFactory.js +++ b/lib/NormalModuleFactory.js @@ -160,7 +160,11 @@ const deprecationChangedHookMessage = (name, hook) => { ); }; -const dependencyCache = new WeakMap(); +/** @type {WeakMap} */ +const unsafeCacheDependencies = new WeakMap(); + +/** @type {WeakMap} */ +const unsafeCacheData = new WeakMap(); const ruleSetCompiler = new RuleSetCompiler([ new BasicMatcherRulePlugin("test", "resource"), @@ -248,14 +252,12 @@ class NormalModuleFactory extends ModuleFactory { this.fs = fs; this._globalParserOptions = options.parser; this._globalGeneratorOptions = options.generator; - /** - * @type {Map>} - */ + /** @type {Map>} */ this.parserCache = new Map(); - /** - * @type {Map>} - */ + /** @type {Map>} */ this.generatorCache = new Map(); + /** @type {WeakSet} */ + this._restoredUnsafeCacheEntries = new WeakSet(); const cacheParseResource = parseResource.bindCache( associatedObjectForCache @@ -522,7 +524,9 @@ class NormalModuleFactory extends ModuleFactory { settings, type, parser: this.getParser(type, settings.parser), + parserOptions: settings.parser, generator: this.getGenerator(type, settings.generator), + generatorOptions: settings.generator, resolveOptions }); callback(); @@ -645,8 +649,16 @@ class NormalModuleFactory extends ModuleFactory { create(data, callback) { const dependencies = /** @type {ModuleDependency[]} */ (data.dependencies); if (this.unsafeCache) { - const cacheEntry = dependencyCache.get(dependencies[0]); - if (cacheEntry) return callback(null, cacheEntry); + const cacheEntry = unsafeCacheDependencies.get(dependencies[0]); + if (cacheEntry) { + const { module } = cacheEntry; + if (!this._restoredUnsafeCacheEntries.has(module)) { + const data = unsafeCacheData.get(module); + module.restoreFromUnsafeCache(data, this); + this._restoredUnsafeCacheEntries.add(module); + } + return callback(null, cacheEntry); + } } const context = data.context || this.context; const resolveOptions = data.resolveOptions || EMPTY_RESOLVE_OPTIONS; @@ -715,10 +727,14 @@ class NormalModuleFactory extends ModuleFactory { this.unsafeCache && resolveData.cacheable && module && + module.restoreFromUnsafeCache && this.cachePredicate(module) ) { for (const d of dependencies) { - dependencyCache.set(d, factoryResult); + unsafeCacheDependencies.set(d, factoryResult); + } + if (!unsafeCacheData.has(module)) { + unsafeCacheData.set(module, module.getUnsafeCacheData()); } } diff --git a/lib/optimize/ConcatenatedModule.js b/lib/optimize/ConcatenatedModule.js index d20605fbd..5bda109e9 100644 --- a/lib/optimize/ConcatenatedModule.js +++ b/lib/optimize/ConcatenatedModule.js @@ -673,9 +673,6 @@ class ConcatenatedModule extends Module { this._modules = modules; this._runtime = runtime; this.factoryMeta = rootModule && rootModule.factoryMeta; - - // Caching - // TODO } /** @@ -686,13 +683,9 @@ class ConcatenatedModule extends Module { * @returns {void} */ updateCacheModule(module) { - super.updateCacheModule(module); - const m = /** @type {ConcatenatedModule} */ (module); - this._identifier = m._identifier; - this.rootModule = m.rootModule; - this._modules = m._modules; - this._runtime = m._runtime; + throw new Error("Must not be called"); } + /** * @returns {Set} types available (do not mutate) */ diff --git a/lib/optimize/ModuleConcatenationPlugin.js b/lib/optimize/ModuleConcatenationPlugin.js index e602119ff..b3f87ee27 100644 --- a/lib/optimize/ModuleConcatenationPlugin.js +++ b/lib/optimize/ModuleConcatenationPlugin.js @@ -427,6 +427,9 @@ class ModuleConcatenationPlugin { } } compilation.modules.delete(rootModule); + ChunkGraph.clearChunkGraphForModule(rootModule); + ModuleGraph.clearModuleGraphForModule(rootModule); + // remove module from chunk chunkGraph.replaceModule(rootModule, newModule); // replace module references with the concatenated module diff --git a/types.d.ts b/types.d.ts index 35246315d..0266e72bd 100644 --- a/types.d.ts +++ b/types.d.ts @@ -873,12 +873,14 @@ declare class ChunkGraph { deprecationCode: string ): ChunkGraph; static setChunkGraphForModule(module: Module, chunkGraph: ChunkGraph): void; + static clearChunkGraphForModule(module: Module): void; static getChunkGraphForChunk( chunk: Chunk, deprecateMessage: string, deprecationCode: string ): ChunkGraph; static setChunkGraphForChunk(chunk: Chunk, chunkGraph: ChunkGraph): void; + static clearChunkGraphForChunk(chunk: Chunk): void; } declare abstract class ChunkGroup { groupDebugId: number; @@ -5823,6 +5825,17 @@ declare class Module extends DependenciesBlock { * and properties. */ updateCacheModule(module: Module): void; + + /** + * Module should be unsafe cached. Get data that's needed for that. + * This data will be passed to restoreFromUnsafeCache later. + */ + getUnsafeCacheData(): object; + + /** + * Assuming this module is in the cache. Remove internal references to allow freeing some memory. + */ + cleanupForCache(): void; originalSource(): null | Source; addCacheDependencies( fileDependencies: LazySet, @@ -6035,6 +6048,7 @@ declare class ModuleGraph { module: Module, moduleGraph: ModuleGraph ): void; + static clearModuleGraphForModule(module: Module): void; static ModuleGraphConnection: typeof ModuleGraphConnection; } declare class ModuleGraphConnection { @@ -6557,10 +6571,18 @@ declare class NormalModule extends Module { * the parser used */ parser: Parser; + /** + * the options of the parser used + */ + parserOptions: object; /** * the generator used */ generator: Generator; + /** + * the options of the generator used + */ + generatorOptions: object; /** * options used for resolving requests from this module */ @@ -6571,11 +6593,17 @@ declare class NormalModule extends Module { rawRequest: string; binary: boolean; parser: Parser; + parserOptions: object; generator: Generator; + generatorOptions: object; resource: string; matchResource?: string; loaders: LoaderItem[]; error?: WebpackError; + restoreFromUnsafeCache( + unsafeCacheData?: any, + normalModuleFactory?: any + ): void; createSourceForAsset( context: string, name: string,