src: remove usages on ScriptCompiler::CompileFunctionInContext

[legendecas]

V8 APIs like HostImportModuleDynamicallyCallback and
ScriptCompiler::CompileFunction are moving away from ScriptOrModule.

Replaces ScriptCompiler::CompileFunctionInContext with
ScriptCompiler::CompileFunction to remove the usages on the optional
out param ScriptOrModule.

Fixes: nodejs/node-v8#214

[nodejs-github-bot]

Review requested:

• @nodejs/startup

[targos]

Fixes nodejs/node-v8#214 ?

[targos]

Why is it draft?

[legendecas]

@targos: Why is it draft?

I was verifying that the lifetime of CompiledFnEntry is still correct with this change. But I found that even without this change, the following script can cause an OOM quite quickly:

const vm = require('vm');

function work() {
  const context = vm.createContext({});
  const fn = vm.compileFunction(`
    import('foo').then(() => {});
  `, [], {
    parsingContext: context,
    importModuleDynamically: async (specifier, fn, importAssertions) => {
      const m = new vm.SyntheticModule(['x'], () => {
        m.setExport('x', 1);
      }, {
        context,
      });

      await m.link(() => {});
      await m.evaluate();

      return m;
    },
  });

  fn();
}

(function main() {
  work()
  // yielding to give chance to the evaluation of promises.
  setTimeout(main, 1);
})();

The reference in the importModuleDynamically to the created function object https://github.com/nodejs/node/blob/main/lib/vm.js#L389 is invalidating the weak reference in CompiledFnEntry.

As this change is not worsening the condition, I'm marking it ready for review in order to land https://chromium-review.googlesource.com/c/v8/v8/+/3172764, with which we can get rid of the current id-based host-defined-options lookups.

(Opened issue #44211 to track this problem)

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/46007/

[legendecas]

@nodejs/vm would you mind taking a look at this PR? thank you!

[legendecas]

Landed in eefe553. Thank you for the review!