Repository navigation
SourceTextModule memory leak #33439
Description
- Version: v14.2.0
- Platform: Darwin Kernel Version 19.4.0
- Subsystem: vm
What steps will reproduce the bug?
run this snippet with --experimental-vm-modules
const {SourceTextModule} = require('vm');
let grow = () => {
new SourceTextModule(`1 + 1`);
setTimeout(grow, 100)
}
grow()How often does it reproduce? Is there a required condition?
Reproducible on every run on my machine.
What is the expected behavior?
SourceTextModule should be cleaned up by GC
What do you see instead?
The SourceTextModule instances never get cleaned up.
Additional information
I think issue is that ModuleWrap is used as a key in WeakMap in vm module, but I am not really familiar with this codebase.
Activity
devsnek commentedon May 16, 2020
Yeah modules can't really be individually collected, the entire context they're in has to be collected.
grosto commentedon May 17, 2020
I am not sure I understand what you mean by that. Is this expected behaviour?
Even with contextified object issue still persists.
guybedford commentedon May 17, 2020
@devsnek are there steps we can take to improve module collection for v8? Or do we have to wait for concepts like compartments to drive this work instead?
devsnek commentedon May 17, 2020
@guybedford atm the api (on our side and on v8's side) is architected to not really care about gc'ing modules because they tend to have to live for the lifetime of an application anyway. If we want to support this, we have to basically go through everywhere we ref a module in node and in v8 and ensure they're collectable on a per-module or per-context basis.
As you also brought up, compartments sort of replace the entire vm api anyway, and implementations will have to ensure those can be collected properly, so we could also choose to do nothing.
justinfagnani commentedon Jun 15, 2020
Possibly related to this, is Node caching SourceTextModules based on their identifier?
We have a loader with its own cache and it seems like sometimes we'll get a module back from
new SourceTextModulethat's already been linked and eval'ed. We're still working on reducing the issue though.This is for a server that's creating a fresh VM context for every request, so we want a new module graph and cache. We've been assuming that Node does no caching itself.
devsnek commentedon Jun 15, 2020
@justinfagnani possibly v8:9968?
justinfagnani commentedon Jun 16, 2020
@devsnek I honestly can't tell if that's related. From that issue, always invalidating the cache seems like what we want - ie, we're taking care of the caching in our loader and if we call
new SourceTextModule()it's because we definitely want a new module.I guess I don't know what the contract is for SourceTextModule. Should reusing the same source and identifier ever result in a module that's already linked? Even if the linker may resolve imports differently? Should we be trying to vary the identifiers so that they're unique across contexts we create?
edit: I'm asking here only because I'm not sure if we should file a new issue yet.
devsnek commentedon Jun 16, 2020
@justinfagnani
No. It sounds like you're hitting v8:9968.
maslow commentedon Jul 22, 2021
same problem + 1.
i use it in HTTP request, memory growing & growing in every request.
caub commentedon Apr 30, 2022
@maslow you could memoize it so that calling
SourceTextModulewith the same sourcecode returns the same instanceelglogins commentedon Sep 13, 2023
Any updates on this? :(
bnoordhuis commentedon Sep 13, 2023
Needs someone to come up with a pull request. It should be safe to remove the module from the WeakMap once the "import meta object" and importModuleDynamically() steps have run.
You're still going to have a bad time if you don't link and evaluate the module like in OP's example but as evaluating them is kind of the point of modules that doesn't seem like a big deal.
Havunen commentedon Oct 5, 2023
@joyeecheung Is there a possibility you could look into this issue please.
joyeecheung commentedon Oct 5, 2023
The issue in the OP has already been fixed by #48510 (which included a similar test, and, if you take a heap snapshot, you can see that the SourceTextModule can be GC'ed). If you are coming from jestjs/jest#12205, I think you may need a separate issue with a different minimal repro that only uses Node.js.
Dustin4444 commentedon Feb 15, 2026