node:module: build require.cache without user-writable globals and propagate its exceptions
Problem
- The first access of
require.cacheorModule._cacheaborts when thecreateRequireCachebuiltin throws. Debug:ASSERTION FAILED: !returnedExceptionatsrc/jsc/modules/NodeModuleModule.cpp:1171. Release:panic(main thread): abort() called. - The builtin ran inside a
LazyPropertyinitializer, which cannot fail. It readMap,Symbol.forandProxyfrom the global object, soglobalThis.Proxy = undefined; require.cachethrew. A first access near the stack limit threw with no tampering.
Fix
m_lazyRequireCacheObjectbecomes aWriteBarrier<JSObject>filled byGlobalObject::lazyRequireCacheObject(). It returnsnullptrwith the exception pending when the builtin throws. Therequire.cachegetter and theModule._cacheproperty callback propagate it. A failure caches nothing, so the next access retries.- Native code passes the builtin a
ProxyConstructorit creates and the registerednodejs.util.inspect.customsymbol. The builtin reads no user-writable global. The unusedmoduleMapis gone (from #40285). - Correct because the cache is built from intrinsics, like Node's
Module._cache. The failure modes left (stack overflow, OOM, termination) now surface as exceptions. - Verified:
test/js/node/module/node-module-module.test.js(two new tests, both abort on main). Alsorequire-extensions.test.tsand thenode:moduleGC tests.
Background
- A
LazyPropertyis JSC's lazily built slot. Its initializer must callinit.setwith a non-null value (LazyProperty::sethasRELEASE_ASSERT(value)). It has no way to report failure. - A
PropertyCallbackis a static hash table entry whose value a C++ function builds on first lookup. Bun's JSC fork lets the builder return empty with an exception pending (setUpStaticFunctionSlot,reifyAllStaticProperties). createRequireCache(src/js/builtins/CommonJS.ts) returns theProxybehindrequire.cache. Its traps read and write$requireMap.
Notes
Found by fuzzing. Both the release and the debug build abort on current main (f2fe7d3249).
Cross-check of the two earlier approaches (debug build):
| Case | main | #40285 alone (pass intrinsics) | this PR before the fold (fallible initializer) | this PR |
|---|---|---|---|---|
globalThis.Proxy = undefined; require.cache | abort (134) | works | throws TypeError | works |
globalThis.Map = 1 / Symbol.for = null / delete globalThis.Proxy | abort | works | throws TypeError / ReferenceError | works |
| first access near the stack limit, no tampering | abort | abort | throws RangeError, next access works | throws RangeError, next access works |
The fallible initializer is the necessary part: with it alone, a tampered global turns the abort into a thrown error. Passing the intrinsics is a cheap hardening: with it alone, the stack overflow case still aborts because the builtin call still sits inside a LazyProperty initializer. The PR carries both. The earlier test that expected a TypeError for a clobbered Proxy is replaced by #40285's test, which expects require.cache to work.
ESM path. node:module is a lazy native module. The _cache export is read through SyntheticModuleRecord::materializeLazyExport, which propagates an exception from the property callback: a named import of _cache fails to link with the error, a read of ns._cache throws it, and the binding stays unmaterialized so the next read retries. Checked import("node:module") and export { _cache } from "node:module" with Proxy and Symbol set to undefined: both resolve to the same object as require.cache.
Termination. LazyProperty::callFunc wrapped the initializer in DeferTerminationForAWhile. The new accessor does not need it: a termination exception raised inside the builtin propagates like any other exception.
Tests. require.cache does not depend on user-writable globals deletes Proxy and sets Map and Symbol to undefined, then reads require.cache and Module._cache and checks the null prototype, the identity of the two, the [eval] entry and the Bun.inspect output. require.cache first accessed near the stack limit throws, and the next access builds it recurses to the stack limit, probes require.cache in the deepest catch block and Module._cache one frame up, and records both outcomes. Both probes must throw RangeError. The access at normal depth must then return the real cache: null prototype, identical to Module._cache, lists the [eval] entry, and an entry written through it is what require() returns. A stand-in object installed on failure (the #31079 shape) fails the last two checks. 20 of 20 runs pass on the debug ASAN build. Both tests fail on the released binary (the child aborts with exit 134 and empty stdout).
Runs. The repros above, the Module._cache variant of the stack limit repro and the ESM cases under BUN_JSC_validateExceptionChecks=1 BUN_JSC_dumpSimulatedThrows=1: no reports. node-module-module.test.js (49 pass, 1 skip), require-extensions.test.ts (10 pass), esm-registry-concurrent-gc.test.ts, module-children-concurrent-gc.test.ts, test/js/node/test/parallel/test-require-cache.js, and the non-leak test in test/cli/run/require-cache.test.ts.
Scope. The review of this diff pointed out that require.cache is one of several LazyProperty slots whose initializer runs JS and so can throw: m_utilInspectFunction and m_utilInspectStylizeColorFunction (#39382), m_processEnvObject on Windows (#38821), m_lazyTestModuleObject (caches an empty object on failure), ImportMetaObject::requireProperty and JSMockFunction::mock. Two changes would fix the class at once instead of one slot per PR. First, an early return from LazyProperty::callFunc in the JSC fork when the initializer leaves an exception pending, the way the fork already treats PropertyCallback builders. That needs a WebKit PR and a pin bump, and every slot then keeps its initLater lambda with a RETURN_IF_EXCEPTION inside. Second, a $Proxy private global for src/js, in the shape of $Buffer: macro(Proxy) in BunBuiltinNames.h, one install on the global object, and "Proxy" in globalsToPrefix in src/codegen/replacements.ts. That is Bun-only, but the prefix rewrites every Proxy reference in src/js (15 files, including node:http2 and node:test), so it is a change of its own. This PR stays a local fix for the slot with the deterministic repro. If the fork change lands, the accessor here collapses back into the initLater lambda and the two caller-side checks and the tests stay as they are.
History. Supersedes #31079, which installed an empty object as require.cache when the builtin threw. That leaves the cache broken for the rest of the process after a transient failure. Folds in #40285 (same fuzzer finding, intrinsics passed as arguments). The util.inspect lazy initializer in ZigGlobalObject.cpp has the same shape and aborts when Symbol is replaced and an object with an inspect.custom method is logged. That is tracked in #39382 and not changed here.
no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/module/node-module-module.test.js