vm: avoid repeated microtask mode parsing

Parse microtaskMode only in createContext() so convenience APIs do not
read type and queue getters repeatedly.

Add regression tests for getter access, invalid options, and shared
manual queues during module evaluation. Clarify that the queue
constructor is not exported and fix the C++ macro line length.

Refs: #65555

Signed-off-by: leah-1ee <selee3196@gmail.com>

Author: leah-1ee

doc/api/vm.md

@@ -1341,8 +1341,8 @@ share where their microtasks (`Promise` reactions and `async function`
 continuations) are placed, and so that those microtasks are drained together,
 explicitly, by the embedder, instead of automatically by Node.js.
 
-Instances are created with [`vm.createMicrotaskQueue()`][]; there is no
-public constructor.
+Instances are created with [`vm.createMicrotaskQueue()`][]; the constructor is
+not exported by the `node:vm` module.
 
 ### `microtaskQueue.runMicrotasks()`
 

lib/vm.js

@@ -263,7 +263,6 @@ function getContextOptions(options) {
       validateBoolean(wasm, 'options.contextCodeGeneration.wasm');
     contextOptions.codeGeneration = { strings, wasm };
   }
-  getMicrotaskModeOptions(options.microtaskMode);
   return contextOptions;
 }
 

src/env_properties.h

@@ -437,7 +437,7 @@
   V(lock_info_template, v8::DictionaryTemplate)                                \
   V(lock_query_template, v8::DictionaryTemplate)                               \
   V(message_port_constructor_template, v8::FunctionTemplate)                   \
-  V(microtask_queue_constructor_template, v8::FunctionTemplate)                 \
+  V(microtask_queue_constructor_template, v8::FunctionTemplate)                \
   V(module_wrap_constructor_template, v8::FunctionTemplate)                    \
   V(mx_record_template, v8::DictionaryTemplate)                                \
   V(naptr_record_template, v8::DictionaryTemplate)                             \

test/parallel/test-vm-shared-microtask-queue-module.js

@@ -0,0 +1,57 @@
+// Flags: --experimental-vm-modules
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const vm = require('vm');
+
+(async () => {
+  const queue = vm.createMicrotaskQueue();
+  const trace = [];
+  const record = (entry) => trace.push(entry);
+  const microtaskMode = { type: 'manual', queue };
+  const contextA = vm.createContext({ record }, { microtaskMode });
+  const contextB = vm.createContext({ record }, { microtaskMode });
+  const moduleA = new vm.SourceTextModule(`
+    await Promise.resolve();
+    record('a');
+    Promise.resolve().then(() => record('a-follow-up'));
+  `, { context: contextA });
+  const moduleB = new vm.SourceTextModule(`
+    await Promise.resolve();
+    record('b');
+    Promise.resolve().then(() => record('b-follow-up'));
+  `, { context: contextB });
+
+  await moduleA.link(common.mustNotCall());
+  await moduleB.link(common.mustNotCall());
+
+  const evaluationA = moduleA.evaluate();
+  const evaluationB = moduleB.evaluate();
+  let completed = 0;
+  let resolveCompleted;
+  const allCompleted = new Promise((resolve) => {
+    resolveCompleted = resolve;
+  });
+  const onCompleted = () => {
+    if (++completed === 2) resolveCompleted();
+  };
+  evaluationA.then(common.mustCall(() => {
+    onCompleted();
+  }));
+  evaluationB.then(common.mustCall(() => {
+    onCompleted();
+  }));
+
+  assert.deepStrictEqual(trace, []);
+  queue.runMicrotasks();
+  assert.deepStrictEqual(trace, [
+    'a',
+    'b',
+    'a-follow-up',
+    'b-follow-up',
+  ]);
+
+  await allCompleted;
+  assert.strictEqual(completed, 2);
+})().then(common.mustCall());

test/parallel/test-vm-shared-microtask-queue-validation.js

@@ -8,14 +8,24 @@ const fake = { __proto__: Object.getPrototypeOf(queue) };
 const proxy = new Proxy(queue, {});
 
 const script = new vm.Script('');
-for (const invalid of [fake, proxy, {}, null, undefined, 1, 'queue']) {
-  const options = { microtaskMode: { type: 'manual', queue: invalid } };
-  for (const create of [
-    () => vm.createContext({}, options),
-    () => vm.runInNewContext('', {}, options),
-    () => script.runInNewContext({}, options),
-  ]) {
-    assert.throws(create, { code: 'ERR_INVALID_ARG_TYPE' });
+const createContext = (options) => vm.createContext({}, options);
+const runInNewContext = (options) => vm.runInNewContext('', {}, options);
+const scriptRunInNewContext =
+  (options) => script.runInNewContext({}, options);
+const contextCreators = [
+  createContext,
+  runInNewContext,
+  scriptRunInNewContext,
+];
+
+for (const create of contextCreators) {
+  assert.throws(() => {
+    create({ microtaskMode: { type: 'automatic', queue } });
+  }, { code: 'ERR_INVALID_ARG_VALUE' });
+
+  for (const invalid of [fake, proxy, {}, null, undefined, 1, 'queue']) {
+    const options = { microtaskMode: { type: 'manual', queue: invalid } };
+    assert.throws(() => create(options), { code: 'ERR_INVALID_ARG_TYPE' });
   }
 }
 

test/parallel/test-vm-shared-microtask-queue.js

@@ -42,9 +42,8 @@ const vm = require('vm');
   assert.notStrictEqual(queue, other);
 }
 
-// `vm.MicrotaskQueue` (the constructor) is intentionally not exposed: only
-// `vm.createMicrotaskQueue()` is public, so there is no way to construct or
-// name the class directly from user code.
+// The constructor is intentionally not exported from `vm`; the public API for
+// creating queues is `vm.createMicrotaskQueue()`.
 assert.strictEqual(vm.MicrotaskQueue, undefined);
 
 // `microtaskQueue.runMicrotasks()` throws when called on an unrelated `this`.
@@ -129,28 +128,69 @@ assert.strictEqual(vm.MicrotaskQueue, undefined);
   assert.deepStrictEqual(trace, ['a', 'b']);
 }
 
-// `vm.runInNewContext()` and `script.runInNewContext()` also accept the
-// object form (they build their context via `getContextOptions()` +
-// `createContext()`, both of which now go through `getMicrotaskModeOptions()`).
+// All context-creation APIs parse the object form once. In particular, they
+// must not read either property a second time after validation.
 {
-  const queue = vm.createMicrotaskQueue();
-  const trace = [];
-  const record = (entry) => trace.push(entry);
-
-  vm.runInNewContext(
-    "Promise.resolve().then(() => record('runInNewContext'));",
-    { record },
-    { microtaskMode: { type: 'manual', queue } },
+  const script = new vm.Script(
+    "Promise.resolve().then(() => record('script.runInNewContext'));",
   );
-  assert.deepStrictEqual(trace, []);
-  queue.runMicrotasks();
-  assert.deepStrictEqual(trace, ['runInNewContext']);
-
-  const script = new vm.Script("Promise.resolve().then(() => record('script.runInNewContext'));");
-  script.runInNewContext({ record }, { microtaskMode: { type: 'manual', queue } });
-  assert.deepStrictEqual(trace, ['runInNewContext']);
-  queue.runMicrotasks();
-  assert.deepStrictEqual(trace, ['runInNewContext', 'script.runInNewContext']);
+  const createAndRun = [
+    {
+      expected: 'createContext',
+      run(sandbox, options) {
+        const context = vm.createContext(sandbox, options);
+        vm.runInContext(
+          "Promise.resolve().then(() => record('createContext'));",
+          context,
+        );
+      },
+    },
+    {
+      expected: 'runInNewContext',
+      run(sandbox, options) {
+        vm.runInNewContext(
+          "Promise.resolve().then(() => record('runInNewContext'));",
+          sandbox,
+          options,
+        );
+      },
+    },
+    {
+      expected: 'script.runInNewContext',
+      run(sandbox, options) {
+        script.runInNewContext(sandbox, options);
+      },
+    },
+  ];
+
+  for (const { expected, run } of createAndRun) {
+    const queue = vm.createMicrotaskQueue();
+    const trace = [];
+    const record = (entry) => trace.push(entry);
+    let typeGetCount = 0;
+    let queueGetCount = 0;
+    const microtaskMode = {
+      get type() {
+        if (++typeGetCount > 1) {
+          throw new Error('microtaskMode.type was read more than once');
+        }
+        return 'manual';
+      },
+      get queue() {
+        if (++queueGetCount > 1) {
+          throw new Error('microtaskMode.queue was read more than once');
+        }
+        return queue;
+      },
+    };
+
+    run({ record }, { microtaskMode });
+    assert.strictEqual(typeGetCount, 1);
+    assert.strictEqual(queueGetCount, 1);
+    assert.deepStrictEqual(trace, []);
+    queue.runMicrotasks();
+    assert.deepStrictEqual(trace, [expected]);
+  }
 }
 
 // `vm.constants.DONT_CONTEXTIFY` sandboxes (no wrapper object, the V8