Skip to content

SourceTextModule memory leak #33439

Description

@grosto
  • 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.
image

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

  1. added
    vmIssues and PRs related to the vm subsystem.
    on May 16, 2020
  2. devsnek commented on May 16, 2020

    @devsnek
    Member

    Yeah modules can't really be individually collected, the entire context they're in has to be collected.

  3. grosto commented on May 17, 2020

    @grosto
    Author

    I am not sure I understand what you mean by that. Is this expected behaviour?
    Even with contextified object issue still persists.

     const context = createContext({});
     new SourceTextModule('1 + 1', { context });
  4. guybedford commented on May 17, 2020

    @guybedford
    Contributor

    @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?

  5. devsnek commented on May 17, 2020

    @devsnek
    Member

    @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.

  6. justinfagnani commented on Jun 15, 2020

    @justinfagnani

    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 SourceTextModule that'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.

  7. devsnek commented on Jun 15, 2020

    @devsnek
    Member
  8. justinfagnani commented on Jun 16, 2020

    @justinfagnani

    @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.

  9. devsnek commented on Jun 16, 2020

    @devsnek
    Member

    @justinfagnani

    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?

    No. It sounds like you're hitting v8:9968.

  10. maslow commented on Jul 22, 2021

    @maslow

    same problem + 1.
    i use it in HTTP request, memory growing & growing in every request.

  11. caub commented on Apr 30, 2022

    @caub

    @maslow you could memoize it so that calling SourceTextModule with the same sourcecode returns the same instance

  12. elglogins commented on Sep 13, 2023

    @elglogins

    Any updates on this? :(

  13. bnoordhuis commented on Sep 13, 2023

    @bnoordhuis
    Member

    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.

  14. Havunen commented on Oct 5, 2023

    @Havunen

    @joyeecheung Is there a possibility you could look into this issue please.

  15. joyeecheung commented on Oct 5, 2023

    @joyeecheung
    Member

    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.

  16. Dustin4444 commented on Feb 15, 2026

    @Dustin4444
    • 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. image

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    vmIssues and PRs related to the vm subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions