Repository navigation
Upgrade WebKit to cf1b36ec8703 - #42319
Merged
Merged
Conversation
- WEBKIT_VERSION -> cf1b36ec8703d8e87436094d21d478d358c7d886 (oven-sh/WebKit main: the upstream merge from #614 and module loader instances from #522). - GlobalObjectMethodTable gains the moduleTypeIsAllowed hook (nullptr: default). - Module records are created against a JSModuleLoader now: thread the loader JSC hands the fetch hook through Bun__analyzeTranspiledModule / the prelinked-record helpers, use it in moduleLoaderImportModule, and give node:vm SourceTextModule / SyntheticModule records their context's loader. - bundler_bytecode_portable: bytecode changes uniformly with the engine (module code declares @ModuleLoader; async/generator bodies per #614).
Collaborator
|
Updated 4:54 AM PT - Sep 11th, 2026
@dylan-conway, your commit 12c45f7 is building: |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
WalkthroughThe change threads ChangesModule loader propagation
Build pin and bytecode snapshots
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The snapshot updates match the WebKit pin change and introduce no identified merge-blocking behavior. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I checked that the five GlobalObjectMethodTable initializers updated are the only ones in the tree (NodeHTTP.cpp, JSCTaskScheduler.cpp, BunGlobalScope.h only include the header or forward the pointer), that the old WEBKIT_VERSION string appears nowhere else in the repo, and that the new JSModuleLoader* parameter sits at the same position in the Rust and C++ extern "C" signatures for zig__ModuleInfoDeserialized__toJSModuleRecord / JSC_JSModuleRecord__create. Callers of NodeVMSyntheticModule::ensureModuleRecord already had RETURN_IF_EXCEPTION guards, so making createModuleRecord throwing doesn't strand an unchecked exception.
Extended reasoning...
This is a WebKit engine bump with cross-language FFI signature changes, so I grepped the tree for the completeness invariants REVIEW.md calls out for this class of change: every GlobalObjectMethodTable literal, every duplicate of the old version pin, and parameter-by-parameter alignment of the reshaped extern "C" decls on both the Rust and C++ sides. All of those held. The one issue that survived is the unqualified getGlobalObjectFromContext call, which is posted inline; nothing further to add in the body.
…d only through a unified-build neighbour)
This was referenced Sep 11, 2026
Open
Closed
robobun
added a commit
that referenced
this pull request
Sep 11, 2026
The fork branch now contains the fork's main (cf1b36ec8703), which bun main pins since #42319.
This was referenced Sep 11, 2026
Merged
dylan-conway
added a commit
that referenced
this pull request
Sep 16, 2026
… names or resolving every import up front (#42823) ### What does this PR do? Upgrades WebKit from `c775a5dc527d` to `d3720d515e14`, which is that commit plus oven-sh/WebKit#665, and adds tests for import bindings between the module records of compiled executables and of `vm.SourceTextModule`s. **The WebKit change.** Since the WebKit upgrade in #42319 a module environment has an import slot per import binding. Three parts of that ran for every module record when it was linked: the record's import names were collected, atomized and sorted (and then binary-searched by string at every import access site when a function's code was linked), every import of the record was resolved to build a key used to compare executables, and every import was resolved again to fill the slots before the module ran. Before, an import was resolved when the code using it was first linked, so programs with many imports that are not all used while the program starts got slower to load. oven-sh/WebKit#665 numbers import slots by position instead of by sorted name (the import's position in the embedded module graph for `--compile --bytecode` executables, the position among the record's import entries otherwise), resolves an import and finds its slot with one lookup per access site, resolves nothing to compare records until a second record for the same module turns up (and then, for a module of the embedded graph, only the imports the graph does not answer), and fills import slots on first use unless the code is shared. Measured with Bun built against WebKit `main` and against the WebKit change (medians, interleaved runs): a generated program of 400 modules with about 40 import bindings each, a sixth of which are read while it starts, went from 79.4 ms to 75.3 ms run from source, and from 22.6 ms to 19.6 ms built with `--compile --bytecode --splitting` (every module a chunk of its own). ### How did you verify your code works? `test/bundler/bundler_compile_prelinked.test.ts` grows from 39 to 79 cases: - Six new programs in which every module is a chunk (module record) of its own, so each import crosses a record boundary: namespace imports of builtin modules before, between and after named imports (the only namespace imports that survive bundling); named, default and namespace imports of builtin modules next to imports of other chunks; one record importing 96 bindings that are all updated live; imports that are never read, first read long after evaluation, or first read on a path only taken once the function is hot; one variable reaching a record under several names and through renaming, star and chained re-exports; a record loaded by `import()` long after the records it imports from. - `bindingCase()`: `graphCase()` (the executable run with the embedded graph, with the graph cross-checked, and with the graph disabled) plus the same program built **without `--bytecode`**, which embeds no graph, so its records are made from each chunk's source; with and without `--splitting`. Used for the new programs and for the existing cycle, star-export, re-export, namespace, default, dynamic import, top-level await, CommonJS interop and TDZ cases. - `RegistryDeleteImporter` and `RegistryDeleteImporterAndExporter` (also through `bindingCase()`): deleting a chunk's registry entry and importing the key again gives the loader a second record of the module while the first one's functions are hot. The module imports bindings of another chunk, a named binding of a builtin module and a builtin module's namespace; with the exporting chunk deleted too, the second record links to a second record of it, with its own state, while the first stays linked to the first. `test/js/node/vm/vm.test.ts`: `SourceTextModule`s with one identifier and one source text, linked to dependencies whose exports sit at different places, each read their own bindings (from functions that were hot before the next record was made), in the main context and in a new one. A second case has several import declarations naming one specifier; it runs where that works and is `todoIf(isDebug || isASAN)`, because `NodeVMSourceTextModule::createModuleRecord` asserts on it in builds with assertions enabled (independent of this change; fixed separately). All of these pass on a local build of this branch against WebKit `main` with the WebKit change and on one without it. On the WebKit side the change comes with 26 new `JSTests/modules/import-slots-*.js` files and a JS↔wasm cycle test (every kind of import binding on the global object's own loader and across additional loaders, in six tier configurations), and every other test in `JSTests/modules` passes.
alii
pushed a commit
that referenced
this pull request
Sep 23, 2026
### Problem - `CronExpression::next()` (`src/runtime/api/cron_parser.rs:250`) converts each candidate day to a calendar date through JSC. Nothing stops the walk at the end of the Date range. JSC converts nothing more than one day past it. - Before #42319, a debug build of `main` aborted: `Bun.cron.parse("0 0 1 1 *", 8.64e15)` gave `ASSERTION FAILED: year >= minYear && year <= maxYear` at `PlainGregorianDateTime.h(91)`. A release build returned `null`. - #42319 (merged, 6a92015) moved `main` to a WebKit where the conversion returns an all-zero date. The year goes back to 0, the 8-year bound never trips, and `Bun.cron.parse("0 0 30 2 *", new Date(8.64e15 - 86400000 * 400))` never returns. `main` and the canary hang on this call today. ### Fix - `next()` returns `None` when the candidate day is past +275760-09-13. Every zone offset is smaller than one day, so no instant on a later wall-clock day fits in a Date. - `next()` also returns `None` for a `from` outside the Date range (fake timers can set the scheduler's clock to any number). It skips a candidate that resolves past 8.64e15, so `cron_parse` needs no clamp. - Verified: `test/js/bun/cron/cron-parse.test.ts`. The 2 new tests fail without the fix and pass with it, on `main` before and after #42319 (6b394bf and 158ff6c). Also ran the rest of `test/js/bun/cron/`. - Self-reviewed: 7 points raised, 4 addressed. 3 are about #42177, #42267 and #36894 and are comments there. ### Background - `next()` walks wall-clock time forward from `from`, day by day, until all five fields match. - 8.64e15 ms is +275760-09-13T00:00:00Z, the largest time value a `Date` holds. - #42319 merged before this PR. `main` has the endless loop until this PR merges. <details><summary>Notes</summary> **Upstream change.** WebKit 320788@main (d5ba0ecec9, bug 323443) changed `DateCache::computeGregorianDateTime` in `JSDateMath.cpp` from `if (!std::isfinite(ms)) return { };` to `if (!canNarrowToInt64Milliseconds(ms)) return { };`, where `canNarrowToInt64Milliseconds(ms)` is `isfinite(ms) && abs(ms) <= maxECMAScriptTime + msPerDay`. `Bun__msToGregorianDateTime` passes the empty value through as year 0, month 1, day 0, weekday 0. With year 0 the walk climbs about 100 million day steps back to year 275760, gets year 0 again, and repeats. **On `main` before #42319 (WebKit dfd696443b, debug+ASAN build).** These calls abort with the `PlainGregorianDateTime` assertion: `Bun.cron.parse("0 0 1 1 *", 8.64e15)`, `Bun.cron.parse("0 0 30 2 *", new Date(8.64e15 - 86400000 * 400))` with and without `{ tz: "UTC" }`, and `Bun.cron.parse("* * * * *", 8.64e15, { tz: "UTC" })`. The assertion fires inside the `ms_to_gregorian_date_time_utc` call that this PR bounds, when the walk reaches year 275761. Bun 1.4.3 (release) returns `null` for all of them. `* * * * *` from 8.64e15 with a named zone takes 1 to 4.6 s there, because it walks 8 years one minute at a time. With this PR the walk ends after at most two days. **Proof on both WebKit versions.** `bun bd test test/js/bun/cron/cron-parse.test.ts`, with `src/` at the base and then at this commit. - `main` (6b394bf): 23 pass and 2 fail, then 25 pass. The parse test fails with exit code 134 and the assertion text. The scheduler test prints `scheduled` for both clock values. - Head of #42177 (676b168, WebKit 4b9ff9959a) with this commit on top: 23 pass and 2 fail, then 25 pass. Both tests time out and bun:test kills the child. - `main` after #42319 (158ff6c, WebKit cf1b36ec8703) with this branch rebased on it: 23 pass and 2 fail, then 25 pass. Both tests time out and bun:test kills the child. **Probe.** 29 calls, each in a child process with a 20 s limit, on the debug build of 676b168 with and without the day bound, and on bun 1.4.3. Inputs: `* * * * *`, `0 0 1 1 *`, `0 0 30 2 *`, `0 0 13 9 *`, `0 0 14 9 *`, `0 12 13 9 *` from 8.64e15, 8.64e15 - 1 minute, 8.64e15 - 10 days, 8.64e15 - 400 days and -8.64e15, in local time (TZ=UTC), `UTC`, `America/New_York` and `Asia/Tokyo`. Without the bound 10 calls hang. With the bound every result is equal to the 1.4.3 result. **Every conversion in `next()` is now inside the window that JSC converts.** `from` is inside the Date range. The noon of each day that the walk can reach is at most 8.64e15 + 12 h. `resolve_local_match` probes at most 121 minutes past `from`. The lower end needs no bound, because the walk only moves forward. The bound keeps +275760-09-13 itself, so the last minute of the range still matches in `Pacific/Kiritimati` (UTC+14, test row `0 14 13 9 *`). **A candidate on the last day can resolve past 8.64e15.** The local path returns a value above 8.64e15. The named-zone path returns NaN. `next()` skips both and continues to the end of that day (at most 1440 more steps). So `next()` never returns a value outside the Date range. Before, `cron_parse` mapped such a value to `null`, but the scheduler armed a timer for it: with the clock at exactly 8.64e15, `Bun.cron("* * * * *", ...)` scheduled a job. Now it throws `has no future occurrences`, the same as a named zone. **Test design.** Both tests spawn a child, because a walk that does not stop spins in native code and cannot be interrupted. They are not concurrent: bun:test kills the children of a timed-out test only when the test is not concurrent. The `timeout` and `killSignal` on the child are the kill switch for CI, where the per-test timeout is 90 s or more. **Timing test.** "impossible day/month (Feb 30) returns null quickly" asserts under 50 ms. It fails on a debug+ASAN build without this PR (159 ms). The first date conversion in a process costs about 150 ms there, and a warm call takes 2 ms. The test now makes one call before it starts the clock. #36894 has the same edit. **Relation to #36894.** #36894 is closed in favor of this PR. It maps a match past 8.64e15 to `None` inside `next()`, removes the clamp in `cron_parse`, and guards the year in `Bun__gregorianDateTimeToMSInZone`. This PR now does the first two. The year guard is not needed, because the walk cannot reach year 275761. #36894 does not bound the day walk, so it does not stop the loop on the new WebKit. Its `{ tz }` boundary rows are part of the new parse test here, and they pass. **Not changed.** `Cookie.cpp:261` also calls `msToGregorianDateTime`. On the new WebKit `new Bun.Cookie("a", "b", { expires: 1e13 }).toString()` prints `Expires=Sun, 00 Jan 0000 00:00:00 GMT`. With the old WebKit it printed `Sun, 20 May 318857 17:46:40 GMT`. Both are outside the Date range, and neither hangs. `toISOString` in `wtf-bindings.cpp` only receives the time value of a `Date` or the current time. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 3 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 2 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/cron/cron-parse.test.ts bun test v1.4.3 (4ff9193) test/js/bun/cron/cron-parse.test.ts: (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > impossible day/month (Feb 30) returns null quickly [236.60ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > Feb 29 finds next leap year [1659.45ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > weekday matching uses local day-of-week [1741.19ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > strictly-after: from = exact match returns the next occurrence [1887.91ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 1-7 means Mon-Sun (every day) [1758.25ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > DOM/DOW OR semantics when both restricted [1844.78ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 6-7 means Sat-Sun [1565.94ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > scalar 7 still means Sunday [1374.75ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 5-7 means Fri-Sun [1793.66ms] (pass) Bun.cron.parse — w ... (truncated) release without fix: 2 FAILED bun test v1.4.3-canary.1 (4ff9193) test/js/bun/cron/cron-parse.test.ts: (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > impossible day/month (Feb 30) returns null quickly [5.77ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > weekday matching uses local day-of-week [25.21ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > Feb 29 finds next leap year [23.54ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 1-7 means Mon-Sun (every day) [24.17ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > DOM/DOW OR semantics when both restricted [25.06ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > strictly-after: from = exact match returns the next occurrence [35.13ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 5-7 means Fri-Sun [61.49ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 0-7 means every day [63.62ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > scalar 7 still means Sunday [65.85ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 6-7 means Sat-Sun [70.84ms] (pass) Bun.cron.parse — invalid `from` argument > throws for out-of-range/non-finite ms: 1e+300 [0.20ms] (pass) Bun ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/cron/cron-parse.test.ts bun test v1.4.3 (4ff9193) test/js/bun/cron/cron-parse.test.ts: (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > impossible day/month (Feb 30) returns null quickly [201.36ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > Feb 29 finds next leap year [1469.26ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > strictly-after: from = exact match returns the next occurrence [1652.81ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > DOM/DOW OR semantics when both restricted [1462.10ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 1-7 means Mon-Sun (every day) [1537.11ms] (pass) Bun.cron.parse — algorithm (pinned TZ=UTC) > weekday matching uses local day-of-week [1977.03ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 5-7 means Fri-Sun [1813.74ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > scalar 7 still means Sunday [1615.41ms] (pass) Bun.cron.parse — weekday 7 = Sunday in ranges > 6-7 means Sat-Sun [1781.97ms] (pass) Bun.cron.parse — w ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 840ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/6] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 243 extern-C blocks audited [1/6] cargo bun_runtime → libbun_runtime.a ^[[1m^[[92m Compiling^[[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) ^[[1m^[[92m Compiling^[[0m bun_errno v0.0.0 (/workspace/bun/src/errno) ^[[1m^[[92m Compiling^[[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) ^[[1m^[[92m Compiling^[[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) ^[[1m^[[92m Compiling^[[0m bun_safety v0.0.0 (/workspace/bun/src/safety) ^[[1m^[[92m Compiling^[[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) ^[[1m^[[92m Compiling^[[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) ^[[1m^[[92m Compiling^[[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) ^[[1m^[[92m Compiling^[[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) ^[[1m^[[92m Compiling^[[0m bun_paths v0.0.0 (/workspace/bun/src/paths) ^[[1m^[[92m Compiling^[[0m bun_collections v0.0.0 (/workspace/bun/src/collections) ^[[1m^[[92m Compiling ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/runtime/api/cron.rs | 7 +-- src/runtime/api/cron_parser.rs | 25 +++++++--- test/js/bun/cron/cron-parse.test.ts | 99 +++++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 12 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/runtime/api/cron.rs 3 2 30 src/runtime/api/cron_parser.rs 6 7 30 test/js/bun/cron/cron-parse.test.ts 6 8 29 ``` </details> <!-- robobun:evidence:end -->
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.
What does this PR do?
Upgrades WebKit to oven-sh/WebKit@
cf1b36ec8703(currentmain: the upstream merge from oven-sh/WebKit#614 plus oven-sh/WebKit#522). Supersedes #42177, which pins the pre-#522 commit.Bun-side changes the new JSC needs:
GlobalObjectMethodTablehas a newmoduleTypeIsAllowedhook; the five tables passnullptr(default behaviour).JSModuleLoader: the loader JSC passes to the fetch hook is threaded throughBun__analyzeTranspiledModuleand the prelinked-record helpers,moduleLoaderImportModuleuses the loader it is given, andnode:vmSourceTextModule/SyntheticModulerecords are created against their context's loader (the same global they already link and evaluate in).bundler_bytecode_portable: the serialized bytecode changes uniformly with the engine (module programs declare@moduleLoader; async/generator bodies per SegmentationFault (26 or big number): fetch().then() #614), so the affected snapshot entries move.How did you verify your code works?
Builds and runs against the
autobuild-cf1b36ec8703d8e87436094d21d478d358c7d886prebuilt;bundler_bytecode_portable.test.ts,test/js/node/vm, thetest-vm-module-*node tests andimport-meta.test.jspass locally. Full matrix in CI.