GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — module: fix the leak in SourceTextModule and ContextifySript · nodejs/node@b0ce78a

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

src
test/es-module
- test-vm-contextified-script-leak.js
- test-vm-source-text-module-leak.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎src/module_wrap.cc‎`

Copy file name to clipboardExpand all lines: src/module_wrap.cc

+6-3Lines changed: 6 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,10 @@ ModuleWrap::ModuleWrap(Environment* env,`
`55`	`55`	`Local<String> url,`
`56`	`56`	`Local<Object> context_object,`
`57`	`57`	`Local<Value> synthetic_evaluation_step)`
`58`		`- : BaseObject(env, object), module_(env->isolate(), module) {`
	`58`	`+ : BaseObject(env, object),`
	`59`	`+ module_(env->isolate(), module),`
	`60`	`+ module_hash_(module->GetIdentityHash()) {`
	`61`	`+ object->SetInternalFieldForNodeCore(kModuleSlot, module);`
`59`	`62`	`object->SetInternalField(kURLSlot, url);`
`60`	`63`	`object->SetInternalField(kSyntheticEvaluationStepsSlot,`
`61`	`64`	`synthetic_evaluation_step);`
`@@ -65,12 +68,12 @@ ModuleWrap::ModuleWrap(Environment* env,`
`65`	`68`	`synthetic_ = true;`
`66`	`69`	`}`
`67`	`70`	`MakeWeak();`
	`71`	`+ module_.SetWeak();`
`68`	`72`	`}`
`69`	`73`
`70`	`74`	`ModuleWrap::~ModuleWrap() {`
`71`	`75`	`HandleScope scope(env()->isolate());`
`72`		`- Local<Module> module = module_.Get(env()->isolate());`
`73`		`- auto range = env()->hash_to_module_map.equal_range(module->GetIdentityHash());`
	`76`	`+ auto range = env()->hash_to_module_map.equal_range(module_hash_);`
`74`	`77`	`for (auto it = range.first; it != range.second; ++it) {`
`75`	`78`	`if (it->second == this) {`
`76`	`79`	`env()->hash_to_module_map.erase(it);`

Collapse file

`‎src/module_wrap.h‎`

Copy file name to clipboardExpand all lines: src/module_wrap.h

+2-1Lines changed: 2 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ enum HostDefinedOptions : int {`
`33`	`33`	`class ModuleWrap : public BaseObject {`
`34`	`34`	`public:`
`35`	`35`	`enum InternalFields {`
`36`		`- kModuleWrapBaseField = BaseObject::kInternalFieldCount,`
	`36`	`+ kModuleSlot = BaseObject::kInternalFieldCount,`
`37`	`37`	`kURLSlot,`
`38`	`38`	`kSyntheticEvaluationStepsSlot,`
`39`	`39`	`kContextObjectSlot, // Object whose creation context is the target Context`
`@@ -106,6 +106,7 @@ class ModuleWrap : public BaseObject {`
`106`	`106`	`contextify::ContextifyContext* contextify_context_ = nullptr;`
`107`	`107`	`bool synthetic_ = false;`
`108`	`108`	`bool linked_ = false;`
	`109`	`+ int module_hash_;`
`109`	`110`	`};`
`110`	`111`
`111`	`112`	`} // namespace loader`

Collapse file

`‎src/node_contextify.cc‎`

Copy file name to clipboardExpand all lines: src/node_contextify.cc

+4Lines changed: 4 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -855,7 +855,11 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {`
`855`	`855`	`"ContextifyScript::New");`
`856`	`856`	`return;`
`857`	`857`	`}`
	`858`	`+`
`858`	`859`	`contextify_script->script_.Reset(isolate, v8_script);`
	`860`	`+ contextify_script->script_.SetWeak();`
	`861`	`+ contextify_script->object()->SetInternalFieldForNodeCore(kUnboundScriptSlot,`
	`862`	`+ v8_script);`
`859`	`863`
`860`	`864`	`std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;`
`861`	`865`	`if (produce_cached_data) {`

Collapse file

`‎src/node_contextify.h‎`

Copy file name to clipboardExpand all lines: src/node_contextify.h

+5Lines changed: 5 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -128,6 +128,11 @@ class ContextifyContext : public BaseObject {`
`128`	`128`
`129`	`129`	`class ContextifyScript : public BaseObject {`
`130`	`130`	`public:`
	`131`	`+ enum InternalFields {`
	`132`	`+ kUnboundScriptSlot = BaseObject::kInternalFieldCount,`
	`133`	`+ kInternalFieldCount`
	`134`	`+ };`
	`135`	`+`
`131`	`136`	`SET_NO_MEMORY_INFO()`
`132`	`137`	`SET_MEMORY_INFO_NAME(ContextifyScript)`
`133`	`138`	`SET_SELF_SIZE(ContextifyScript)`

Collapse file

`‎test/es-module/test-vm-contextified-script-leak.js‎`

Copy file name to clipboard

+19Lines changed: 19 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,19 @@`
	`1`	`+// Flags: --max-old-space-size=16 --trace-gc`
	`2`	`+'use strict';`
	`3`	`+`
	`4`	`+// This tests that vm.Script with dynamic import callback does not leak.`
	`5`	`+// See: https://github.com/nodejs/node/issues/33439`
	`6`	`+require('../common');`
	`7`	`+const vm = require('vm');`
	`8`	`+let count = 0;`
	`9`	`+`
	`10`	`+function main() {`
	`11`	`+ // Try to reach the maximum old space size.`
	`12`	+ new vm.Script(`"${Math.random().toString().repeat(512)}";`, {
	`13`	`+ async importModuleDynamically() {},`
	`14`	`+ });`
	`15`	`+ if (count++ < 2 * 1024) {`
	`16`	`+ setTimeout(main, 1);`
	`17`	`+ }`
	`18`	`+}`
	`19`	`+main();`

Collapse file

`‎test/es-module/test-vm-source-text-module-leak.js‎`

Copy file name to clipboard

+23Lines changed: 23 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,23 @@`
	`1`	`+// Flags: --experimental-vm-modules --max-old-space-size=16 --trace-gc`
	`2`	`+'use strict';`
	`3`	`+`
	`4`	`+// This tests that vm.SourceTextModule() does not leak.`
	`5`	`+// See: https://github.com/nodejs/node/issues/33439`
	`6`	`+require('../common');`
	`7`	`+`
	`8`	`+const vm = require('vm');`
	`9`	`+let count = 0;`
	`10`	`+async function createModule() {`
	`11`	`+ // Try to reach the maximum old space size.`
	`12`	+ const m = new vm.SourceTextModule(`
	`13`	`+ const bar = new Array(512).fill("----");`
	`14`	`+ export { bar };`
	`15`	+`);
	`16`	`+ await m.link(() => {});`
	`17`	`+ await m.evaluate();`
	`18`	`+ if (count++ < 4096) {`
	`19`	`+ setTimeout(createModule, 1);`
	`20`	`+ }`
	`21`	`+ return m;`
	`22`	`+}`
	`23`	`+createModule();`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.