Skip to content

bun test --isolate=modules: test index keys that a file defines without configurable - #48630

Draft
robobun wants to merge 2 commits into
mainfrom
robobun/4f2d4122/isolate-modules-index-keys
Draft

robobun wants to merge 2 commits into
mainfrom
robobun/4f2d4122/isolate-modules-index-keys

Conversation

@robobun

@robobun robobun commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Needs oven-sh/WebKit#1039 in Bun's WebKit. The new tests fail until then.

Problem

  • With bun test --isolate=modules, an index key that a test file defines on a built-in without configurable: true stays for the later files: [][7] reads fake. On Array.prototype the file's other changes stay too.
  • restoreProperties() (src/js/internal/test/globals.ts:122) deletes the key through jsFunctionDeleteOwnProperty (src/jsc/bindings/TestEnvironment.cpp:139), in JavaScriptCore's IgnoreConfigurable mode. JSObject::deletePropertyByIndex() does not read that mode and returns false.

Fix

Background

  • --isolate=modules (bun test: Vitest compatibility #44833, in no release) keeps one global object for all test files. Between files, globals.ts puts the built-ins back from a snapshot and deletes each key that a file added.
  • IgnoreConfigurable is JavaScriptCore's delete mode that also removes a property that is not configurable. For an index key it did nothing.
  • The DFG compiler folds a read of an element that is read-only and not configurable to a constant, and a later file runs code that earlier files compiled. So the delete must drop that code. The third test holds this.
  • Considered the delete in TestEnvironment.cpp: from Bun, only a drop of all optimized code invalidates. In JavaScriptCore only code that depends on the object goes.
Notes

Repro. bun test --isolate=modules ./a.test.js ./b.test.js ./c.test.js

// a.test.js
test("a", () => {
  Object.defineProperty(Array.prototype, "7", { value: "fake" });
  Array.prototype.at = function fake() { return "fake"; };
  Array.prototype.zz = 1;
});
// b.test.js. c.test.js is the same but for the letter
test("b", () => {
  console.log([[][7], [1, 2].at(1), [].zz, Array.prototype.length].map(String).join(" "));
});
what b and c print main with oven-sh/WebKit#1039
the files above fake fake 1 8 undefined 2 undefined 0
the same three lines on Map.prototype fake 2 undefined undefined 2 undefined
a getter on Object.prototype[0]: ({})[0], [][0], "0" in {} trap trap true undefined undefined false
Object.defineProperty(globalThis, "7", { value: "fake" }): globalThis[7], "7" in globalThis fake true undefined false

With configurable: true, with the name key "seven", and with --isolate, main already prints the right column.

Why Array.prototype keeps everything. The key makes Array.prototype.length 8. The restore then sets length back to 0, which throws while the element is there, and restoreGlobals() catches the error for the whole object (globals.ts:187). With the element deleted first, length goes back and the rest follows. Index keys come first in Reflect.ownKeys().

The three tests.

  • an index key that a file defines without configurable is undone too: file a defines a read-only key on Array.prototype (and replaces at, adds zz), globalThis, Map.prototype and String.prototype, a writable key on Math, and getters on Date and Object.prototype[0]. File b prints the clean state and defines all of them again with other values, which throws while a key of file a is there. File c prints the clean state.
  • a preload defines an index key without configurable for every file: the preload runs again for each file and defines a getter on Math[7]. On main the second run throws TypeError: Attempting to change the getter of an unconfigurable property.
  • compiled code doesn't keep an index key that a file defined without configurable: a shared module has function read() { return Math[7]; }. The second file defines the key and calls read() until the DFG has compiled it. The third file must read undefined, then defines the key with another value and must read that. With the key removed from the object and the code kept, it prints read b b. The first file only loads the module: code that is compiled while a function has one copy is dropped when the next file makes its copy.

Results (debug ASAN build, main 447ff87, WebKit 264cf6db built from source with and without oven-sh/WebKit#1039)

  • The three new tests: 0 of 3 pass without the change, 3 of 3 with it. With the WebKit change but without the new structure: 2 of 3, the third prints read b b.
  • The bun test --isolate=modules block of isolation.test.ts with the change: 19 of 20 pass. no other builtin module patches a global class when it loads fails because its inner test takes 15 to 19 s against its limit of 5 s on this debug build. That test defines no index key.
  • With their child runs under --aot (the preload of test/aot/harness/children-under-aot.ts): the preload test and the compiled-code test pass. The first test fails, as the new line in runner-suites.known.txt says: the build warns TS100014: this assignment throws if it runs: built-in objects are read-only in a compiled executable for the keys on Array.prototype, Map.prototype, String.prototype and Object.prototype.
  • mock-fn.test.js (848 pass), spyMatchers.test.ts (150 pass, 3 todo) and capture-stack-trace.test.js (76 pass): the same counts with and without the change. They are the tests of the other users of IgnoreConfigurable.

What it costs at run time. Nothing in Bun: 0 native lines, 0 host functions. The numbers for JavaScriptCore are in oven-sh/WebKit#1039. In short: a delete of an element runs the same instructions but for two cases (a configurable sparse element: 2 more, a refused delete: 27 more), and a restore that force-deletes a key makes one Structure and drops the optimized code that depends on that object (3 of 31 warmed functions for Math, 0 of 31 for Object.prototype and globalThis). In 60 test files that define no such key, the new function is never called.

When this can merge. After oven-sh/WebKit#1039 is merged and WEBKIT_VERSION (scripts/build/deps/webkit.ts) is at or past that commit. WebKit's main is past what Bun's main builds against, so this PR does not move the pin.

Still carried over after this (none is new)

  • A var or function declaration of a classic script: JSSymbolTableObject::deleteProperty() refuses it in every mode.
  • A frozen, sealed or non-extensible built-in stays so. A frozen Array.prototype keeps its length.
  • A read-only or accessor index key on a prototype makes JavaScriptCore store every array in the slower storage for the rest of the run (haveABadTime). Only speed.
  • On arm64 JavaScriptCore does not fold such a read, so the third test cannot fail there.

Seen on the way, not part of this. On a debug ASAN build, bun test --aot sometimes dies while it compiles the files of the first test (4 of 14 runs): SEGV in WTF::removeIterator under JSC::AOT::Typed::LinkFlow::findWhatSomeLaneWasGiven (aot/facts/AOTLinkFlow.cpp:2980). The HashMap iterator of that function is destroyed after its lock is released. Only builds with hash table iterator checks have that list.

…ut `configurable`

Three tests beside "what a file defines without `configurable` is undone too".
They pass once the WebKit that Bun builds against deletes such a key in
VM::DeletePropertyMode::IgnoreConfigurable.
A compiled run refuses an index key on a built-in prototype (TS100014), so the
first two files of that test fail under --aot. The warm-up loop of the
compiled-code test stops at 1000 rounds.
@robobun

robobun commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 3:54 PM PT - Nov 11th, 2026

❌ @robobun, your commit 5b8328a has 1 failures in Build #140261 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 48630

That installs a local version of the PR into your bun-48630 executable, so you can run:

bun-48630 --bun

@robobun

robobun commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on a debug build of main: bun test --isolate=modules ./a.test.js ./b.test.js ./c.test.js, where a.test.js runs Object.defineProperty(Array.prototype, "7", { value: "fake" }). The files b and c print fake fake 1 8 for [][7], [1, 2].at(1), [].zz and Array.prototype.length. With oven-sh/WebKit#1039 they print undefined 2 undefined 0.

The fix is oven-sh/WebKit#1039. This PR holds the tests and waits for that change in Bun's WebKit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants