-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
module: fix leak of vm.SyntheticModule
Previously we maintain a strong persistent reference to the ModuleWrap to retrieve the ID-to-ModuleWrap mapping from the HostImportModuleDynamicallyCallback using the number ID stored in the host-defined options. As a result the ModuleWrap would be kept alive until the Environment is shut down, which would be a leak for user code. With the new symbol-based host-defined option we can just get the ModuleWrap from the JS-land WeakMap so there's now no need to maintain this strong reference. This would at least fix the leak for vm.SyntheticModule. vm.SourceTextModule is still leaking due to the strong persistent reference to the v8::Module.
- Loading branch information
1 parent
aa328cb
commit 1751348
Showing
3 changed files
with
48 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
'use strict'; | ||
|
||
// Flags: --experimental-vm-modules --max-heap-size=20 | ||
// This tests that vm.SyntheticModule does not leak. | ||
// See https://github.com/nodejs/node/issues/44211 | ||
|
||
require('../common'); | ||
const vm = require('vm'); | ||
|
||
let count = 0; | ||
async function createModule() { | ||
const m = new vm.SyntheticModule(['bar'], () => { | ||
m.setExport('bar', new Date().toISOString().repeat(1e6)); | ||
}); | ||
await m.link(() => {}); | ||
await m.evaluate(); | ||
count++; | ||
if (count < 30000) { | ||
setImmediate(createModule); | ||
} | ||
return m; | ||
} | ||
|
||
createModule(); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
'use strict'; | ||
|
||
// Flags: --experimental-vm-modules --max-heap-size=20 | ||
// This tests that using vm.compileFunction() with vm.SyntheticModule in | ||
// the dynamic import callback does not leak. | ||
// See https://github.com/nodejs/node/issues/44211 | ||
|
||
require('../common'); | ||
const vm = require('vm'); | ||
|
||
let count = 0; | ||
async function createModule() { | ||
const m = new vm.SourceTextModule('export default "hello".repeat(1e5)'); | ||
await m.link(() => {}); | ||
await m.evaluate(); | ||
count++; | ||
if (count < 30000) { | ||
setImmediate(createModule); | ||
} | ||
return m; | ||
} | ||
|
||
createModule(); |