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

importModuleDynamically for vm module is cached #36351

Closed
Tracked by #37648
takerusilt opened this issue Dec 2, 2020 · 4 comments
Closed
Tracked by #37648

importModuleDynamically for vm module is cached #36351

takerusilt opened this issue Dec 2, 2020 · 4 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. vm Issues and PRs related to the vm subsystem.

Comments

@takerusilt
Copy link

  • Version: v14.15.1, v15.3.0
  • Platform: Windows 10 Pro 64-bit / Mac Darwin Kernel Version 19.6.0: Thu Oct 29 22:56:45 PDT 2020; root:xnu-6153.141.2.2~1/RELEASE_X86_64 x86_64
  • Subsystem: vm

What steps will reproduce the bug?

const vm = require('vm');

var counter = 0;

async function link() {
    var thisCounter = ++counter;
    var module = new vm.SourceTextModule(`
        const foo = (await import('foo')).default;
        console.log('import.meta.counter', import.meta.counter);
        console.log('foo.counter', foo.counter);
    `, {
        identifier: '',
        importModuleDynamically: async () => {
            console.log('importModuleDynamically', thisCounter);
            const module = new vm.SyntheticModule(['default'], function() {
              this.setExport('default', { counter: thisCounter });
            });
            await module.link(()=>{});
            await module.evaluate();
            return module;
        },
        initializeImportMeta: (meta) => {
            meta.counter = thisCounter
        }
    });
    await module.link(() => {});
    await module.evaluate();
    return module;
}

link();
link();

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

Always.

What is the expected behavior?

importModuleDynamically 1
importModuleDynamically 2
import.meta.counter 1
foo.counter 1
import.meta.counter 2
foo.counter 2

What do you see instead?

importModuleDynamically 1
importModuleDynamically 1
import.meta.counter 1
foo.counter 1
import.meta.counter 2
foo.counter 1

Additional information

vm.SourceTextModule is caching the importModuleDynamically callback as long as the source code is the same, where as initializeImportMeta does not.

If how importModuleDynamically does its work depends on an external state, often in test suite, it will load modules inconsistently to those state and cause discrepancies as viewed inside the VM modules.

@targos
Copy link
Member

targos commented Dec 6, 2020

@nodejs/modules

@targos targos added the esm Issues and PRs related to the ECMAScript Modules implementation. label Dec 6, 2020
@devsnek
Copy link
Member

devsnek commented Dec 6, 2020

this is a bug in v8 that they have been very unenthusiastic about fixing.

@targos targos added the vm Issues and PRs related to the vm subsystem. label Dec 6, 2020
@avivkeller
Copy link
Member

In Node.js v22 this is no longer the case AFAICT:

importModuleDynamically 1
importModuleDynamically 2
import.meta.counter 1
foo.counter 1
import.meta.counter 2
foo.counter 2

@joyeecheung
Copy link
Member

The host defined options leak in the compilation cache has already been fixed by V8. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants