From fcf265ab6750926a084f4098f648328cbf538033 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Wed, 3 Oct 2018 21:47:02 -0700 Subject: [PATCH] =?UTF-8?q?fix=20memory=20leak=20(adapted=20from=20@arthir?= =?UTF-8?q?m=E2=80=99s=20https://github.com/ember-cli/ember-cli-htmlbars-i?= =?UTF-8?q?nline-precompile/pull/112/)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .npmignore | 1 + ember-addon-main.js | 35 ++++++++++++++++++++++-------- node-tests/fixtures/compiler.js | 5 +++++ node-tests/purge-module-test.js | 38 +++++++++++++++++++++++++++++++++ package.json | 1 + yarn.lock | 37 ++++++++++++++++++++++++++++++++ 6 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 node-tests/fixtures/compiler.js create mode 100644 node-tests/purge-module-test.js diff --git a/.npmignore b/.npmignore index f4a79dae..10be8b4f 100644 --- a/.npmignore +++ b/.npmignore @@ -15,6 +15,7 @@ bower.json ember-cli-build.js testem.js yarn.lock +node-tests # ember-try .node_modules.ember-try/ diff --git a/ember-addon-main.js b/ember-addon-main.js index 5b468ecb..e23c4e88 100644 --- a/ember-addon-main.js +++ b/ember-addon-main.js @@ -9,6 +9,29 @@ module.exports = { parentRegistry: null, + purgeModule(templateCompilerPath) { + // ensure we get a fresh templateCompilerModuleInstance per ember-addon + // instance NOTE: this is a quick hack, and will only work as long as + // templateCompilerPath is a single file bundle + // + // (╯°□°）╯︵ ɹǝqɯǝ + // + // we will also fix this in ember for future releases + + // Module will be cached in .parent.children as well. So deleting from require.cache alone is not sufficient. + let mod = require.cache[templateCompilerPath]; + if (mod && mod.parent) { + let index = mod.parent.children.indexOf(mod); + if (index >= 0) { + mod.parent.children.splice(index, 1); + } else { + throw new TypeError(`ember-cli-htmlbars attempted to purge '${templateCompilerPath}' but something went wrong.`); + } + } + + delete require.cache[templateCompilerPath]; + }, + setupPreprocessorRegistry(type, registry) { // ensure that broccoli-ember-hbs-template-compiler is not processing hbs files registry.remove('template', 'broccoli-ember-hbs-template-compiler'); @@ -64,14 +87,7 @@ module.exports = { let EmberENV = projectConfig.EmberENV || {}; let templateCompilerPath = this.templateCompilerPath(); - // ensure we get a fresh templateCompilerModuleInstance per ember-addon - // instance NOTE: this is a quick hack, and will only work as long as - // templateCompilerPath is a single file bundle - // - // (╯°□°）╯︵ ɹǝqɯǝ - // - // we will also fix this in ember for future releases - delete require.cache[templateCompilerPath]; + this.purgeModule(templateCompilerPath); // do a full clone of the EmberENV (it is guaranteed to be structured // cloneable) to prevent ember-template-compiler.js from mutating @@ -93,7 +109,8 @@ module.exports = { pluginCacheKey: pluginInfo.cacheKeys }; - delete require.cache[templateCompilerPath]; + this.purgeModule(templateCompilerPath); + delete global.Ember; delete global.EmberENV; diff --git a/node-tests/fixtures/compiler.js b/node-tests/fixtures/compiler.js new file mode 100644 index 00000000..52c5aba0 --- /dev/null +++ b/node-tests/fixtures/compiler.js @@ -0,0 +1,5 @@ +'use strict'; + +module.exports = function() { + return 'I AM MODULE OF COMPILER'; +}; diff --git a/node-tests/purge-module-test.js b/node-tests/purge-module-test.js new file mode 100644 index 00000000..f6396138 --- /dev/null +++ b/node-tests/purge-module-test.js @@ -0,0 +1,38 @@ +'use strict'; + +const purgeModule = require('../ember-addon-main').purgeModule; +const expect = require('chai').expect; + +describe('purgeModule', function() { + const FIXTURE_COMPILER_PATH = require.resolve('./fixtures/compiler'); + + it('it works correctly', function() { + expect(purgeModule('asdfasdfasdfaf-unknown-file')).to.eql(undefined); + + expect(require.cache[FIXTURE_COMPILER_PATH]).to.eql(undefined); + + require(FIXTURE_COMPILER_PATH); + + const mod = require.cache[FIXTURE_COMPILER_PATH]; + + expect(mod.parent).to.eql(module); + expect(mod.parent.children).to.include(mod); + + purgeModule(FIXTURE_COMPILER_PATH); + + expect(require.cache[FIXTURE_COMPILER_PATH]).to.eql(undefined); + expect(mod.parent.children).to.not.include(mod); + + require(FIXTURE_COMPILER_PATH); + + const freshModule = require.cache[FIXTURE_COMPILER_PATH]; + + expect(freshModule.parent).to.eql(module); + expect(freshModule.parent.children).to.include(freshModule); + + purgeModule(FIXTURE_COMPILER_PATH); + + expect(require.cache[FIXTURE_COMPILER_PATH]).to.eql(undefined); + expect(freshModule.parent.children).to.not.include(mod); + }); +}); diff --git a/package.json b/package.json index 57b64d03..543469c3 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "devDependencies": { "broccoli": "^2.0.0-beta.4", "broccoli-concat": "^3.5.1", + "chai": "^4.2.0", "co": "^4.6.0", "ember-cli": "~3.3.0", "ember-cli-app-version": "^3.0.0", diff --git a/yarn.lock b/yarn.lock index df2cbc11..84da2213 100644 --- a/yarn.lock +++ b/yarn.lock @@ -217,6 +217,10 @@ arrify@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/arrify/-/arrify-1.0.1.tgz#898508da2226f380df904728456849c1501a4b0d" +assertion-error@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/assertion-error/-/assertion-error-1.1.0.tgz#e60b6b0e8f301bd97e5375215bda406c85118c0b" + assign-symbols@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/assign-symbols/-/assign-symbols-1.0.0.tgz#59667f41fadd4f20ccbc2bb96b8d4f7f78ec0367" @@ -1347,6 +1351,17 @@ center-align@^0.1.1: align-text "^0.1.3" lazy-cache "^1.0.3" +chai@^4.2.0: + version "4.2.0" + resolved "https://registry.yarnpkg.com/chai/-/chai-4.2.0.tgz#760aa72cf20e3795e84b12877ce0e83737aa29e5" + dependencies: + assertion-error "^1.1.0" + check-error "^1.0.2" + deep-eql "^3.0.1" + get-func-name "^2.0.0" + pathval "^1.1.0" + type-detect "^4.0.5" + chalk@^1.0.0, chalk@^1.1.3: version "1.1.3" resolved "https://registry.yarnpkg.com/chalk/-/chalk-1.1.3.tgz#a8115c55e4a702fe4d150abd3872822a7e09fc98" @@ -1375,6 +1390,10 @@ charm@^1.0.0: dependencies: inherits "^2.0.1" +check-error@^1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/check-error/-/check-error-1.0.2.tgz#574d312edd88bb5dd8912e9286dd6c0aed4aac82" + chokidar@1.7.0: version "1.7.0" resolved "https://registry.yarnpkg.com/chokidar/-/chokidar-1.7.0.tgz#798e689778151c8076b4b360e5edd28cda2bb468" @@ -1740,6 +1759,12 @@ decompress-response@^3.3.0: dependencies: mimic-response "^1.0.0" +deep-eql@^3.0.1: + version "3.0.1" + resolved "https://registry.yarnpkg.com/deep-eql/-/deep-eql-3.0.1.tgz#dfc9404400ad1c8fe023e7da1df1c147c4b444df" + dependencies: + type-detect "^4.0.0" + deep-extend@^0.6.0: version "0.6.0" resolved "https://registry.yarnpkg.com/deep-extend/-/deep-extend-0.6.0.tgz#c4fa7c95404a17a9c3e8ca7e1537312b736330ac" @@ -2905,6 +2930,10 @@ get-caller-file@^1.0.0: version "1.0.3" resolved "https://registry.yarnpkg.com/get-caller-file/-/get-caller-file-1.0.3.tgz#f978fa4c90d1dfe7ff2d6beda2a515e713bdcf4a" +get-func-name@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/get-func-name/-/get-func-name-2.0.0.tgz#ead774abee72e20409433a066366023dd6887a41" + get-stdin@^4.0.1: version "4.0.1" resolved "https://registry.yarnpkg.com/get-stdin/-/get-stdin-4.0.1.tgz#b968c6b0a04384324902e8bf1a5df32579a450fe" @@ -4845,6 +4874,10 @@ path-type@^1.0.0: pify "^2.0.0" pinkie-promise "^2.0.0" +pathval@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/pathval/-/pathval-1.1.0.tgz#b942e6d4bde653005ef6b71361def8727d0645e0" + pify@^2.0.0: version "2.3.0" resolved "https://registry.yarnpkg.com/pify/-/pify-2.3.0.tgz#ed141a6ac043a849ea588498e7dca8b15330e90c" @@ -5936,6 +5969,10 @@ type-check@~0.3.2: dependencies: prelude-ls "~1.1.2" +type-detect@^4.0.0, type-detect@^4.0.5: + version "4.0.8" + resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-4.0.8.tgz#7646fb5f18871cfbb7749e69bd39a6388eb7450c" + type-is@~1.6.15, type-is@~1.6.16: version "1.6.16" resolved "https://registry.yarnpkg.com/type-is/-/type-is-1.6.16.tgz#f89ce341541c672b25ee7ae3c73dee3b2be50194"