[robobun]

Related to #866, #492

Problem

• After node:vm's timeout cuts a module load, a later import() never settles. On main with [JSC] Test hooks for a termination inside a module load: $vm.callWithTerminationAtTrapCheck(), $vm.moduleLoaderLoadSync(), setModuleLoaderFetchHook() #975: two, trap check 25, later loads 0: a later import() is still pending.
• JSPromise::rejectWithCaughtException() leaves the promise of the cut load step pending, and the registry entry keeps answering with it.
• MicrotaskQueue::drainImpl() clears the queue when a job ends with a termination. Every load in flight loses its jobs.

Fix

• The loader's jobs are set aside at the cut, and VM::cancelTermination() puts them back on the VM's queue.
• The cut step ends its own load: its entry leaves the registry or gives up its load promise, and so does each load that waits for it (abandonLoadsWaitingFor()).
• An import() that waited asks the host again. A require() that was cut is not evaluated later.
• Verified: four module-load-cut-short-by-termination*.js tests, testaot native module-programs.

Background

• A termination begins at any exception check (RETURN_IF_EXCEPTION polls for it). VM::cancelTermination() withdraws it.
• A registry entry has a module promise (the record) and a load promise (the record with its imports). One job settles each.
• Considered: run the cut job again. That calls the cut fetch hook with no limit.

Downsides

• An import() whose own job the termination cut stays pending, as on main. A module that awaits it never finishes.
• The jobs that are put back evaluate modules after the time limit. A VM with compiled code (--aot) is unchanged.
• .text +8,256 B. A load that meets no termination: 50,159 to 50,180 instructions per fresh import().

Notes

Draft until a maintainer answers questions 1 and 2 in the comment below. They also decide #866.

Stack. #975 (test hooks), #976 (link cleanup), then this PR. Only the last commit is new.

Who can reach it. There is no user report and no issue. Two routes reach the hang on released Bun:

• a require() of an ES module graph that a node:vm timeout or breakOnSigint cuts in its load phase, then an import() of the same graph;
• a host call under the time limit that runs the checkpoint of the VM's queue while a load is in flight.

An import() alone under the limit did not reach it. Node v26.3.0 fails the first route with ERR_INTERNAL_ASSERTION. oven-sh/bun#30598 and oven-sh/bun#33411 would add callers of VM::cancelTermination(), and both are stale.

What changed in this revision (the third). Self-reviewed a second time: 43 concerns raised. Addressed in the engine:

• The keep rule is at the withdrawal now. MicrotaskQueue::clearAfterTermination() sets the loader's jobs aside, and VM::cancelTermination() puts them back. A termination that nobody withdraws drops them as on main. It applies to the default queue of the VM only, and not in a VM that runs compiled code.
• abandonLoadsWaitingFor() hands an import() that waited back to the queue, where it asks again. The job of a load that evaluates (a synchronous load, an entry point) is dropped.
• askAgainForAbandonedLoad() could get no promise from the host for a module of a cycle (ASSERT(loadedPromise), a null dereference in release). A step that waited for the requests of its module now loads them again itself, and the other steps drop the stale row of [[LoadedModules]] first.
• moduleLoadTopRejected() takes the ThrowScope (the arm stopped a run with --validateExceptionChecks=true).
• moduleGraphLoadingError() is the upstream line again, with no trap check in front of the rejection.
• A synchronous load hands its jobs to the synchronous load that it runs inside, if there is one.
• flags() is under the guard where [JSC] A load by name starts again when clearAll() overtook its fetch #853 adds the same accessor, so the two conflict in text.
• The tests: a graph whose root awaits its own import(), two import() in flight, a second cut at three counts, a synchronous later load, the final state of each import() that was in flight, jobs of script that stay dropped, and a nested test that runs the ask-again arms through setModuleLoaderFetchHook() of [JSC] Test hooks for a termination inside a module load: $vm.callWithTerminationAtTrapCheck(), $vm.moduleLoaderLoadSync(), setModuleLoaderFetchHook() #975.

Not addressed here, and needed in the Bun PR that takes this engine:

• moveLoadedModule() and keepPreloadModule in BunPlugin.cpp read "record and no pending load" as loaded. An abandoned entry is in that state. They must ask entry->isLoaded().
• The steps of a load that Bun owns (the completion of an async onLoad, Bun__onFulfillAsyncModule) have no exit for a termination. They can call makeModuleWasCutShort().
• A repeated require() after a cut returns {} from Bun's require map (require(): remove the cache entry of a built-in whose load a node:vm timeout ended bun#47543 lists it as not fixed).
• Bun's reactions on loader promises can get the loader cell as the rejection value.
• A cut require() of an ES module that imports a CommonJS module: a later import() rejects at 22 of 210 cut positions (Failed to evaluate module, Export named 'x' not found). evaluateCommonJSModuleOnce() in JSCommonJSModule.cpp moves the source out of the module object before it runs it. The engine is not involved: the result is the same with and without [JSC] A link that a termination cuts leaves no environment on a module that is linked again #976.

What changed in the fourth revision. The loader code is the same. The tests no longer rely on the order of the cuts: a garbage collection drops what the VM compiled, the next loads of the same text have 4 to 22 more trap checks, and a walk then goes back some places. With --largeHeapSize=1048576 the walk of the third revision stopped at a module is stopped, and with a smaller count it was not. Each round now tells the kind of its cut from the modules that had begun to run. The tests pass with that option, with --slowPathAllocsBetweenGCs=100 and with --collectContinuously=true. The hooks of #975 changed too: the call withdraws only its own termination, and no termination is pending before the n-th check.

Measured on a debug build of this head (--useJIT=0).

• The four tests pass, also with --validateExceptionChecks=true.
• Under a debugger: the two walk reaches loadAgainAfterAbandonedLoad() 28 times, and the nested test reaches askAgainForAbandonedLoad() 22 times for each outer load.
• The graph whose root awaits its own import(): a later import() of the root waits at 104 of 272 rounds, all in the part of the walk where the module is evaluated. The cause is the first bullet of Downsides.
• testaot native module-programs: 8 of 8.
• Debug Bun with this engine (Bun branch aot-follow-up-1031, WebKit 115632d plus this stack): the repro of the status comment prints second import: from plugin, and Bun 1.4.3 hangs there. Three tests for test/js/node/vm/vm.test.ts, which the Bun PR will add, pass.
• The same Bun, a walk over 200 cut positions of a require() with six kinds of later load (1,200 runs): 0 later loads pending, 0 aborts, 0 modules evaluated twice. With no later load, the module of the cut require() was evaluated at the next checkpoint at 6 of 200 positions (4 on main). A later require() of the same root returned the {} placeholder at 146 of 200 positions: the Bun item above.

Residue.

• Questions 1 and 2.
• The evaluation that a termination stops rejects later imports with the termination value, and a module with a top-level await that waits for it never finishes ([JSC] A module that a termination stops records null and rejects its capability with null #866).
• A job of script that the cut checkpoint drops stays dropped, as in Node.
• A behaviour change with no termination: innerModuleLoading() ends its graph loading state when a host hook throws. A load that was in flight and ends later no longer goes on with that drain.
• Seen while testing, not caused by this PR: a require() of an importer from inside a fetch hook, with no termination, stops a debug build at ASSERT(needsErrorReaction != module->loadedModules().contains(...)) in innerModuleLoading().

Considered. Bun's Bun__evictUnfinishedModules() (oven-sh/bun#44833), called where the time limit ends, passes the first Bun test on an engine without this PR. It evaluates modules twice at some cut positions, leaves an import() that was in flight pending, and cannot repair the shell.

Order with the other open PRs. #975 and #976 first. #853 adds the same flags() accessor: whichever lands second drops its copy. #941 edits the same lines of loadModuleSync(). #492 rewrites the New arm, which this PR leaves as it is. #866 (evaluation) and this PR do not touch the same exits. With both merged, an import() of a stopped root that was in flight rejects with null while the one that was cut waits: question 1 covers it.

Cost for a load that meets no termination. bin/jsc of bun-webkit-linux-amd64-lto-aot (the flavor that Bun links), #976 (cb3fc7f) against this PR (3859604). llvm-size, llvm-nm --print-size, and user instructions from perf_event_open with --useJIT=0 --useConcurrentGC=0 (median of 5):

• .text: +8,256 B (37,862,387 to 37,870,643). 53 symbols change size.
• Unchanged in size: hostLoadImportedModule (9,966 B), runInternalMicrotask (6,646 B), finishLoadingImportedModule (3,097 B), moduleLoadStep (3,073 B).
• innerModuleLoading: +28 B, the call in its exception exit. drainWithUseCallOnEachMicrotask: +16 B, the three cold arms.
• VM::setHasTerminationRequest() is inlined where a termination is taken: VMTraps::handleTraps() +73 B, waitSyncImpl() +43 B each. An Atomics.wait() that times out runs 1,531 instructions with and without this PR.
• Fresh two-module import(): 50,159 to 50,180 instructions (+21, 0.04%). Repeated import(): 12,698 to 12,696.
• Dense graph of 201 modules and 5,100 edges: 107,103,634 to 109,685,811 instructions. On that graph main b83d8a1 runs 109,747,212 and [JSC] Test hooks for a termination inside a module load: $vm.callWithTerminationAtTrapCheck(), $vm.moduleLoaderLoadSync(), setModuleLoaderFetchHook() #975 109,313,055, with no change of loader code between them and [JSC] A link that a termination cuts leaves no environment on a module that is linked again #976. So the builds differ by up to 2.4% there for other reasons, and this line shows no cost.
• The helpers that the link-time optimizer no longer inlines (getRegisteredMayBeNull(), endAbandonedLoad(), Deque::clear()) are called only from rejection arms, exception exits and the termination path.
• Per VM: one more empty deque in each MicrotaskQueue (40 bytes). No allocation, queue entry or promise is added for a load that succeeds.
• Some functions of aot/runtime change size by the same inlining (Modules::startImport -503 B, Modules::evaluateSynchronously +243 B). They run only with an image.

Release builds (3859604; the head since then, d061c78, differs only in jsc.cpp of #975). On the lto build of linux amd64 the three loader tests take 93 to 143 ms and the nested test 20 ms. All four pass on the assertions-aot build too. On the lto build of #976 (cb3fc7f) all four fail: two, trap check 25, later loads 0: a later import() is still pending, chain, trap check 25, two, trap check 26 (synchronous), and siblings, import(), trap check 27 of the load inside the hook: the outer load: root.js is still pending.

[robobun]

Status

The PR is reworked a second time after a self-review. It is the third of a stack: #975 (test hooks), #976 (link cleanup), then this one. Only the last commit is new. It stays a draft until a maintainer answers the two questions in the comment below.

How to reproduce on main with #975:

jsc --useDollarVM=1 --useJIT=0 JSTests/stress/module-load-cut-short-by-termination.js
Exception: Error: two, trap check 25, later loads 0: a later import() is still pending

In Bun 1.4.3, with root.mjs (export { dep } from "./dep.mjs";) and an empty dep.mjs beside this file:

import vm from "node:vm";
import { createRequire } from "node:module";
const require = createRequire(import.meta.url);
let spin = true;
Bun.plugin({
  name: "spin",
  setup(build) {
    build.onLoad({ filter: /dep\.mjs$/ }, () => {
      if (spin) for (;;) {} // The vm timeout lands here, inside the fetch hook of the loader.
      return { loader: "js", contents: "export const dep = 'from plugin';" };
    });
  },
});
try {
  vm.runInNewContext("load()", { load: () => require("./root.mjs") }, { timeout: 100 });
} catch (e) {
  console.log("caught", e.code); // ERR_SCRIPT_EXECUTION_TIMEOUT
}
spin = false;
console.log("second import:", (await import("./root.mjs")).dep); // never printed

With this head the second import() fulfils and the script prints second import: from plugin (debug build of Bun with this engine).

[github-actions]

Preview build of d061c78: autobuild-preview-pr-966-d061c789

[robobun]

@dylan-conway, two questions about VM::cancelTermination() decide the shape of this PR and of #866. #975 and #976 do not depend on them.

1. A load that was in flight when the termination came. What happens to it after the withdrawal?

• (a) The job that was cut stays cut. An import() whose own job was cut never settles. This PR does that, and main does.
• (b) That import() rejects. [JSC] A module that a termination stops records null and rejects its capability with null #866 does that for a module that was stopped in its evaluation.
• (c) That import() goes on at the next checkpoint.

With (a), a module that awaits such an import() at top level never finishes, and a later import() of that module waits for it. In the walk of this PR that is 104 of 272 rounds for a graph whose root does await import("./dep.js"). With (b) that module fails for good. Only (c) lets it load later, and (c) calls a fetch hook again that the time limit has cut.

This PR already does (c) for an import() that only waited for the step that was cut. It never evaluates a require() that was cut.

2. Which jobs cancelTermination() puts back on the queue.

• (a) isModuleLoaderInternalMicrotask, the set that the synchronous queue diverts. This PR does that. The loads go on, so module bodies run after the time limit.
• (b) isDrainScopeLoaderJob of [JSC] Microtask drain scopes #427, which leaves out the jobs that resume a module body.
• (c) Only the four registry jobs and the pipe job.

The PR stays a draft until then. I will rework it to the answers.