Skip to content
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

Linking cycle with vm.SourceTextModule #3535

Closed
mrbbot opened this issue Sep 9, 2021 · 2 comments
Closed

Linking cycle with vm.SourceTextModule #3535

mrbbot opened this issue Sep 9, 2021 · 2 comments

Comments

@mrbbot
Copy link

mrbbot commented Sep 9, 2021

Version

v16.9.0

Platform

Darwin MrBBots-MBP 19.6.0 Darwin Kernel Version 19.6.0: Thu May 6 00:48:39 PDT 2021; root:xnu-6153.141.33~1/RELEASE_X86_64 x86_64

Subsystem

vm

What steps will reproduce the bug?

With the import graph A -> B <-> C, vm.SourceTextModule#link ends up in a cycle.
See reproduction here: https://github.com/mrbbot/cycles-issue.

How often does it reproduce? Is there a required condition?

No required condition, happening all the time.

What is the expected behavior?

The link callback is called once per module.

What do you see instead?

The link callback is called infinitely many times, in a cycle between modules B andC.

Additional information

Discovered when trying to run the example from https://blog.cloudflare.com/workers-rust-sdk/ with Miniflare.

/cc @jasnell

@Jamesernator
Copy link

Jamesernator commented Sep 9, 2021

The behaviour in Node is correct, although admittedly it is a bit confusing, I think what you're expecting is that because two modules have the same identifier the same module should be returned, but actually the identifier is just a thing for tracebacks, you can have many modules with the same identifier.

If you want it to actually link in a cycle you need to return the same module object, i.e. you'll need to cache and return the same object.

Here's a simple path loader as an example:

import path from "path";
import vm from "vm";
import fs from "fs/promises";

const BASE_PATH = process.cwd();
const moduleMap = new Map();

async function resolveModule(specifier, parent=null) {
    const modPath = path.normalize(path.join(
        parent?.identifier ?? BASE_PATH,
        specifier,
    ));
    if (moduleMap.has(modPath)) {
        return moduleMap.get(modPath);
    }
    const source = await fs.readFile(modPath, "utf8");
    const module = new vm.SourceTextModule(source, {
        identifier: modPath,
    });
    moduleMap.set(modPath, module);
    return module;
}

const entry = await resolveModule('./path/to/entry.js');
await entry.link(resolveModule);
await entry.evaluate();

@devsnek devsnek transferred this issue from nodejs/node Sep 9, 2021
@mrbbot
Copy link
Author

mrbbot commented Sep 10, 2021

Thank you very much for your quick response and the code example! 😃
All working now. 👍

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

No branches or pull requests

3 participants