-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SourceTextModule leaks memory when context is used #50113
Comments
Hmm after further testing it seems that setting context to null doesnt fully fix the issue, but it does not go OOM every time I run the program, but more like 90% of the time? Or maybe there is some flakiness in the test case |
@joyeecheung is this something you could take a look at? I ran the test using latest nightly version which should include all the merged fixes. |
From a quick glance I think it's the resolve cache causing the leak, you don't actually need the context, it just causes the process to crash sooner because contexts are big and add more pressure, you can still reproduce the OOM with Off the top of my head, moving the resolve cache to JS land may fix the leak (I don't actually know if it's feasible to move it to JS land, however). |
Yep even this goes OOM:
import {SourceTextModule, SyntheticModule} from 'vm';
async function link() {
const subModule = new SourceTextModule(`
import asd from 'asd.json';
export default asd;
`, {
identifier: 'foo'
})
const jsonModule = new SyntheticModule(
['default'],
function () {
// mimic some large file
this.setExport('default', {a: new Array(1000).fill('-')});
},
{
identifier: 'asd.json'
}
)
await subModule.link(() => jsonModule)
await subModule.evaluate();
return subModule;
}
for (let i = 0; i < 100000; i++) {
if (i % 100 === 0) {
console.log("iterations" + i)
}
await link();
}
I also tested with a single SyntheticModule but I believe that does not leak. Single SourceTextModule however seems like it does?? import {SourceTextModule} from 'vm';
async function link() {
const subModule = new SourceTextModule(`
export default {a: new Array(1000).fill('-')}
`, {
identifier: 'foo'
})
await subModule.link(() => undefined);
await subModule.evaluate();
return subModule;
}
for (let i = 0; i < 100000; i++) {
if (i % 100 === 0) {
console.log("iterations" + i)
}
await link();
} |
I compiled NodeJS from sources with
If we ignore all the memory leak errors and warnings before 2000 iterations of edit: ah... Seems I need to provide |
test.js import {SourceTextModule} from 'vm';
async function link() {
const subModule = new SourceTextModule(`
export default {a: new Array(1000).fill('-')}
`, {
identifier: 'foo'
})
await subModule.link(() => undefined);
await subModule.evaluate();
return subModule;
}
for (let i = 0; i < 100000; i++) {
if (i % 100 === 0) {
console.log("iterations" + i)
}
await link();
} command: output (ignoring first 500 iterations) This error stack looks suspicious to me:
It includes |
Version
21.0.0-nightly20231008fce8fbadcd
Platform
Microsoft Windows NT 10.0.22621.0 x64
Subsystem
No response
What steps will reproduce the bug?
Create two or more
SourceTextModules
and aSyntheticModule
and link them together using a shared context (vm.createContext()
)https://github.com/Havunen/nodejs-memory-leak
How often does it reproduce? Is there a required condition?
100%
What is the expected behavior? Why is that the expected behavior?
NodeJs should not leak memory because the context object goes out of scope and should get garbage collected and all the modules associated with it.
What do you see instead?
The memory grows and grows depending on a number of modules and size of their data until nodejs fails to OOM.
Additional information
Note: Manually setting the context object null in JS side fixes the leak, but that should not be required
The text was updated successfully, but these errors were encountered: