Repository navigation
Conversation
…ds after the preloads Module.runMain was a custom accessor. Only its setter told the runtime that a preload had replaced the function, and the runtime called a copy that the setter kept. Object.defineProperty and delete followed by an assignment never reached the setter. Module.runMain is now a function row of the static table. After the preloads, the runtime looks at the property of the Module constructor and calls what it holds, with Module as the receiver, when that is not the default function. Node.js does the same with Module.runMain(main). An entry point that is text (--eval, --print, stdin, a Worker from a data: or blob: URL) is not started through Module.runMain, as in Node.js. Deleted: the accessor pair, m_moduleRunMainFunction, hasOverriddenModuleRunMain, has_patched_run_main and Bun__VirtualMachine__setOverrideModuleRunMain.
…preload stopped A Worker whose preload calls process.exit(), or that its parent terminates while the preload runs, no longer reads Module.runMain and no longer calls a replacement. load_preloads makes the same check. Tests: the rows for what stays different from Node.js (an --import preload, a write in a later tick, a wrapper that calls the original late), cron runs, bun run --aot and --hot.
Collaborator
Author
|
Status How this was reproduced:
CI (build 137180, finished):
This PR is #47833. It is a draft: it waits for an answer to the question at the top of the description. |
Collaborator
Author
…le-runmain-data-property
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft. @dylan-conway: only the
runMainrow of Layer 3 (#35466). May it land before oven-sh/WebKit#339?Problem
--requirepreload that replacesModule.runMainwithObject.defineProperty, or withdeletethen=, is ignored: Bun printsmain ran. Node.js v26.3.0 calls the replacement.runMainis aCustomAccessorrow (src/jsc/modules/NodeModuleModule.cpp:1016). Only its setter setshas_patched_run_main, whichreload_entry_pointtests (src/jsc/VirtualMachine.rs:3832).Fix
runMainis aFunctionrow. After the preloads, Bun calls what the property holds, withModuleasthis, unless that is the default. The setter, its copy and both flags are deleted.--eval, stdin) and a stopped Worker no longer go throughModule.runMain, as in Node.js.test/js/node/module/node-module-module.test.js(45 new tests, 37 fail on main), rows inrun-aot.test.tsandhot.test.ts.Background
Module.runMain(main)starts the main file in Node.js (run_main_module.js).CustomAccessorrow of a static table is a native getter and setter. AFunctionrow is a plain data property.Moduleand a number still reach it.Downsides
Module.runMain = 5ordeleteends with aTypeError, as in Node.js.--importpreload is called (12 of 22 forms equal before, 7 after). A defined wrapper that awaits before the original hides the main script's throw (node:module: report a script that Module.runMain() starts and that fails #45358).Notes
The decision this draft needs.
_resolveFilename,runMain: plain data properties", "slow path reads the current value and calls it", and "The deadhasOverriddenModuleRunMainbool onZigGlobalObjectand the threeCustomAccessorpairs are deleted".NodeModuleModule__hasReplacedRunMain) where the plan has the bit of Layer 1.ifinreload_entry_point. Say if they must be a PR of their own.Repro.
-r ./preload.cjs main.cjsrunMain patch calledmain ranrunMain patch called22 ways to change the property, 8 ways to start. Each cell compares stdout and the exit code with Node.js v26.3.0. Release builds of main bdb0471 and of this PR.
--requirepreload, file--eval--printeval: truedata:URL--importpreload, file--importrow: 7 ways now call the patch and 4 end with theTypeError, where Node.js runs the main script.Reflect.set,Object.assign,Object.defineProperty/Reflect.defineProperty/Object.definePropertieswith a value, a getter,deleteand an assignment,deleteand a function onFunction.prototype, an assignment and thenObject.defineProperty, three ways to put the original back,Object.definePropertywith other attributes only, a write to a subclass, to a frozenModule, with another receiver, and5,null,undefined,{},deletealone.execArgv: ["--require", file], Bun with thepreloadoption.thisrows, are rows of the test. Node.js prints the same for each with a CommonJS and with an ES module main file (57 of 57 runs).What changes for a preload that runs on main.
Module.runMain = 5,= null,= undefinedanddelete Module.runMain: the main script ran. NowTypeError: 5 is not a functionand exit code 1. Node.js:TypeError: require(...).Module.runMain is not a function, exit code 1. Fix a segfault when Module.runMain is not a function, and report what an override throws #44352 did the same for{}, a string, a symbol and a BigInt.Sub.runMain = fon a subclass ofModule,Module.runMain = fon a frozenModule,Reflect.set(Module, "runMain", f, other):fwas called. Now the main script runs, as in Node.js.Object.definePropertywas still called. Now the main script runs.Moduleasthis. It was the global object.--eval,--print, stdin,bun --interactive, a cron run (--cron-title,--cron-period), a Worker from adata:orblob:URL (alsoeval: trueof node:worker_threads): an assigned patch was called and the text did not run unless the patch called the original. Now the text runs. Node.js has no cron run. For the others it does the same (eval_string.js reads norunMain).process.exit(), or that its parent terminates while the preload runs: an assigned patch was called after the stop. Now nothing is called, as in Node.js.load_preloadsmakes the same check (src/runtime/jsc_hooks.rs:952).Still different from Node.js.
--importpreload is called. Node.js readsModule.runMainbefore it loads an--importmodule. On main an assigned patch is called (12 of 22 equal), with this PR every way to install one (7 of 22 equal).--import,--preload,--requireand apreloadof bunfig.toml are one list in Bun (src/runtime/cli/Arguments.rs:1470).process.nextTick) is called. Node.js reads the property in the tick of the last--requiremodule. Bun waits for each preload."bun:main"for a file, not the path.original(...args)then loads the generated entry module. A wrapper that passesprocess.argv[1]starts the main module too (a row of the test), but not a server fromexport default { fetch }. node:module: default Module.runMain() to process.argv[1] and reject a non-string argument #41147 and node:module: make a script that Module.runMain() starts the main module #47665 are open for the argument.Module._load(main, null, true)starts nothing:_loadis a stub (Route require() through an overridden Module._load #33026).await, a timer or a tick: what the main script throws is not reported, and the exit code is 0. On main that is so for an assigned wrapper. With this PR it is so for every way to install one. The row istest.failingwith the output of Node.js. node:module: report a script that Module.runMain() starts and that fails #45358 reports such a load.--hotreload does not call the replacement again (a row ofhot.test.ts). The preload list is empty at a reload.--requirepreloads of the command line.NODE_OPTIONS=--requireis not read.TypeError.The same defect in other hooks, not changed here.
Module._resolveFilenameandModule.prototype.require:require()reads them at every call, so they need a watch. node:module: watch Module._resolveFilename and Module.prototype.require as data properties #47763.Error.prepareStackTrace: Read Error.prepareStackTrace from the Error constructor when a stack is formatted #45570.Module.wrapperis an accessor in Node.js too.Open PRs that read what this PR deletes.
has_patched_run_main. node:module: make a script that Module.runMain() starts the main module #47665 readsm_moduleRunMainFunction.isInitialized()at two places. The one that lands second needs a rebase.ZigGlobalObject.hand one comment inNodeModuleModule.cpp.How it works.
runMain jsFunctionRunMain Function 2: namerunMain, length 2,{ writable, enumerable, configurable }, as the function had on main.reload_entry_pointruns the check only if there are preloads, the entry point is a file, and script may still run. Then it callsNodeModuleModule__hasReplacedRunMain.Get. No own property and rows that are not own properties yet: the default. No own property after adelete: replaced. Anything but the default function, an accessor too: replaced.NodeModuleModule__callOverriddenRunMaindoes oneGet(a getter runs once) and the call. A value that is no function throws theTypeErrorof Fix a segfault when Module.runMain is not a function, and report what an override throws #44352.Sub.runMain = fnow makes an own property ofSub. jest-runtime copies the statics ofModulethat way.Measurements. Release builds of main bdb0471 and of this PR on one machine.
size: text 140,285,692 to 140,285,180 bytes (-512). data and bss are the same.llvm-nm:moduleRunMain(126),setModuleRunMain(384) and the initializer of the copy (437) are gone.NodeModuleModule__hasReplacedRunMainis 274 bytes,NodeModuleModule__callOverriddenRunMain628 to 750,reload_entry_point4,351 to 4,459.sizeof(Zig::GlobalObject): 11,072 to 11,064 bytes (DWARF of the debug builds).load_preloadsto the load of the entry point (gdbstepi): 37 to 64 with a preload that does not load node:module, 37 to 269 with one that does, 37 to 410 with one that readsModule.runMain. No syscall in that window on either build.reload_entry_pointitself with no preload: 361 to 360.NodeModuleModule__hasReplacedRunMain(gdb breakpoints onWTF::fastMallocand its siblings and onmi_mallocand its siblings): 0 for each of four preloads.--evalwith a preload. It makes no Module constructor.Bun.gc(true)(heapStats().objectTypeCounts): the same as main with no preload, an empty preload and a preload that loads node:module. A preload that readsModule.runMain: +1Structure. A wrapper that reads and assigns: +1Structure, +1StructureRareData.NodeModuleModule.lut.h, one host export fewer (119 to 118).aot_builtin_table.jsonis the same file.Verified.
node-module-module.test.js125 pass. 37 of the 45 new tests fail on main. The other 8 hold what main does already.run-aot.test.ts: the new row passes forbun runandbun run --debug-aot, and fails on main.hot.test.ts: 27 pass, the new row 15 of 15 runs.worker.test.ts43 pass,worker_threads.test.ts164,preload-test.test.js8,test-module-run-main-monkey-patch.js.plugins.test.ts(a timeout), one ofrun-aot.test.ts.BUN_JSC_validateExceptionChecks=1: a getter that throws, a Proxy on the prototype chain, a value that is no function, a stopped Worker. No report.test/aot/builtinspass.aotBuiltinMismatches()returns the same list as on main. The rest oftest/aotwas not run here.test-module-*,test-require-*,test-preload*andtest-worker-*give the same results as on main.bun build --compile --compile-exec-argv="--require ./preload.cjs"calls a defined wrapper.mock.method(Module, "runMain")of node:test works (on main it throwsERR_INVALID_ARG_VALUE).Self-review. Eleven concerns came back.
--import, a later tick, a cron run,bun run --aot,--hot, a wrapper that passesprocess.argv[1], and the late wrapper astest.failing.no test proof · iteration 0 · 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, test/cli/run/run-aot.test.ts, test/cli/hot/hot.test.ts