Shared-memory threads for JavaScriptCore (experimental, not working yet)

[Jarred-Sumner]

What this is

Shared-memory threads for JavaScriptCore. new Thread(fn) runs fn on another thread, in the same heap, with the same objects. No structured clone, no message passing, no SharedArrayBuffer-only escape hatch. You share an object by sharing the object.

Status: parallel JavaScript executes, through all four JIT tiers, with no global lock — and the thread test suite now passes that way. It is not done: thread-sanitizer cleanup, fuzzing, one benchmark over budget, and a long soak stand between "tests pass" and anything more. The locked fallback mode and the threads-disabled configuration remain untouched and verified. The bring-up log at the bottom is honest about what broke and what it took. This PR exists so the design and the code can be read and argued with. It may never merge.

The API

A thread is a function call on another core:

const t = new Thread((a, b) => {
  return expensive(a, b);
}, x, y);

t.join();          // blocks, returns fn's value or rethrows its exception
await t.asyncJoin(); // same, as a promise; never blocks
Thread.current;    // your Thread
t.id;              // engine thread id, main is 0

That's the whole spawn story. No separate file, no blob URL, no bundler config for a worker entry point, no onmessage protocol. The function is a closure — it sees the variables it closes over, on the other thread, because there is one heap.

"Run this function on another core"

This is the thing workers have never been able to do, because a function can't cross a worker boundary. What people actually write today:

// the state of the art: stringify your own source code and eval it in a blob
const src = `self.onmessage = e => self.postMessage((${heavy.toString()})(e.data))`;
const worker = new Worker(URL.createObjectURL(new Blob([src])));
// ...and `heavy` better not close over anything, call anything you imported,
// or reference any class it didn't define inside itself. it's a string now.
const result = await new Promise((resolve, reject) => {
  worker.onmessage = e => resolve(e.data);
  worker.onerror = reject;          // and errors arrive as ErrorEvents, not your thrown value
});
worker.terminate();

This branch:

const result = new Thread(heavy, input).join();

heavy is a real closure. It sees your imports, your classes, the variables around it. If it throws, join() rethrows the actual exception object with the actual stack.

Parallel map, in place

const items = loadItems();           // 100k plain objects
const results = new Array(items.length);
const next = { i: 0 };

const workers = Array.from({ length: 8 }, () => new Thread(() => {
  for (;;) {
    const i = Atomics.add(next, "i", 1);     // atomic counter on a plain property
    if (i >= items.length) return;
    results[i] = transform(items[i]);        // write straight into the shared array
  }
}));
workers.forEach(t => t.join());
// results is fully populated. nothing was copied, ever.

Eleven lines, and it's the real items and the real results — no chunking the input into per-worker messages, no reassembling N result arrays back into one, no transferable ping-pong. The worker version of this is a project.

A shared cache that's actually shared

const cache = new Map();
const lock = new Lock();

function memoized(key) {
  let hit;
  lock.hold(() => { hit = cache.get(key); });
  if (hit) return hit;
  const value = compute(key);
  lock.hold(() => { cache.set(key, value); });
  return value;
}

Call memoized from any thread. One Map, one miss per key, ever. With workers your options are: N workers with N private caches recomputing each other's entries, a "cache server" worker you talk to via postMessage round-trips, or hand-rolling a hashmap + string interning + allocator on top of a SharedArrayBuffer. People have done all three. None of them is a Map.

Cancellation is a boolean

const ctl = { stop: false };
const search = new Thread(() => {
  for (const candidate of space) {
    if (ctl.stop) return null;               // reads the live object — sees the write
    if (good(candidate)) { ctl.stop = true; return candidate; }  // ...and stops the others
  }
}, /* one per core */);

// from the main thread, any time:
ctl.stop = true;

Eight threads race; the winner flips the flag and the rest stand down. The worker equivalent is a termination protocol or worker.terminate() (which can't return the partial result and leaks whatever the worker was holding).

Live progress without an event protocol

const progress = { done: 0, total: files.length };
const t = new Thread(() => {
  for (const f of files) { process(f); Atomics.add(progress, "done", 1); }
});

const ticker = setInterval(() => {
  render(`${progress.done}/${progress.total}`);   // just... read it
  if (progress.done === progress.total) clearInterval(ticker);
}, 100);
await t.asyncJoin();

No postMessage({type: "progress", ...}) events, no message-rate throttling so you don't flood the channel, no event-listener bookkeeping. The counter is a property; you read it.

Blocking handoff, when blocking is what you mean

const lock = new Lock(), cond = new Condition();
const mailbox = { ready: false, payload: null };

const consumer = new Thread(() => {
  let received;
  lock.hold(() => {
    while (!mailbox.ready) cond.wait(lock);  // releases lock, parks the thread, reacquires
    received = mailbox.payload;
  });
  return received.process();                 // a real object with real methods
});

lock.hold(() => { mailbox.payload = buildThing(); mailbox.ready = true; cond.notify(); });

This is the textbook condition-variable handshake, in JavaScript, with a JS object as the mailbox. It has never been writable before — workers can't block, and SAB Atomics.wait can only hand off integers in a byte buffer, so the "payload" always ends up being an index into some serialization scheme you invented.

One global object, one module graph

Spawned threads run in the same realm. globalThis is the same object. Array, Object.prototype, your polyfills, your framework's singletons — one of each, not one per thread. x instanceof Foo is true on every thread because there is exactly one Foo.

The same goes for the module graph: workers re-fetch, re-parse, re-compile, re-execute, and re-JIT every transitive import per worker — and every module-level side effect (registries, schemas, connection setup) runs again in each one, which is its own class of bug. Threads share the already-executed module graph the way they share everything else: it's just objects in the heap. Spawning thread number 8 costs what spawning a thread costs (~150KB–1MB busy, ~30–50KB parked), not another copy of your application's startup.

This is the part that's hard to convey in a feature list: workers give JavaScript a multi-process model with shared-memory bolted on for byte arrays. This gives it a multi-threaded model. Every pattern in the standard concurrency literature — thread pools over shared structures, fine-grained locking, condition-variable handshakes, lock-free counters — translates directly instead of through a serialization boundary.

The full synchronization surface

const lock = new Lock();           // non-recursive
lock.hold(() => { /* critical */ }); // tryLock fast path; release is finally-equivalent
await lock.asyncHold(fn);          // or no fn: resolves to a release() function

const cond = new Condition();
cond.wait(lock);                   // atomic release+block, spurious wakeups allowed
cond.asyncWait(lock);              // promise resolves holding the lock again
cond.notify(); cond.notifyAll();

const tls = new ThreadLocal();     // .value is per-thread, any JS value

Atomics.* is extended from typed arrays to ordinary object properties: Atomics.load(obj, "k"), store, add/sub/and/or/xor, exchange, compareExchange (SameValueZero, so NaN CAS loops work), wait, waitAsync, notify. Each is one SeqCst atomic step on an own data property.

Thread.restrict(obj) pins an object to the calling thread — any enforced access from another thread throws ConcurrentAccessError. It's the opt-out for objects you know shouldn't escape.

Each thread runs its own microtask queue and event loop turn; join() settles when the thread's fn has returned and its queues are drained and nothing is keeping it alive — so await works inside threads like you'd expect.

Everything is behind --useJSThreads. Flag off, none of this exists and the engine is unchanged.

Promises and async across threads

Promises are ordinary heap objects, so they're shared like everything else. The semantics we pinned (these were design decisions, not accidents — the alternatives are in the spec history):

• Reactions run on the settling thread. If thread A registers .then() (or is suspended at an await) and thread B resolves the promise, the continuation runs on B's microtask queue, not A's. There is no "hop back to the registering thread" for ordinary promises in v1. This is the cheap, predictable rule; a registrant-affinity mode was considered and deferred. The exception: asyncJoin/asyncHold/waitAsync tickets do settle on the thread that created them — those are explicitly tied to their requester's event loop.
• Concurrent then() vs resolve() is safe. Promise internal-state transitions take the promise's per-object cell lock, so two threads racing to register and settle can't tear the reaction list. You can't observe a half-settled promise.
• AsyncLocalStorage survives thread migration. This matters for Bun: ALS context is captured per-reaction at registration time — .then()/await stashes the current context into the reaction job itself, and whichever thread runs the job swap/restores it around the callback. So a continuation that migrates to the settling thread still observes the store it was registered under, not the settling thread's store. The "current async context" cursor itself becomes per-thread state (it's thread-local by definition; sharing it would just be a race). No embedder hop is needed to keep ALS correct — the carry is structural.
• Each thread drains its own microtask queue; queues never interleave jobs from other threads. A thread's join() settles when its function returned, its queues are empty, and nothing (pending tickets, timers) is keeping it alive.
• Termination is VM-wide, not per-thread, and a terminated thread's undrained microtasks are dropped — but settlements it already published stay visible to everyone (you can drop pending work; you can't un-resolve a promise). join() on a terminated thread rethrows an ordinary Error, not the engine's internal termination exception.

Overhead expectations

Pizlo's post lays out the cost model and our phase-1 numbers are consistent with it so far. Being concrete about what to expect:

• Code that never shares: ~zero. Inline-cached property access on TTL (transition-thread-local) objects is today's machine code plus at most ~one arithmetic instruction's worth of tag check, and inline-cell fields (where the optimizer puts hot properties anyway) are atomic for free. Phase 1's measured serial regression with the flag on is 0.45% worst-case across our benchmark set, and the R8 gate fails the branch if flag-off ever exceeds 1%.
• Objects that fail TTL inference: segmented storage. Out-of-line property and array access grows an extra load + arithmetic. The arraylet literature puts this class of indirection at ~10% if applied to everything — the entire point of TTL inference is that it's applied surgically to the objects that actually shared, so the program-level cost should be far below that. This is the number to watch skeptically; it's the least-validated part of the model until the parallel benchmarks exist.
• Transitions on shared objects: ~7x. Adding a property to an object another thread is using takes the cell lock and possibly a structure operation under it (a CAS pair is ~22 cycles before you've done any work). Transitions are rare relative to accesses in real code, which is what makes this tolerable; a workload that's transition-heavy on shared objects will feel it.
• First shared write costs a watchpoint fire: jettisoning compiled code that assumed thread-locality and recompiling. One-time per code/object-type pair, amortized to nothing in steady state, but a program that gradually starts sharing everything will pay a recompile storm during the phase change.
• Per-thread memory: ~150KB–1MB while busy (native stack touch plus retained allocator blocks across hot size classes), ~30–50KB parked after the scavenger runs — versus roughly 5–15MB for a Worker today. Thread IDs come from a 2^15 space, recycled at GC. This is the difference between "a thread per connection is silly" and "a thread per connection is a thing you could measure."
• Scalability target, not promise: the design goal is near-linear when threads don't deliberately share. Nothing in phase 2 is validated against that claim yet — the ladder's amplifier and bench rungs are where it gets tested, and we'll publish whatever they say.

What it means for JavaScript

Today if you want parallelism in JS you get workers: separate heaps, postMessage, structured clone, and SharedArrayBuffer if you're willing to write your program against a byte buffer. That's fine for some workloads and miserable for others — anything where the working set is an object graph (parsers, bundlers, servers with shared caches) either pays serialization on every hop or gets rewritten into typed arrays.

This is the other model — the Java/Go/C# model: threads share the heap, races on your own data are your problem, races on the engine's data are the engine's problem. The language stays single-threaded in its semantics per thread; the VM guarantees memory safety (no torn JSValues, no broken butterflies, no type confusion) no matter how badly your program races. A data race in your code gives you stale or surprising values, never a corrupted heap.

The design, and what it's based on

This is an implementation of the design Filip Pizlo published in 2017: "Concurrent JavaScript: It Can Work!". If you want the full argument, read that; THREAD.md in this branch is the adaptation. The short version of the mechanisms:

• TID-tagged flat butterflies. Every object's property storage carries the owning thread's id in spare bits of the butterfly pointer. The owning thread accesses its own objects exactly as fast as today — one check that folds into existing structure checks. Objects don't pay for concurrency until a second thread actually touches them.
• Segmented butterflies on first shared write. When another thread writes, the object transitions (DCAS) to a segmented butterfly: an immutable spine pointing at fragments. Resizing appends fragments instead of reallocating-and-copying, which is what makes concurrent resize safe without locking every access. Reads stay lock-free.
• Per-object cell locks (2 bits in the header) for the slow paths that genuinely need mutual exclusion — dictionary transitions, certain deletes (deleted slots are quarantined until a GC safepoint so racing readers never see reuse).
• Transition-thread-local / write-thread-local watchpoints. The JITs speculate that an object's transitions and writes stay on one thread; the first counterexample fires a watchpoint and recompiles. This is how almost all of the cost stays off the fast path: the optimizing tiers keep emitting today's code until the program actually shares.
• A shared heap server with per-thread clients. One GC, N mutators, safepoint epochs, per-thread allocation caches; the atom table is sharded; each thread has a lightweight VM view ("VMLite") over the shared VM.

The bring-up strategy is two-phase and that's why the status is what it is. Phase 1 landed all of the above machinery — the tagged/segmented butterflies, the locks, the watchpoints, the heap server, the API — but kept JS execution serialized on a global lock, so every concurrent-object-model path could be tested and TSAN'd deterministically with the GIL as a semantic oracle. Phase 2 ("ungil") removes the lock: per-thread VM entry, a stop-the-world conductor protocol for the things that genuinely need it (watchpoint fires, haveABadTime, certain OSR cases), per-thread microtask/task queues, and a thread-teardown protocol that took several design revisions to get right (the spec history in docs/threads/ is honest about that). Phase 2 is what's mid-bringup now.

Tradeoffs

• Serial cost is the headline constraint. The design's premise is that code which never shares pays ~nothing. Phase 1 measured under 0.5% on our serial benchmarks with the flag on. That's a gate, not a hope — if ungil can't hold something close to that, it fails its own bench rung (R8) and that's a real "may not merge" outcome.
• Shared writes are slower, on purpose. First foreign write transitions the object; subsequent shared access goes through segmented storage and, on contended slow paths, cell locks. If your program hammers one object from 8 threads, it will not scale like a lock-free hashmap. The bet is that real programs mostly share read-heavy graphs and coordinate through a few hot objects, which is what Lock/Atomics are for.
• GC: concurrent marking is traded away in shared mode, for now. Flag off, the GC is byte-for-byte today's protocol — concurrent marking, incremental assist, all of it. With threads actually running, v1 collects synchronous and stop-the-world: all mutators stop, the parallel marking fleet (same thread count as today) marks inside the stop, conservative scanning covers every thread's stack and registers (you can't acquire heap access without registering for the scan). Extending the concurrent-GC protocol to N mutators is chartered, deferred work — pause times for thread-heavy, heap-heavy programs will reflect that until it lands.
• Memory: segmented butterflies fragment; deleted slots are quarantined until safepoints; watchpoint metadata exists per structure. None of it is free.
• Complexity is the real price. This touches the object model, all four execution tiers, the GC, and the VM lifecycle. The diff is ~60k lines. That's a maintenance burden on every future merge from upstream WebKit, and it's the strongest argument against merging.
• join() is blocking JS. Yes, that's deliberate — threads that can't block can't coordinate. Blocking is disallowed on threads where the embedder forbids it (the main thread of a browser-shaped embedder), where you get asyncJoin/asyncHold/waitAsync instead.

What was considered instead

• Just use workers + SAB. Already exists, doesn't solve the object-graph problem, and pushes every user into manual memory layout. If that were enough this branch wouldn't exist.
• A global lock forever (CPython-style). Phase 1 is that, and it's useful — but it gives you concurrency, not parallelism. We kept it as a supported fallback mode; it has to keep passing the corpus even after ungil.
• Per-object locking on every access. Pizlo's post benchmarks this class of approach; the serial slowdown is the reason the TID/watchpoint design exists. Paying a lock per property access to maybe-share is the wrong trade for a language where most objects never escape a thread.
• STM. Transactional semantics for JS object access is a research project with a long record of not shipping in production VMs. We didn't try to be the exception.
• Actor-ish ownership transfer (transferables for object graphs). Solves serialization cost, doesn't give you shared structures at all, and the ownership-transfer programming model has its own footguns. Thread.restrict is a small nod in this direction — opt-in confinement rather than opt-in sharing.

What it took to get here

The bring-up strategy was: land everything under a GIL first, use that as a semantic oracle, then remove the lock and let a fixed verification ladder tell you what you missed. Some numbers and the bugs that mattered, because they're the actual argument for whether this design works:

Design before code. Six frozen specs (~50KB each, hard-capped) covering heap, VM state, object model, JIT, API, and GIL removal — each beaten on by looped adversarial review until findings stopped being real. The GIL-removal spec alone went through ~30 review rounds and 32 recorded revisions. The hardest design corner wasn't the object model — it was lifecycle: what happens when a thread exits while the engine is stopping the world. Getting that right took five consecutive revisions (a teardown state machine, a real completion fence in ~VM, and a registration-time bit for threads whose TLS destructors never run — main threads, as it turns out). Every revision and what review caught is in docs/threads/*-history.md; nothing is retconned.

Phase 1 (GIL'd): the machinery. ~66k LOC: tagged/segmented butterflies, cell locks, TTL watchpoints, shared heap server, the full API, ~95-test corpus. Green, TSAN-clean with an empty suppression list, serial bench within 0.45%.

Phase 2 (GIL removal): the education. The corpus under real parallelism went 0-runnable → 33 → 88-of-91 passing (interpreter-only) across successive fix rounds. Each round had one dominant root cause, and the sequence is a tour of everything in a JS engine that silently assumes one thread:

• Stack-overflow checks: every tier's generated code compared the stack pointer against a limit stored in the shared VM — thread B was checking against thread A's stack. Rerouting that touched LLInt (three backends), Baseline, DFG, FTL, thunks, Yarr, and every C++ reader, gated behind a tripwire that refused multi-thread entry until every leg landed (a partial fix here means JIT code silently using a foreign stack limit — reviewers rejected two premature attempts, correctly).
• Exception state: the ThrowScope/ExceptionScope chain was anchored in the spawning thread's stack; a spawned thread that threw walked another thread's stack frames. 104 of 110 corpus failures at one point were this single mechanism.
• Stop-the-world: parked threads (in Atomics.wait, Lock, Condition) weren't polling the new per-thread stop words — "stop the world" waited 30 seconds for a world that couldn't hear it.
• OSR exits: two threads taking DFG exits simultaneously spilled both register files into the same scratch buffer — thread B resumed baseline execution holding thread A's object pointers. That was the source of the scariest crash signature (garbage structure IDs on live cells). Now per-thread, with exit compilation serialized and no runtime jump-patching under threads (a torn rel32 is itself a race).
• The long tail of per-VM scratch: regexp match vectors, string-search tables, date caches — each enumerated in two binding audits (every VM-singular field reachable from a spawned thread, each with a ruling: per-thread, locked, or refused) rather than discovered by crashing.

The invariants that held the whole way — these were gates, not goals: flag-off (useJSThreads=false) emits byte-identical code (verified at one point by dumping the live JIT bodies of a running process and diffing instruction streams against the pre-threads form); GIL-on mode stays a green fallback (92/0 every round); serial perf within 1% (one allocation-heavy bench is currently noise-bound on the build host — the disposition and measurement procedure are documented in-tree rather than hand-waved).

Status, current

• The thread test suite passes under real parallelism. ~93 tests covering the API, the shared object model, atomics on properties, deliberate race stress, thread lifecycle, and JIT interactions — green with the interpreter only, green with all JIT tiers enabled (held across three consecutive full runs), and green with aggressive tier-up thresholds forced. The race-stress tests additionally hold under repeat runs (5× each) and the heaviest object-model test holds at hundreds of runs under deliberate machine load.
• The safety modes are intact. The single-lock fallback mode passes everything (94/0). With threads disabled entirely, output is byte-for-byte identical to an untouched engine across a stress-test sample, and serial performance is within the 1% budget on 7 of 8 benchmarks — the 8th (an allocation/property-transition microbenchmark) sits ~3–4% over and is under investigation; it's the price of one of the correctness fixes and we haven't accepted it yet.
• The silent-corruption investigation is closed, with a controlled experiment. Early in the bring-up, a property read twice returned another property's value under heavy load. The cause was isolated to a shared string-cache race; the proof is causal, not statistical — re-enabling the pre-fix cache behavior behind a debug switch reproduces the corruption on demand (4/15 runs), while the fixed path shows the same race window firing with zero corruption (0/15), plus 240/240 clean runs under load on the shipped code.
• Thread-sanitizer: clean. Rebuilt the TSAN configuration around the real engine (assembly interpreter + JIT, instead of the C++ interpreter that only existed because TSAN can't instrument generated code). The honest starting count was ~10,600 reports; a batched campaign (fields converted to relaxed atomics where the memory model blesses the race, real bugs fixed — including routing inline-cache/call-link deallocation through epoch-based retirement) drove it to zero unsuppressed, with every suppression carrying a written justification. Races inside generated code itself are TSAN-invisible by nature; that coverage belongs to the stress/amplifier suites.
• Test suite keeps growing: a recent expansion (GC-stress modes, a parallel-scalability suite measuring the design's actual speedup claims, inline-cache state-machine races, per-thread stack-overflow/OOM injection) immediately found a 100%-reproducible livelock in the property-caching path that existed even with the lock on — now fixed. New tests have a habit of paying for themselves here.
• Concurrent GC marking with threads active: implemented, measured, and honestly not the win we hoped — yet. Marking now demonstrably overlaps execution (12–14% of collection wall time runs while programs execute, with ~40–66× more program progress during collections than stop-the-world). But on the big-program benchmark below it changes total wall time by less than noise (±2.4%), and it has a known crash when a thread exits mid-collection — so it stays off by default. The measurement that justified building it also demoted it: collector pauses turn out not to be where the time goes (see scalability section).
• Not started: Windows; running portions of test262 inside threads (planned as an end-stage semantics sweep).
• Hardening queued: a Fuzzilli setup with a custom thread-operations profile is built and smoke-tested; an audit of concurrency CVEs across JVM/HotSpot, V8/SpiderMonkey, and other runtimes mapped 20 bug-mechanism classes onto this design, with 37 targeted susceptibility tests written and ready to run.

If you want to poke at it: build with --useJSThreads=1, tests live in JSTests/threads/, design docs in docs/threads/ (the specs are deliberately frozen documents; the -history files show every revision and what review caught), and THREAD.md is the adapted design narrative.

Scalability, measured (the honest section)

The question that matters for this design is whether big programs get faster with more threads. To answer it the same multithreaded program — a document indexing and query engine, several phases, checksum-verified output — was written three times: JavaScript on this engine, Go, and Java, and run across 1–32 threads on a 64-core machine. Identical inputs, identical required outputs, deliberately identical (naive) locking structure in all three.

Current answer, after nine profiling-and-fix passes plus three exhaustive root-cause-hunt rounds: on the threading machinery itself, JavaScript beats Java at 16 and 32 threads. On the spec-exact workload, JavaScript is ~13× Java — and that gap is now cleanly attributed.

The flat arm (a checked-in variant that swaps strings/Map<string>/BigInt-PRNG for flat Int32Array storage and integer term-IDs, while keeping the concurrency surface — locks, barriers, threads, allocator, GC — byte-identical) isolates threading cost from object-model cost:

Threads	JS (flat)	Java	Go	Go (GOGC=off floor)
1	3759 ms	1974 ms	1836 ms	—
8	1010 ms	939 ms	535 ms	—
16	872 ms	976 ms	422 ms	354 ms
32	870 ms	1022 ms	378 ms	—

At 16 threads JS is 0.89× Java, and 2.46× the zero-GC Go floor (Java is at 2.76× of that floor — JS is now closer to the algorithmic ceiling than Java is). At 32 threads JS is 0.85× Java. The first measurement of this benchmark four weeks ago was negative scaling at every thread count.

The spec-exact arm (strings, Map<string>, arbitrary-precision BigInt — the program written the way idiomatic JS would write it):

Threads	JS	Java	Go
1	~19 600 ms	1974 ms	1836 ms
16	~13 000 ms	976 ms	422 ms

The 13× gap between the spec-exact arm and the flat arm at W=16 is the JS object-model cost on this workload, decomposed by the in-tree discriminating arms:

• ~40% heap-allocated BigInt (no native u64) — addressable by a "BigInt-stays-in-u64-range" fast path, separate PR
• ~50% strings + Map<string> lookup (the workload tokenizes synthetic text and hashes terms into a map; Java's String/HashMap are faster here)
• ~10% the threading machinery — closed by this branch

What three root-cause-hunt rounds found and fixed in the threading layer (each survived adversarial refutation by experiment):

• A generic upstream JSC bug where for (const x of <ints>) with a closure-captured x never reaches FTL (TDZ sentinel vs ValueProfile; bugzilla report drafted)
• lock.hold(fn) was paying full getCallData→JSC::call per call (117→35 ns via CachedCall fast dispatch)
• CodeBlock::jitCode() returning RefPtr by value → 16 threads bouncing one refcount cache line at every slow-path entry (~14% of W=16 on-CPU)
• The DFG OSR-exit lazy-compile path took a process-global lock and ran a full reconstruct before checking if the ramp already existed; under N mutators that lock fired inside held shard locks (the W=32 cliff)
• addPropertyTransitionToExistingStructureConcurrently took the source Structure's lock unconditionally on a read-only steady-state hit
• Three lazy species/watchpoint install races (50% SIGABRT at W=16)
• A single-handoff concurrent-GC window so the shared collector isn't a degenerate STW

Correctness held: every benchmark run at every thread count produced bit-identical checksums; the full test corpus and 40-test flag-off-identity gate stayed green after every change; W=32 is 30/30 crash-free; the CVE-mechanism-class re-audit (20 classes, 194 verdicts) closed three of five confirmed memory-safety items, with two and a documented residual list in progress.

If the ladder gets fully green — parallel JS passing the full corpus under TSAN with serial perf held — this becomes a real proposal. The machinery, the corpus, the discriminating arms, and the bring-up log are all checked in either way.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

💤 Files selected but had no reviewable changes (5)

• JSTests/threads/objectmodel/array-storage-property-transition.js
• JSTests/threads/objectmodel/cow-named-property-transition.js
• JSTests/threads/objectmodel/r47-foreign-dictionary-flatten.js
• JSTests/threads/objectmodel/r47-typedarray-slowdown-wastememory.js
• JSTests/threads/objectmodel/r48-typedarray-segmented-arraybuffer.js

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fe93c148-eb28-4e2b-bbf8-92948928047f

📥 Commits

Reviewing files that changed from the base of the PR and between f6a854f and 3a14f2a.

📒 Files selected for processing (5)

• JSTests/threads/objectmodel/array-storage-property-transition.js
• JSTests/threads/objectmodel/cow-named-property-transition.js
• JSTests/threads/objectmodel/r47-foreign-dictionary-flatten.js
• JSTests/threads/objectmodel/r47-typedarray-slowdown-wastememory.js
• JSTests/threads/objectmodel/r48-typedarray-segmented-arraybuffer.js

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This PR adds multiple thread-* Claude workflows, updates thread test harnesses and ignore rules, and expands JSTests/threads with new API, lifecycle, sync, shared-object, array, atomics, object-model, race, GC, vmstate, JIT, benchmark, and CVE regression coverage.

Changes

Cohort / File(s)	Summary
Workflow automation .claude/workflows/thread-*.js, .claude/workflows/aot-design.js	Adds many new workflow scripts and meta exports for thread prep/implement/fix/fuzz/scanners/TSAN/CVE/bughunt/closeout/spec/corpus/scalebench/AB-17 flows.
Harnesses, manifests, and repo ignore JSTests/threads.yaml, JSTests/threads/harness.js, JSTests/threads/resources/assert.js, JSTests/threads/bench/harness.js, .gitignore	Adds shared JSThreads assertions, timing/benchmark helpers, a test manifest, and broader ignore patterns for generated thread artifacts.
API, lifecycle, sync, shared-object, arrays, atomics JSTests/threads/api/*, JSTests/threads/lifecycle/*, JSTests/threads/sync/*, JSTests/threads/shared-objects/*, JSTests/threads/arrays/*, JSTests/threads/atomics/*	Expands coverage for Thread, Lock, Condition, ThreadLocal, joins, async behavior, Atomics property paths, shared object semantics, and array sharing/resizing.
Object model, races, heap, GC, vmstate, and congc JSTests/threads/objectmodel/*, JSTests/threads/invariants/*, JSTests/threads/races/*, JSTests/threads/heap-*, JSTests/threads/gc-stress/*, JSTests/threads/congc-*, JSTests/threads/vmstate/*	Adds multithreaded regression tests for object transitions, quarantines, GC interleavings, stack/exception state, microtask ordering, and vmstate consistency.
Bench, JIT, and deepwater tests JSTests/threads/bench/*, JSTests/threads/jit/*, JSTests/threads/checktraps-*, JSTests/threads/dw*	Adds benchmark scripts, JIT runner/lint/disassembly tooling, checktraps tests, and DeepWater regression cases.
CVE/regression corpus JSTests/threads/cve/*	Adds extensive CVE-style tests, crash logs, and diagnoses for GC, wait/notify, detach/resize, generator, rope, atomics, and JIT race scenarios.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description does not follow the repository template and omits the bug link, reviewed-by line, and file/change list.	Rewrite the PR description in the required template format, including a Bugzilla link, Reviewed by line, explanation, and changed-file bullets.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: experimental shared-memory threads in JavaScriptCore.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 20

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

Source/JavaScriptCore/bytecode/GetByIdMetadata.h (1)

263-291: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Similar ASSERT-only guard for ProtoLoad mode.

Line 268 asserts that setProtoLoadMode is unreachable under JS threads, but uses ASSERT which is compiled out in release builds. If the caller (setupGetByIdPrototypeCache or similar) is not also guarded, this will silently write a 16-byte record non-atomically, violating the concurrency contract.

🛡️ Proposed fix to add runtime guard

 inline void GetByIdModeMetadata::setProtoLoadMode(Structure* structure, PropertyOffset offset, JSObject* cachedSlot)
 {
     // SPEC-jit §4.3/I18: ProtoLoad's 16-byte record cannot be published as one
     // word; flag-on its sole installer (setupGetByIdPrototypeCache) is disabled
     // wholesale, so this must be unreachable.
-    ASSERT(!Options::useJSThreads());
+    RELEASE_ASSERT(!Options::useJSThreads());
 `#if` CPU(LITTLE_ENDIAN) && CPU(ADDRESS64)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/GetByIdMetadata.h` around lines 263 - 291,
Replace the compile-only ASSERT(!Options::useJSThreads()) in
GetByIdModeMetadata::setProtoLoadMode with a runtime guard: at the top of the
function check Options::useJSThreads() and bail out in release builds (e.g.
RELEASE_ASSERT_NOT_REACHED or an early return/explicit crash with a clear
message) so the 16-byte non-atomic write never executes under JS threads; also
ensure callers such as setupGetByIdPrototypeCache are similarly guarded so this
path cannot be reached when Options::useJSThreads() is true.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/workflows/thread-implement.js:
- Around line 259-260: The code silently truncates the ordered task list by
using plan.tasks.slice(0, 16) and assigns it to tasks, risking incomplete
implementations; change this so you don't drop tasks: either remove the slice
and use the full plan.tasks array (replace tasks = plan.tasks.slice(0, 16) with
tasks = plan.tasks) or, if a hard limit is required, explicitly detect when
plan.tasks.length > 16 and surface a blocker (throw or log an error including
w.key and the actual length) before proceeding so callers know the spec exceeded
capacity; reference the existing tasks variable, plan.tasks, and w.key when
making the change.
- Around line 235-239: The COMMON prompt template injects ${NO_SLOW} into every
agent prompt (symbol: COMMON and NO_SLOW), which contradicts the build runner's
instruction to execute "bun build.ts debug" and causes the build to be skipped;
modify the prompt construction so NO_SLOW is not appended for the build runner
(and the similar block around the second occurrence noted), e.g. split COMMON
into a base template and a no-slow suffix or add a conditional in the prompt
generator (e.g. makePromptFor(role) or the build-runner branch) that omits
NO_SLOW when role === 'build-runner' (or otherwise explicitly allow running the
build) to ensure the build runner receives an actionable, non-contradictory
prompt.

In @.claude/workflows/thread-ungil.js:
- Around line 33-38: The character class SAFE_PATH_RE currently permits '+'
(const SAFE_PATH_RE = /^[\w./+-]+$/), which is unusual for repo paths; update
SAFE_PATH_RE to remove '+' from the class so it no longer accepts plus signs,
then run tests and ensure safeScopePath continues to reject paths with '..' and
absolute paths outside REPO_ROOT; keep the existing logic in safeScopePath
unchanged (it references SAFE_PATH_RE and REPO_ROOT).

In `@JSTests/threads/api/thread-restrict.js`:
- Around line 1-9: The test file's header claims it's skipped pending the 9.2-6
hook ("SKIPPED until the 9.2-6 choke-point hook...") but there's no actual //@
skip directive; add a top-level `//@ skip` directive (or, if the hook is
integrated and the test is ready, remove the SKIPPED comment instead) in
JSTests/threads/api/thread-restrict.js so the file's runtime behavior matches
the comment.

In `@JSTests/threads/api/threadlocal-basic.js`:
- Line 52: The assertion should verify signed-zero semantics; replace the
fragile numeric equality shouldBe(tl2.value, -0) with a signed-zero check such
as using Object.is: assert that Object.is(tl2.value, -0) (or alternately assert
1 / tl2.value === -Infinity) so the test fails for +0 but passes only for -0;
update the line referencing tl2.value and shouldBe accordingly.

In `@JSTests/threads/arrays/push-resize-multithread.js`:
- Line 5: The test currently only loads assert.js but uses harness helpers
(Lock, spawnN, joinAll); update the top of the file to also import the harness
helpers by adding a load of harness.js (so Lock, spawnN, joinAll are defined)
alongside the existing load("../resources/assert.js", "caller relative") call;
ensure the new load uses the same "caller relative" path style so the references
to Lock, spawnN, and joinAll in the file work correctly.

In `@JSTests/threads/jit/lint.sh`:
- Around line 92-97: The test currently always prints the success message even
if a missing macro called by threadedButterflyReadPredicate,
threadedButterflyWritePredicate, or loadButterflyTIDTagToT4 was detected; update
the loop in lint.sh so failures terminate or set a failure flag and only call
pass "I14 LLInt choke macros present" when no failures occurred — e.g., exit
non‑zero inside the if that calls fail or introduce a boolean like
found_all=true that is set false on missing macros and only invoke pass when
found_all remains true.

In `@JSTests/threads/lifecycle/create-basics.js`:
- Line 4: The test is missing the harness import that provides spawnN and
joinAll; add a load for the harness module (same pattern as the existing
load("../resources/assert.js", "caller relative")) so spawnN and joinAll are
available to the test; update the top of create-basics.js to load
"../harness.js" (caller relative) before using spawnN and joinAll in the test.

In `@JSTests/threads/smoke.js`:
- Around line 106-108: Remove the dead spin loop that checks Atomics.load(futex,
"turn") !== 0 because futex.turn is initialized to 0 and the loop never
executes; delete the entire while (Atomics.load(futex, "turn") !== 0) { } block
and keep the crude warm-up spins, or if you actually need to wait for the waiter
thread to park, replace that loop with a real synchronization using
Atomics.wait/notify on futex and the "turn" field (e.g., Atomics.wait(futex,
"turn", expectedValue) / Atomics.notify) instead of Atomics.load.

In `@JSTests/threads/sync/condition-notify-all-multi-waiter.js`:
- Line 38: The hardcoded registered object (registered) doesn't scale with
WAITERS; replace the literal with dynamic initialization that builds keys
0..WAITERS-1 at runtime (e.g., create an empty object/array and loop or use
Array.from to populate indices) so registered's keys and initial values reflect
the current WAITERS constant; update any code that references registered to work
with the dynamically created structure.

In `@Source/JavaScriptCore/assembler/MacroAssemblerARM64.h`:
- Around line 6441-6461: Guard the Linux-only TLS helper with the Bun feature
flag: wrap the existing block that defines loadFromELFTLS64 and
loadFromELFTLS64NeedsMacroScratchRegister (and the m_assembler.mrs_TPIDR_EL0/use
of Address) so it is compiled only when both OS(LINUX) and
USE(BUN_JSC_ADDITIONS) are enabled (e.g. change the preprocessor condition to
require USE(BUN_JSC_ADDITIONS) or nest an `#if` USE(BUN_JSC_ADDITIONS) around the
block); ensure the RELEASE_ASSERT and function signatures remain unchanged and
the closing `#endif` comments reflect both conditions.

In `@Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h`:
- Around line 7424-7443: The Linux-only ELF TLS helpers (functions
loadFromELFTLS64 and loadFromELFTLS64NeedsMacroScratchRegister) expose
Bun-specific behavior; wrap the entire block currently guarded by OS(LINUX) with
an additional feature guard USE(BUN_JSC_ADDITIONS) so this path is only compiled
when Bun additions are enabled (i.e., change the conditional to require both
OS(LINUX) and USE(BUN_JSC_ADDITIONS)). Ensure both function declarations and
their RELEASE_ASSERT remain inside that combined guard.

In `@Source/JavaScriptCore/assembler/X86Assembler.h`:
- Line 199: The PRE_FS enum entry and the fs() declaration are Bun-specific and
must be wrapped with the feature guard; locate the PRE_FS symbol and the fs()
method declaration in X86Assembler.h (and the repeating occurrences around the
regions noted, e.g., the block covering lines ~4121-4129) and enclose them in a
conditional compilation block: add `#if` USE(BUN_JSC_ADDITIONS) before the PRE_FS
and fs() declarations and `#endif` after them so non-Bun builds do not see these
Bun-specific additions.

In `@Source/JavaScriptCore/bytecode/ArrayProfile.h`:
- Line 267: The method declaration for mayInterceptIndexedAccesses currently
ends with a double semicolon; remove the extra semicolon so the declaration
reads a single terminating semicolon. Edit the SUPPRESS_TSAN bool
mayInterceptIndexedAccesses(const ConcurrentJSLocker&) const { return
m_arrayProfileFlags.contains(ArrayProfileFlag::MayInterceptIndexedAccesses);; }
line to eliminate the trailing semicolon after the return expression, leaving
only one semicolon at the end of the statement.

In `@Source/JavaScriptCore/bytecode/CodeBlock.cpp`:
- Around line 986-1006: The code nulls m_jitData then only clears watchpoints
under Options::useJSThreads(), which leaves the DFGJITData returned by
dfgJITData() permanently leaked; instead ensure the JIT data is either retired
or transferred to the same leak/retire path used for m_jitCode. Locate the
dfgJITData()/m_jitData handling in CodeBlock::~CodeBlock (the block that calls
jitData->clearWatchpoints()) and: do not simply drop m_jitData before handling
ownership; after clearWatchpoints() either call the existing
retire/retireJITData-style API for DFGJITData (or push it into the same
leaked/retired container used for m_jitCode), or if no retire API exists, delete
jitData just like the non-threaded path — preserving the same safety commentary
— so that DFGJITData is not permanently leaked under --useJSThreads.

In `@Source/JavaScriptCore/bytecode/GetByIdMetadata.h`:
- Around line 219-227: The ASSERT in GetByIdModeMetadata::setUnsetMode only
protects in debug builds; replace it with a runtime guard so Unset mode is never
set under Options::useJSThreads() in release builds — e.g., check
Options::useJSThreads() at the top of setUnsetMode and abort/RELEASE_ASSERT if
true (or otherwise refuse to set mode), then proceed to set mode =
GetByIdMode::Unset, unsetMode.structureID = structure->id(), and
defaultMode.cachedOffset = 0 when safe; ensure you reference
GetByIdModeMetadata::setUnsetMode, Options::useJSThreads, and GetByIdMode::Unset
in your change.

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp`:
- Around line 3961-3964: The code currently prevents JIT-emitted structure-only
transitions under useJSThreads but still allows the shared handler path to
perform the unsafe structure-only transition: update setPrivateBrandHandler(VM&)
to include the same guard used in InlineCacheCompiler.cpp (i.e., check
Options::useJSThreads() and bail out or assert) so that
AccessCase::SetPrivateBrand routed via CommonJITThunkID::SetPrivateBrandHandler
cannot unconditionally write the new structure ID when useJSThreads is enabled;
mirror the defense-in-depth logic used in the JIT emission path and ensure the
handler returns/avoids the structure-only write if the guard trips.

In `@Source/JavaScriptCore/bytecode/JSThreadsSafepoint.cpp`:
- Around line 60-72: The JSThreads safepoint declarations
(jsThreadsThreadGranularStopTheWorldAndRun,
jsThreadsThreadGranularWorldIsStopped and the JSThreadsSafepoint namespace) are
Bun-specific and must be wrapped with the compile-time guard; modify the file so
these declarations are enclosed in a conditional block using
USE(BUN_JSC_ADDITIONS) (i.e. add the appropriate `#if` USE(BUN_JSC_ADDITIONS)
before the declarations and the matching `#endif` after) to ensure the Bun-only
threading feature is only compiled when the flag is enabled.

In `@Source/JavaScriptCore/bytecode/JSThreadsSafepoint.h`:
- Around line 28-214: Wrap the Bun-specific safepoint API in this header with
the feature guard: surround the JSThreadsSafepoint namespace and its
declarations (including stopTheWorldAndRun,
worldIsStopped(VM&)/worldIsStopped(), AlreadyStoppedWorldWitnessScope,
ClassAStopWatchdogContext, watchdogAssertStopProgress,
gilRemovalPreconditionsMetValue/gilRemovalPreconditionsMet, and related
declarations) with `#if` USE(BUN_JSC_ADDITIONS) ... `#endif`, and similarly guard
the corresponding includes/call sites that reference these symbols so non-Bun
builds neither parse nor require this API.

In `@Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp`:
- Around line 1116-1132: The displaced chain (displacedHead /
displacedInlinedHandler of type InlineCacheHandler) still contains codeBlock in
owners for nodes beyond the head, so before calling
RetiredJITArtifacts::retireHandlerChain you must walk the full displaced
chain(s) and call removeOwner(codeBlock) on every node (not just m_handler) —
similar to resetStubAsJumpInAccess(); locate where publishHandlerChainHead(...)
is called and after creating displacedHead/displacedInlinedHandler iterate each
handler node, invoking handler->removeOwner(codeBlock) for each, then proceed to
retireHandlerChain(vm, WTF::move(displacedHead)) and retireHandlerChain(vm,
WTF::move(displacedInlinedHandler)).

---

Outside diff comments:
In `@Source/JavaScriptCore/bytecode/GetByIdMetadata.h`:
- Around line 263-291: Replace the compile-only ASSERT(!Options::useJSThreads())
in GetByIdModeMetadata::setProtoLoadMode with a runtime guard: at the top of the
function check Options::useJSThreads() and bail out in release builds (e.g.
RELEASE_ASSERT_NOT_REACHED or an early return/explicit crash with a clear
message) so the 16-byte non-atomic write never executes under JS threads; also
ensure callers such as setupGetByIdPrototypeCache are similarly guarded so this
path cannot be reached when Options::useJSThreads() is true.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7f058a7b-c599-4dd3-b66f-d5e7923eed9a

📥 Commits

Reviewing files that changed from the base of the PR and between 5851d47 and 60ced36.

📒 Files selected for processing (300)

• .claude/workflows/thread-cve-audit.js
• .claude/workflows/thread-fix.js
• .claude/workflows/thread-fuzz.js
• .claude/workflows/thread-implement.js
• .claude/workflows/thread-prep.js
• .claude/workflows/thread-scanners.js
• .claude/workflows/thread-ungil-spec.js
• .claude/workflows/thread-ungil.js
• JSTests/threads.yaml
• JSTests/threads/api/blocking-gate.js
• JSTests/threads/api/condition-async-wait.js
• JSTests/threads/api/condition-basic.js
• JSTests/threads/api/condition-wait-termination.js
• JSTests/threads/api/lock-async-hold.js
• JSTests/threads/api/lock-basic.js
• JSTests/threads/api/lock-hold-termination.js
• JSTests/threads/api/park-no-microtask-drain.js
• JSTests/threads/api/thread-basic.js
• JSTests/threads/api/thread-ctor-errors.js
• JSTests/threads/api/thread-exc.js
• JSTests/threads/api/thread-id-bounds.js
• JSTests/threads/api/thread-lifecycle.js
• JSTests/threads/api/thread-restrict.js
• JSTests/threads/api/threadlocal-basic.js
• JSTests/threads/api/wasm-refused-sd7.js
• JSTests/threads/arrays/copy-on-write.js
• JSTests/threads/arrays/holes.js
• JSTests/threads/arrays/push-resize-multithread.js
• JSTests/threads/arrays/shared-element-read-write.js
• JSTests/threads/arrays/typed-arrays-sab.js
• JSTests/threads/atomics/property-cas-delete-undefined-sentinel-u5.js
• JSTests/threads/atomics/property-cas-dictionary-delete-u5.js
• JSTests/threads/atomics/property-cas-samevaluezero.js
• JSTests/threads/atomics/property-cas-storm-u28-flat.js
• JSTests/threads/atomics/property-cas-storm-u5-as.js
• JSTests/threads/atomics/property-errors.js
• JSTests/threads/atomics/property-load-store.js
• JSTests/threads/atomics/property-rmw.js
• JSTests/threads/atomics/property-store-missing-define-race.js
• JSTests/threads/atomics/property-wait-notify.js
• JSTests/threads/atomics/property-wait-termination.js
• JSTests/threads/atomics/property-waitasync-timeout.js
• JSTests/threads/atomics/property-wtr-isolation.js
• JSTests/threads/atomics/ta-path-unchanged.js
• JSTests/threads/atomics/ta-wait-thread-gate.js
• JSTests/threads/bench/array-element-read.js
• JSTests/threads/bench/array-element-write.js
• JSTests/threads/bench/flat-butterfly-read.js
• JSTests/threads/bench/flat-butterfly-write.js
• JSTests/threads/bench/harness.js
• JSTests/threads/bench/inline-property-read.js
• JSTests/threads/bench/inline-property-write.js
• JSTests/threads/bench/megamorphic-access.js
• JSTests/threads/bench/transition-heavy-constructor.js
• JSTests/threads/harness.js
• JSTests/threads/heap-access-blocking.js
• JSTests/threads/heap-allocation-storm.js
• JSTests/threads/heap-bench-allocation.js
• JSTests/threads/heap-client-churn.js
• JSTests/threads/heap-deferral-storm.js
• JSTests/threads/heap-epoch-reclaim.js
• JSTests/threads/heap-iss-revert.js
• JSTests/threads/heap-option-off.js
• JSTests/threads/heap-precise-storm.js
• JSTests/threads/heap-stop-interleavings.js
• JSTests/threads/invariants/delete-quarantine-dictionary.js
• JSTests/threads/invariants/delete-quarantine.js
• JSTests/threads/invariants/no-lost-elements.js
• JSTests/threads/invariants/no-lost-properties-same-name.js
• JSTests/threads/invariants/no-lost-properties.js
• JSTests/threads/invariants/no-time-travel.js
• JSTests/threads/invariants/no-torn-shapes.js
• JSTests/threads/jit/README.md
• JSTests/threads/jit/bench-gates.sh
• JSTests/threads/jit/construction-shared-constructor.js
• JSTests/threads/jit/fires-per-sec.js
• JSTests/threads/jit/ftl-osr-entry-catch-loop-amplifier.js
• JSTests/threads/jit/golden-disasm-corpus.js
• JSTests/threads/jit/golden-disasm.sh
• JSTests/threads/jit/ic-publish-reset-loops.js
• JSTests/threads/jit/int-gate-direct-call-relink.js
• JSTests/threads/jit/int-gate-epoch-reclaim.js
• JSTests/threads/jit/int-gate-fire-vs-execute.js
• JSTests/threads/jit/int-gate-jettison-vs-execute.js
• JSTests/threads/jit/int-gate-stop-budget.js
• JSTests/threads/jit/lint.sh
• JSTests/threads/jit/run-jit-tests.sh
• JSTests/threads/jit/shared-arraystorage-stress.js
• JSTests/threads/jit/spawned-thread-butterfly-stress.js
• JSTests/threads/jit/tag-discipline.js
• JSTests/threads/jit/tid-tag-3-threads.js
• JSTests/threads/lifecycle/async-join.js
• JSTests/threads/lifecycle/create-basics.js
• JSTests/threads/lifecycle/current-and-id.js
• JSTests/threads/lifecycle/exceptions-cross-join.js
• JSTests/threads/lifecycle/join-semantics.js
• JSTests/threads/lifecycle/nested-threads.js
• JSTests/threads/lifecycle/restrict-foreign-access.js.skip
• JSTests/threads/lifecycle/restrict.js
• JSTests/threads/lifecycle/return-values.js
• JSTests/threads/objectmodel/i03-array-resize-cas.js
• JSTests/threads/objectmodel/i03-as-shift-unshift.js
• JSTests/threads/objectmodel/i03-as-sparse-holes.js
• JSTests/threads/objectmodel/i03-b2-stay-flat-growth-vs-sw-flip.js
• JSTests/threads/objectmodel/i03-convert-grow-gc-read.js
• JSTests/threads/objectmodel/i03-cow-materialize-race.js
• JSTests/threads/objectmodel/i03-i37-same-shape-add-storm.js
• JSTests/threads/objectmodel/i03-n2-inline-add-races.js
• JSTests/threads/objectmodel/i03-n3-first-install-races.js
• JSTests/threads/objectmodel/i03-pa-global-races.js
• JSTests/threads/objectmodel/i03-quarantine-readd-across-gc.js
• JSTests/threads/objectmodel/i03-restart-locked-vs-conversion.js
• JSTests/threads/objectmodel/i03-selftest.js
• JSTests/threads/objectmodel/i03-shared-double.js
• JSTests/threads/objectmodel/i03-single-threaded-flag-on.js
• JSTests/threads/objectmodel/i03-single-threaded-no-change.js
• JSTests/threads/objectmodel/i03-stale-spine-reader-vs-grow.js
• JSTests/threads/objectmodel/i03-stress-force-segmented.js
• JSTests/threads/objectmodel/i03-stress-force-sw.js
• JSTests/threads/objectmodel/i03-t1-vs-sw-flip.js
• JSTests/threads/objectmodel/i03-t5-racing-growers.js
• JSTests/threads/objectmodel/i03-visit-range-outofline.js
• JSTests/threads/races/counter-atomics.js
• JSTests/threads/races/counter-lock.js
• JSTests/threads/races/join-storm.js
• JSTests/threads/races/transition-vs-read.js
• JSTests/threads/races/transition-vs-write.js
• JSTests/threads/races/wait-notify-storm.js
• JSTests/threads/resources/assert.js
• JSTests/threads/shared-objects/dictionary-mode.js
• JSTests/threads/shared-objects/frozen-sealed.js
• JSTests/threads/shared-objects/getters-setters.js
• JSTests/threads/shared-objects/property-add.js
• JSTests/threads/shared-objects/property-delete.js
• JSTests/threads/shared-objects/property-read-write.js
• JSTests/threads/shared-objects/prototype-chain.js
• JSTests/threads/smoke.js
• JSTests/threads/sync/atomics-futex-lock.js
• JSTests/threads/sync/atomics-object-basic.js
• JSTests/threads/sync/condition-notify-all-multi-waiter.js
• JSTests/threads/sync/condition-notify-all-shared-lock.js
• JSTests/threads/sync/condition-notify-all.js
• JSTests/threads/sync/condition-wait-notify.js
• JSTests/threads/sync/condition-worker-waiter.js
• JSTests/threads/sync/lock-async-hold.js
• JSTests/threads/sync/lock-hold-basic.js
• JSTests/threads/sync/lock-hold-mutual-exclusion.js
• JSTests/threads/sync/thread-local-isolation.js
• JSTests/threads/vmstate/README.md
• JSTests/threads/vmstate/all-flags-identity.js
• JSTests/threads/vmstate/exception-state-per-thread.js
• JSTests/threads/vmstate/flags-off-baseline.js
• JSTests/threads/vmstate/microtask-ordering.js
• JSTests/threads/vmstate/regexp-churn-threads.js
• JSTests/threads/vmstate/resources/workload.js
• JSTests/threads/vmstate/stack-limits-per-thread.js
• JSTests/threads/vmstate/structure-churn-dictionary.js
• JSTests/threads/vmstate/structure-churn-threads.js
• JSTests/threads/vmstate/structure-lock-single-thread.js
• JSTests/threads/vmstate/vmlite-single-thread-identity.js
• Source/JavaScriptCore/CMakeLists.txt
• Source/JavaScriptCore/Sources.txt
• Source/JavaScriptCore/assembler/ARM64Assembler.h
• Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
• Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h
• Source/JavaScriptCore/assembler/X86Assembler.h
• Source/JavaScriptCore/bytecode/ArrayProfile.cpp
• Source/JavaScriptCore/bytecode/ArrayProfile.h
• Source/JavaScriptCore/bytecode/BytecodeList.rb
• Source/JavaScriptCore/bytecode/CallLinkInfo.cpp
• Source/JavaScriptCore/bytecode/CallLinkInfo.h
• Source/JavaScriptCore/bytecode/CodeBlock.cpp
• Source/JavaScriptCore/bytecode/CodeBlock.h
• Source/JavaScriptCore/bytecode/ExecutionCounter.cpp
• Source/JavaScriptCore/bytecode/ExecutionCounter.h
• Source/JavaScriptCore/bytecode/GetByIdMetadata.h
• Source/JavaScriptCore/bytecode/GetByStatus.cpp
• Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp
• Source/JavaScriptCore/bytecode/InlineCacheCompiler.h
• Source/JavaScriptCore/bytecode/InlineCacheHandler.h
• Source/JavaScriptCore/bytecode/JSThreadsSafepoint.cpp
• Source/JavaScriptCore/bytecode/JSThreadsSafepoint.h
• Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp
• Source/JavaScriptCore/bytecode/PropertyInlineCache.h
• Source/JavaScriptCore/bytecode/Repatch.cpp
• Source/JavaScriptCore/bytecode/RetiredJITArtifacts.cpp
• Source/JavaScriptCore/bytecode/RetiredJITArtifacts.h
• Source/JavaScriptCore/bytecode/SharedJITStubSet.cpp
• Source/JavaScriptCore/bytecode/SharedJITStubSet.h
• Source/JavaScriptCore/bytecode/ValueProfile.h
• Source/JavaScriptCore/bytecode/Watchpoint.cpp
• Source/JavaScriptCore/bytecode/Watchpoint.h
• Source/JavaScriptCore/debugger/Debugger.cpp
• Source/JavaScriptCore/debugger/Debugger.h
• Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
• Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h
• Source/JavaScriptCore/dfg/DFGClobberize.h
• Source/JavaScriptCore/dfg/DFGCommonData.cpp
• Source/JavaScriptCore/dfg/DFGCommonData.h
• Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
• Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.cpp
• Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.h
• Source/JavaScriptCore/dfg/DFGJITCode.h
• Source/JavaScriptCore/dfg/DFGJumpReplacement.cpp
• Source/JavaScriptCore/dfg/DFGMayExit.cpp
• Source/JavaScriptCore/dfg/DFGOSREntry.cpp
• Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
• Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.h
• Source/JavaScriptCore/dfg/DFGOperations.cpp
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
• Source/JavaScriptCore/domjit/DOMJITEffect.h
• Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.cpp
• Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.h
• Source/JavaScriptCore/ftl/FTLJITCode.cpp
• Source/JavaScriptCore/ftl/FTLJITCode.h
• Source/JavaScriptCore/ftl/FTLJITFinalizer.cpp
• Source/JavaScriptCore/ftl/FTLLazySlowPath.cpp
• Source/JavaScriptCore/ftl/FTLLocation.cpp
• Source/JavaScriptCore/ftl/FTLLocation.h
• Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
• Source/JavaScriptCore/ftl/FTLOSREntry.cpp
• Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
• Source/JavaScriptCore/ftl/FTLOperations.cpp
• Source/JavaScriptCore/ftl/FTLSaveRestore.cpp
• Source/JavaScriptCore/ftl/FTLSaveRestore.h
• Source/JavaScriptCore/ftl/FTLState.cpp
• Source/JavaScriptCore/ftl/FTLThunks.cpp
• Source/JavaScriptCore/heap/AbstractSlotVisitorInlines.h
• Source/JavaScriptCore/heap/AllocatingScope.h
• Source/JavaScriptCore/heap/Allocator.h
• Source/JavaScriptCore/heap/BlockDirectory.cpp
• Source/JavaScriptCore/heap/BlockDirectory.h
• Source/JavaScriptCore/heap/BunV8HeapSnapshotBuilder.cpp
• Source/JavaScriptCore/heap/CellContainerInlines.h
• Source/JavaScriptCore/heap/CollectingScope.h
• Source/JavaScriptCore/heap/CompleteSubspace.cpp
• Source/JavaScriptCore/heap/CompleteSubspace.h
• Source/JavaScriptCore/heap/CompleteSubspaceInlines.h
• Source/JavaScriptCore/heap/GCActivityCallback.cpp
• Source/JavaScriptCore/heap/GCSafepointEpoch.cpp
• Source/JavaScriptCore/heap/GCSafepointEpoch.h
• Source/JavaScriptCore/heap/GCThreadLocalCache.cpp
• Source/JavaScriptCore/heap/GCThreadLocalCache.h
• Source/JavaScriptCore/heap/HandleSet.cpp
• Source/JavaScriptCore/heap/HandleSet.h
• Source/JavaScriptCore/heap/Heap.cpp
• Source/JavaScriptCore/heap/Heap.h
• Source/JavaScriptCore/heap/HeapCellInlines.h
• Source/JavaScriptCore/heap/HeapClientSet.cpp
• Source/JavaScriptCore/heap/HeapClientSet.h
• Source/JavaScriptCore/heap/HeapInlines.h
• Source/JavaScriptCore/heap/HeapIterationScope.h
• Source/JavaScriptCore/heap/HeapProfiler.h
• Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp
• Source/JavaScriptCore/heap/IncrementalSweeper.cpp
• Source/JavaScriptCore/heap/IsoCellSet.cpp
• Source/JavaScriptCore/heap/IsoSubspace.cpp
• Source/JavaScriptCore/heap/IsoSubspace.h
• Source/JavaScriptCore/heap/IsoSubspaceInlines.h
• Source/JavaScriptCore/heap/LocalAllocator.cpp
• Source/JavaScriptCore/heap/LocalAllocatorInlines.h
• Source/JavaScriptCore/heap/MachineStackMarker.cpp
• Source/JavaScriptCore/heap/MachineStackMarker.h
• Source/JavaScriptCore/heap/MarkedBlock.cpp
• Source/JavaScriptCore/heap/MarkedBlock.h
• Source/JavaScriptCore/heap/MarkedBlockInlines.h
• Source/JavaScriptCore/heap/MarkedSpace.cpp
• Source/JavaScriptCore/heap/MarkedSpace.h
• Source/JavaScriptCore/heap/PreciseAllocation.cpp
• Source/JavaScriptCore/heap/PreciseAllocation.h
• Source/JavaScriptCore/heap/PreciseSubspace.cpp
• Source/JavaScriptCore/heap/RunningScope.h
• Source/JavaScriptCore/heap/SharedHeapTestHarness.cpp
• Source/JavaScriptCore/heap/SharedHeapTestHarness.h
• Source/JavaScriptCore/heap/SlotVisitor.cpp
• Source/JavaScriptCore/heap/Strong.h
• Source/JavaScriptCore/heap/StrongInlines.h
• Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp
• Source/JavaScriptCore/heap/Subspace.cpp
• Source/JavaScriptCore/heap/SweepingScope.h
• Source/JavaScriptCore/heap/WeakBlock.cpp
• Source/JavaScriptCore/heap/WeakSet.cpp
• Source/JavaScriptCore/heap/WeakSet.h
• Source/JavaScriptCore/heap/WeakSetInlines.h
• Source/JavaScriptCore/interpreter/CLoopStack.cpp
• Source/JavaScriptCore/interpreter/CLoopStack.h
• Source/JavaScriptCore/interpreter/CLoopStackInlines.h
• Source/JavaScriptCore/interpreter/CallFrame.cpp
• Source/JavaScriptCore/interpreter/FrameTracers.h
• Source/JavaScriptCore/interpreter/Interpreter.cpp
• Source/JavaScriptCore/interpreter/InterpreterInlines.h
• Source/JavaScriptCore/interpreter/StackVisitor.cpp
• Source/JavaScriptCore/jit/AssemblyHelpers.cpp
• Source/JavaScriptCore/jit/CCallHelpers.cpp
• Source/JavaScriptCore/jit/CCallHelpers.h
• Source/JavaScriptCore/jit/ConcurrentButterflyOperations.cpp
• Source/JavaScriptCore/jit/ConcurrentButterflyOperations.h
• Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

The build runner inherits a hard “do not run the build” rule.

COMMON injects ${NO_SLOW} into every agent prompt, including the build runner. That same prompt then tells the agent to run bun build.ts debug, so the build phase is self-contradictory and can legitimately skip itself.

Also applies to: 364-370

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/workflows/thread-implement.js around lines 235 - 239, The COMMON
prompt template injects ${NO_SLOW} into every agent prompt (symbol: COMMON and
NO_SLOW), which contradicts the build runner's instruction to execute "bun
build.ts debug" and causes the build to be skipped; modify the prompt
construction so NO_SLOW is not appended for the build runner (and the similar
block around the second occurrence noted), e.g. split COMMON into a base
template and a no-slow suffix or add a conditional in the prompt generator (e.g.
makePromptFor(role) or the build-runner branch) that omits NO_SLOW when role ===
'build-runner' (or otherwise explicitly allow running the build) to ensure the
build runner receives an actionable, non-contradictory prompt.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't silently drop spec tasks after the first 16.

plan.tasks.slice(0, 16) truncates the ordered task list without surfacing a blocker. If any SPEC-*.md grows past 16 tasks, this workflow can converge on an incomplete implementation and still report success.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/workflows/thread-implement.js around lines 259 - 260, The code
silently truncates the ordered task list by using plan.tasks.slice(0, 16) and
assigns it to tasks, risking incomplete implementations; change this so you
don't drop tasks: either remove the slice and use the full plan.tasks array
(replace tasks = plan.tasks.slice(0, 16) with tasks = plan.tasks) or, if a hard
limit is required, explicitly detect when plan.tasks.length > 16 and surface a
blocker (throw or log an error including w.key and the actual length) before
proceeding so callers know the spec exceeded capacity; reference the existing
tasks variable, plan.tasks, and w.key when making the change.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Missing //@ skip directive despite comment claiming it exists.

The comment block (lines 4-8) states "the integrator deletes the //@ skip line when applying the 9.2-6 diff," but no such directive is present. If the 9.2-6 hook is not yet integrated, the foreign-thread CAE tests will fail when this test runs. Either add the skip directive or update the comment if the test is now ready to run.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/api/thread-restrict.js` around lines 1 - 9, The test file's
header claims it's skipped pending the 9.2-6 hook ("SKIPPED until the 9.2-6
choke-point hook...") but there's no actual //@ skip directive; add a top-level
`//@ skip` directive (or, if the hook is integrated and the test is ready,
remove the SKIPPED comment instead) in JSTests/threads/api/thread-restrict.js so
the file's runtime behavior matches the comment.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

-0 assertion is not actually validating signed zero.

shouldBe(tl2.value, -0) can pass for +0, so this check misses regressions in signed-zero preservation.

Proposed fix

-    shouldBe(tl2.value, -0);
+    shouldBeTrue(Object.is(tl2.value, -0), "ThreadLocal preserves -0");

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	shouldBe(tl2.value, -0);
	shouldBeTrue(Object.is(tl2.value, -0), "ThreadLocal preserves -0");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/api/threadlocal-basic.js` at line 52, The assertion should
verify signed-zero semantics; replace the fragile numeric equality
shouldBe(tl2.value, -0) with a signed-zero check such as using Object.is: assert
that Object.is(tl2.value, -0) (or alternately assert 1 / tl2.value ===
-Infinity) so the test fails for +0 but passes only for -0; update the line
referencing tl2.value and shouldBe accordingly.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Misleading pass after potential macro failures.

The pass on line 97 executes unconditionally even when fail was called for missing macros in the loop above. This produces confusing output like "LINT FAIL: ... macro missing" followed by "LINT pass: LLInt choke macros present".

Proposed fix

+macro_fail=0
 for macro in threadedButterflyReadPredicate threadedButterflyWritePredicate loadButterflyTIDTagToT4; do
     if ! grep -q "$macro" "$ASM"; then
         fail "I14: LLInt choke macro $macro missing from LowLevelInterpreter64.asm"
+        macro_fail=1
     fi
 done
-pass "I14 LLInt choke macros present"
+if [[ "$macro_fail" -eq 0 ]]; then
+    pass "I14 LLInt choke macros present"
+fi

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	for macro in threadedButterflyReadPredicate threadedButterflyWritePredicate loadButterflyTIDTagToT4; do
	if ! grep -q "$macro" "$ASM"; then
	fail "I14: LLInt choke macro $macro missing from LowLevelInterpreter64.asm"
	fi
	done
	pass "I14 LLInt choke macros present"
	macro_fail=0
	for macro in threadedButterflyReadPredicate threadedButterflyWritePredicate loadButterflyTIDTagToT4; do
	if ! grep -q "$macro" "$ASM"; then
	fail "I14: LLInt choke macro $macro missing from LowLevelInterpreter64.asm"
	macro_fail=1
	fi
	done
	if [[ "$macro_fail" -eq 0 ]]; then
	pass "I14 LLInt choke macros present"
	fi

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/jit/lint.sh` around lines 92 - 97, The test currently always
prints the success message even if a missing macro called by
threadedButterflyReadPredicate, threadedButterflyWritePredicate, or
loadButterflyTIDTagToT4 was detected; update the loop in lint.sh so failures
terminate or set a failure flag and only call pass "I14 LLInt choke macros
present" when no failures occurred — e.g., exit non‑zero inside the if that
calls fail or introduce a boolean like found_all=true that is set false on
missing macros and only invoke pass when found_all remains true.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Assert-only protection for Unset mode under JS threads may hide bugs in release builds.

Line 223 asserts that setUnsetMode is never called when Options::useJSThreads() is true, per the comment that Unset mode is "poison" for concurrent access. However, ASSERT is typically compiled out in release builds. If this code path is reachable in production (e.g., via dynamic dispatch or indirect calls), it will silently corrupt metadata.

🛡️ Proposed fix to add runtime guard

 inline void GetByIdModeMetadata::setUnsetMode(Structure* structure)
 {
     // SPEC-jit §4.3/I18: Unset mode is poison under JS threads (the asm reads
     // the mode byte and word 1 non-coherently); flag-on this must be unreachable.
-    ASSERT(!Options::useJSThreads());
+    RELEASE_ASSERT(!Options::useJSThreads());
     mode = GetByIdMode::Unset;
     unsetMode.structureID = structure->id();
     defaultMode.cachedOffset = 0;
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	inline void GetByIdModeMetadata::setUnsetMode(Structure* structure)
	{
	// SPEC-jit §4.3/I18: Unset mode is poison under JS threads (the asm reads
	// the mode byte and word 1 non-coherently); flag-on this must be unreachable.
	ASSERT(!Options::useJSThreads());
	mode = GetByIdMode::Unset;
	unsetMode.structureID = structure->id();
	defaultMode.cachedOffset = 0;
	}
	inline void GetByIdModeMetadata::setUnsetMode(Structure* structure)
	{
	// SPEC-jit §4.3/I18: Unset mode is poison under JS threads (the asm reads
	// the mode byte and word 1 non-coherently); flag-on this must be unreachable.
	RELEASE_ASSERT(!Options::useJSThreads());
	mode = GetByIdMode::Unset;
	unsetMode.structureID = structure->id();
	defaultMode.cachedOffset = 0;
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/GetByIdMetadata.h` around lines 219 - 227, The
ASSERT in GetByIdModeMetadata::setUnsetMode only protects in debug builds;
replace it with a runtime guard so Unset mode is never set under
Options::useJSThreads() in release builds — e.g., check Options::useJSThreads()
at the top of setUnsetMode and abort/RELEASE_ASSERT if true (or otherwise refuse
to set mode), then proceed to set mode = GetByIdMode::Unset,
unsetMode.structureID = structure->id(), and defaultMode.cachedOffset = 0 when
safe; ensure you reference GetByIdModeMetadata::setUnsetMode,
Options::useJSThreads, and GetByIdMode::Unset in your change.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Block the handler-IC SetPrivateBrand path too.

These lines only stop the JIT-emitted transition. compileOneAccessCaseHandler() still routes AccessCase::SetPrivateBrand to CommonJITThunkID::SetPrivateBrandHandler, and that thunk still writes the new structure ID unconditionally, so --useJSThreads can still take the unsafe structure-only transition through the shared handler path.

Suggested direction

// In compileOneAccessCaseHandler(...)

case AccessType::CheckPrivateBrand:
case AccessType::SetPrivateBrand: {
    switch (accessCase.m_type) {
    case AccessCase::CheckPrivateBrand:
        thunkID = CommonJITThunkID::CheckPrivateBrandHandler;
        break;
    case AccessCase::SetPrivateBrand:
+       if (Options::useJSThreads())
+           break;
        thunkID = CommonJITThunkID::SetPrivateBrandHandler;
        break;

Add the same guard inside setPrivateBrandHandler(VM&) as defense in depth.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp` around lines 3961 -
3964, The code currently prevents JIT-emitted structure-only transitions under
useJSThreads but still allows the shared handler path to perform the unsafe
structure-only transition: update setPrivateBrandHandler(VM&) to include the
same guard used in InlineCacheCompiler.cpp (i.e., check Options::useJSThreads()
and bail out or assert) so that AccessCase::SetPrivateBrand routed via
CommonJITThunkID::SetPrivateBrandHandler cannot unconditionally write the new
structure ID when useJSThreads is enabled; mirror the defense-in-depth logic
used in the JIT emission path and ensure the handler returns/avoids the
structure-only write if the guard trips.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing USE(BUN_JSC_ADDITIONS) guard for Bun-specific threading feature.

Per coding guidelines, Bun-specific features in JavaScriptCore should be guarded with USE(BUN_JSC_ADDITIONS). The JSThreads safepoint implementation is Bun-specific and should be conditionally compiled.

Suggested structure

+#if USE(BUN_JSC_ADDITIONS)
+
 namespace JSC {
 
 // UNGIL §A.3 thread-granular conductor...
 void jsThreadsThreadGranularStopTheWorldAndRun(VM&, const ScopedLambda<void()>&);
 bool jsThreadsThreadGranularWorldIsStopped();
 
 namespace JSThreadsSafepoint {
 // ... entire implementation ...
 } // namespace JSThreadsSafepoint
 } // namespace JSC
+
+#endif // USE(BUN_JSC_ADDITIONS)

As per coding guidelines: "Guard Bun-specific features with USE(BUN_JSC_ADDITIONS)".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/JSThreadsSafepoint.cpp` around lines 60 - 72,
The JSThreads safepoint declarations (jsThreadsThreadGranularStopTheWorldAndRun,
jsThreadsThreadGranularWorldIsStopped and the JSThreadsSafepoint namespace) are
Bun-specific and must be wrapped with the compile-time guard; modify the file so
these declarations are enclosed in a conditional block using
USE(BUN_JSC_ADDITIONS) (i.e. add the appropriate `#if` USE(BUN_JSC_ADDITIONS)
before the declarations and the matching `#endif` after) to ensure the Bun-only
threading feature is only compiled when the flag is enabled.

Source: Coding guidelines

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Guard this Bun-specific safepoint API with USE(BUN_JSC_ADDITIONS).

This header introduces Bun-only threading surfaces unconditionally into Source/JavaScriptCore/bytecode, which forces non-Bun builds to parse and satisfy the API as well. Please wrap the declarations here, and the matching includes/call sites, in the Bun feature guard.

As per coding guidelines, Source/JavaScriptCore/**/*.{cpp,h}: Guard Bun-specific features with USE(BUN_JSC_ADDITIONS) and event-loop integration with USE(BUN_EVENT_LOOP).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/JSThreadsSafepoint.h` around lines 28 - 214,
Wrap the Bun-specific safepoint API in this header with the feature guard:
surround the JSThreadsSafepoint namespace and its declarations (including
stopTheWorldAndRun, worldIsStopped(VM&)/worldIsStopped(),
AlreadyStoppedWorldWitnessScope, ClassAStopWatchdogContext,
watchdogAssertStopProgress,
gilRemovalPreconditionsMetValue/gilRemovalPreconditionsMet, and related
declarations) with `#if` USE(BUN_JSC_ADDITIONS) ... `#endif`, and similarly guard
the corresponding includes/call sites that reference these symbols so non-Bun
builds neither parse nor require this API.

Source: Coding guidelines

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Drop the owner from the whole displaced chain before retiring it.

This path removes codeBlock from the old head only, then retires displacedHead as a full chain. Older nodes still keep codeBlock in their owner lists, unlike resetStubAsJumpInAccess(), which walks every node first. Since the retired chain now outlives this CodeBlock, those stale back-pointers can survive past codeblock destruction.

♻️ Minimal fix sketch

-        if (m_handler)
-            m_handler->removeOwner(codeBlock);
+        if (m_handler) {
+            for (auto* cursor = m_handler.get(); cursor; cursor = cursor->next())
+                cursor->removeOwner(codeBlock);
+        }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp` around lines 1116 -
1132, The displaced chain (displacedHead / displacedInlinedHandler of type
InlineCacheHandler) still contains codeBlock in owners for nodes beyond the
head, so before calling RetiredJITArtifacts::retireHandlerChain you must walk
the full displaced chain(s) and call removeOwner(codeBlock) on every node (not
just m_handler) — similar to resetStubAsJumpInAccess(); locate where
publishHandlerChainHead(...) is called and after creating
displacedHead/displacedInlinedHandler iterate each handler node, invoking
handler->removeOwner(codeBlock) for each, then proceed to retireHandlerChain(vm,
WTF::move(displacedHead)) and retireHandlerChain(vm,
WTF::move(displacedInlinedHandler)).

[coderabbitai]

Actionable comments posted: 22

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

JSTests/threads/heap-allocation-storm.js (1)

21-35: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail when $vm.sharedHeapTest is unavailable instead of passing silently.

Line 34 prints PASS even when the guarded block never runs. That masks missing harness integration as a passing test.

Suggested fix

 if (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest === "function") {
@@
-}
-print("PASS");
+    print("PASS");
+} else
+    throw new Error("$vm.sharedHeapTest is required for heap-allocation-storm.js");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/heap-allocation-storm.js` around lines 21 - 35, The test
currently prints "PASS" even when the guarded block that uses $vm.sharedHeapTest
never runs; update the top-level guard around $vm.sharedHeapTest to detect when
$vm is undefined or $vm.sharedHeapTest is not a function and explicitly fail the
test instead of silently skipping: e.g., when typeof $vm === "undefined" ||
typeof $vm.sharedHeapTest !== "function", invoke a test failure (throw an Error
or call the existing test failure helper like shouldBeTrue(false,
"...")/shouldBe to indicate the missing harness) so the absence of
$vm.sharedHeapTest is reported as a test failure rather than letting
print("PASS") run.

Source/JavaScriptCore/bytecode/InlineCacheHandler.h (1)

35-61: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard Bun-specific refcounting behind USE(BUN_JSC_ADDITIONS).

Line 61 makes InlineCacheHandler thread-safe-refcounted unconditionally, so this Bun-threading behavior applies to non-Bun builds too. Gate the include/inheritance path with USE(BUN_JSC_ADDITIONS) and keep RefCounted for the default path.

Proposed patch

 `#include` <wtf/RefCounted.h>
-#include <wtf/ThreadSafeRefCounted.h>
+#if USE(BUN_JSC_ADDITIONS)
+#include <wtf/ThreadSafeRefCounted.h>
+#endif
@@
-class InlineCacheHandler : public ThreadSafeRefCounted<InlineCacheHandler> {
+class InlineCacheHandler
+#if USE(BUN_JSC_ADDITIONS)
+    : public ThreadSafeRefCounted<InlineCacheHandler>
+#else
+    : public RefCounted<InlineCacheHandler>
+#endif
+{

As per coding guidelines, Source/JavaScriptCore/**/*.{cpp,h}: Guard Bun-specific features with USE(BUN_JSC_ADDITIONS) and event-loop integration with USE(BUN_EVENT_LOOP).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/InlineCacheHandler.h` around lines 35 - 61,
The InlineCacheHandler class is currently inheriting ThreadSafeRefCounted
unconditionally; wrap the Bun-specific refcounting include and inheritance with
a compile-time guard: replace the unconditional `#include`
<wtf/ThreadSafeRefCounted.h> and the InlineCacheHandler : public
ThreadSafeRefCounted<InlineCacheHandler> path with a conditional compilation
that uses ThreadSafeRefCounted only when USE(BUN_JSC_ADDITIONS) is true and
falls back to RefCounted<InlineCacheHandler> (and its include) otherwise; update
the class declaration and related forward includes accordingly so non-Bun builds
keep RefCounted while Bun builds get ThreadSafeRefCounted.

Source: Coding guidelines

Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp (1)

1196-1204: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Verify patching safety assertion is reachable.

JSThreadsSafepoint::assertPatchingIsSafe is called here to enforce world-stopped discipline, but the function is guarded by RELEASE_ASSERT(!Options::useHandlerICInFTL()) above it. This means the assertion is only reachable when handler ICs are disabled in FTL. If the design intent is that this path is unreachable under useJSThreads(), consider adding an explicit RELEASE_ASSERT(!Options::useJSThreads()) for clarity, or document that the assertion is defense-in-depth for flag-off paths only.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp` around lines 1196 -
1204, The call to JSThreadsSafepoint::assertPatchingIsSafe is only reachable
when RELEASE_ASSERT(!Options::useHandlerICInFTL()) passes, which can be
confusing; update the branch to make the intended invariant explicit by adding a
guard or assertion for the threads flag: add
RELEASE_ASSERT(!Options::useJSThreads()) (or an equivalent comment) alongside
the existing RELEASE_ASSERT(!Options::useHandlerICInFTL()) before calling
JSThreadsSafepoint::assertPatchingIsSafe so the world-stopped discipline is
clearly enforced/expressed when this code path executes (references:
RELEASE_ASSERT(!Options::useHandlerICInFTL()), Options::useJSThreads(),
JSThreadsSafepoint::assertPatchingIsSafe).

♻️ Duplicate comments (11)

JSTests/threads/lifecycle/create-basics.js (1)

74-78: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing harness.js import for spawnN and joinAll.

Lines 74 and 78 use spawnN and joinAll, but only assert.js is loaded. Add the harness import to make these helpers available.

 load("../resources/assert.js", "caller relative");
+load("../harness.js", "caller relative");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/lifecycle/create-basics.js` around lines 74 - 78, The test is
using spawnN and joinAll but doesn't import the harness helpers; add the
harness.js import at the top of the file so spawnN and joinAll are available
(update the imports where assert.js is currently loaded to also require or load
"harness.js"); ensure the symbols spawnN and joinAll used in the code remain
unchanged so they resolve to the harness implementations.

Source/JavaScriptCore/bytecode/ArrayProfile.h (1)

267-267: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix double semicolon typo.

The method declaration ends with a double semicolon.

🔧 Proposed fix

-    SUPPRESS_TSAN bool mayInterceptIndexedAccesses(const ConcurrentJSLocker&) const { return m_arrayProfileFlags.contains(ArrayProfileFlag::MayInterceptIndexedAccesses);; }
+    SUPPRESS_TSAN bool mayInterceptIndexedAccesses(const ConcurrentJSLocker&) const { return m_arrayProfileFlags.contains(ArrayProfileFlag::MayInterceptIndexedAccesses); }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/ArrayProfile.h` at line 267, The method
declaration mayInterceptIndexedAccesses has an accidental double semicolon at
the end; open the function declaration for mayInterceptIndexedAccesses(const
ConcurrentJSLocker&) and remove the extra trailing semicolon so the line ends
with a single semicolon after the return expression that checks
m_arrayProfileFlags.contains(ArrayProfileFlag::MayInterceptIndexedAccesses).

Source/JavaScriptCore/assembler/X86Assembler.h (1)

199-200: ⚠️ Potential issue | 🟠 Major

Guard Bun-specific %fs prefix additions with USE(BUN_JSC_ADDITIONS).

PRE_FS and fs() are Bun/JSThreads-specific and are still exposed unconditionally, which can break non-Bun build surfaces.

Proposed fix

@@
-        PRE_FS                          = 0x64,
+#if USE(BUN_JSC_ADDITIONS)
+        PRE_FS                          = 0x64,
+#endif
@@
-    // Causes the memory access in the next instruction to be offset by %fs. On ELF/Linux
-    // x86-64, %fs is the thread pointer, so pairing this with a 32-bit absolute address
-    // load yields an initial-exec TLS load: the "address" is the (sign-extended, typically
-    // negative) TPOFF of the thread_local. Used for g_jscButterflyTIDTag (SPEC-jit-annex
-    // App. R5, Task 1b).
-    void fs()
-    {
-        m_formatter.prefix(PRE_FS);
-    }
+#if USE(BUN_JSC_ADDITIONS)
+    // Causes the memory access in the next instruction to be offset by %fs. On ELF/Linux
+    // x86-64, %fs is the thread pointer, so pairing this with a 32-bit absolute address
+    // load yields an initial-exec TLS load: the "address" is the (sign-extended, typically
+    // negative) TPOFF of the thread_local. Used for g_jscButterflyTIDTag (SPEC-jit-annex
+    // App. R5, Task 1b).
+    void fs()
+    {
+        m_formatter.prefix(PRE_FS);
+    }
+#endif

As per coding guidelines, "Source/JavaScriptCore//*.{cpp,h}: Guard Bun-specific features with USE(BUN_JSC_ADDITIONS) and event-loop integration with USE(BUN_EVENT_LOOP)"**.

Also applies to: 4121-4129

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/assembler/X86Assembler.h` around lines 199 - 200, The
PRE_FS enum value and the fs() helper are Bun-specific and must be guarded by
the build flag; wrap the PRE_FS = 0x64 and PRE_GS = 0x65 declarations (and the
fs() method/usage) with `#if` USE(BUN_JSC_ADDITIONS) / `#endif` so these symbols are
only defined when USE(BUN_JSC_ADDITIONS) is enabled; also apply the same guard
to the other Bun additions referenced around the same region (the code indicated
at the 4121-4129 area) to prevent exposing Bun-only prefixes in non-Bun builds.

Source: Coding guidelines

JSTests/threads/smoke.js (1)

106-106: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove the no-op pre-wait loop.

Line 106 always evaluates false here (futex.turn is 0 until Line 109), so this loop never synchronizes anything.

Suggested minimal fix

-while (Atomics.load(futex, "turn") !== 0) { }
 let spins = 0;
 while (spins++ < 1e7) { } // crude warm-up; wait() tolerates either ordering

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/smoke.js` at line 106, The busy-wait loop using
Atomics.load(futex, "turn") is a no-op because futex.turn is guaranteed to be 0
until later, so remove the loop at Line 106; simply delete the while
(Atomics.load(futex, "turn") !== 0) { } pre-wait and rely on the actual
synchronization that happens at the later Atomics.wait/Atomics.notify usage on
the futex array (references: futex variable and
Atomics.load/Atomics.wait/Atomics.notify calls) so the test no longer contains
an ineffective busy-wait.

Source/JavaScriptCore/bytecode/JSThreadsSafepoint.h (1)

34-216: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Wrap this Bun-specific safepoint surface with USE(BUN_JSC_ADDITIONS).

This header currently exports Bun-only JSThreads safepoint APIs unconditionally; they should be feature-gated for non-Bun builds.

Suggested shape

 namespace JSC {
 
 class VM;
 
+#if USE(BUN_JSC_ADDITIONS)
 namespace JSThreadsSafepoint {
@@
 } // namespace JSThreadsSafepoint
+#endif // USE(BUN_JSC_ADDITIONS)
 
 } // namespace JSC

As per coding guidelines, Source/JavaScriptCore/**/*.{cpp,h} must guard Bun-specific features with USE(BUN_JSC_ADDITIONS).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/JSThreadsSafepoint.h` around lines 34 - 216,
The header exposes Bun-only JSThreadsSafepoint APIs unguarded; wrap the entire
Bun-specific surface in a compile-time feature gate. Surround the
JSThreadsSafepoint namespace and its declarations (including stopTheWorldAndRun,
worldIsStopped(VM&)/worldIsStopped(), AlreadyStoppedWorldWitnessScope,
ClassAStopWatchdogContext, watchdogAssertStopProgress,
gilRemovalPreconditionsMetValue/gilRemovalPreconditionsMet, and
assertPatchingIsSafe overloads if they are Bun additions) with `#if`
USE(BUN_JSC_ADDITIONS) ... `#endif` so those symbols are only exported when the
BUN_JSC_ADDITIONS feature is enabled, preserving other platform builds.

Source: Coding guidelines

JSTests/threads/arrays/push-resize-multithread.js (1)

5-5: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing harness import for thread helper symbols.

Line 5 only loads assert.js, but this test uses Lock, spawnN, joinAll, and Thread; add the harness import so the file runs reliably.

Proposed fix

 load("../resources/assert.js", "caller relative");
+load("../harness.js", "caller relative");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/arrays/push-resize-multithread.js` at line 5, This test
currently only loads assert.js but uses harness thread helpers (Lock, spawnN,
joinAll, Thread); add the missing harness import that provides those symbols by
adding a load(...) call for the thread helper harness near the top of
push-resize-multithread.js (adjacent to the existing
load("../resources/assert.js")) so Lock, spawnN, joinAll and Thread are defined
before use.

JSTests/threads/jit/lint.sh (1)

92-97: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Unconditional success message still contradicts detected macro failures.

Line 97 prints a pass even when a missing macro was already reported in the loop, which makes lint output misleading.

Proposed minimal fix

+macro_fail=0
 for macro in threadedButterflyReadPredicate threadedButterflyWritePredicate loadButterflyTIDTagToT4; do
     if ! grep -q "$macro" "$ASM"; then
         fail "I14: LLInt choke macro $macro missing from LowLevelInterpreter64.asm"
+        macro_fail=1
     fi
 done
-pass "I14 LLInt choke macros present"
+if [[ "$macro_fail" -eq 0 ]]; then
+    pass "I14 LLInt choke macros present"
+fi

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/jit/lint.sh` around lines 92 - 97, The script prints a
success message unconditionally after checking for macros; change the logic in
the loop that greps for threadedButterflyReadPredicate,
threadedButterflyWritePredicate, and loadButterflyTIDTagToT4 in $ASM so that a
failure prevents printing pass "I14 LLInt choke macros present". Introduce a
boolean/status flag (or cause an immediate exit) before the for-loop, set it to
failure when any grep check fails (or call the existing fail handler so it
exits), and only call pass "I14 LLInt choke macros present" if the flag
indicates all macros were found; update the loop that contains the grep checks
to reference this flag and avoid the unconditional pass.

Source/JavaScriptCore/assembler/MacroAssemblerARM64.h (1)

6441-6461: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard this ELF TLS helper with USE(BUN_JSC_ADDITIONS).

Lines 6441-6461 add Bun/JSThreads-specific TLS plumbing behind OS(LINUX) only, so it still leaks into non-Bun Linux builds.

Proposed fix

-#if OS(LINUX)
+#if OS(LINUX) && USE(BUN_JSC_ADDITIONS)
     // ELF initial-exec TLS load: TPIDR_EL0 + ldr at a constant offset, baked
     // as an immediate at emission. The offset comes from
     // JSC::butterflyTIDTagELFTLSOffset() (jit/ConcurrentButterflyOperations.h),
@@
-#endif // OS(LINUX)
+#endif // OS(LINUX) && USE(BUN_JSC_ADDITIONS)

As per coding guidelines, “Source/JavaScriptCore/**/*.{cpp,h}: Guard Bun-specific features with USE(BUN_JSC_ADDITIONS) and event-loop integration with USE(BUN_EVENT_LOOP)”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/assembler/MacroAssemblerARM64.h` around lines 6441 -
6461, Wrap the ELF TLS helper declarations (the functions loadFromELFTLS64 and
loadFromELFTLS64NeedsMacroScratchRegister) with the USE(BUN_JSC_ADDITIONS)
feature macro so they are only compiled for Bun-specific builds; i.e. change the
current `#if` OS(LINUX) block to require both OS(LINUX) and USE(BUN_JSC_ADDITIONS)
(or nest an `#if` USE(BUN_JSC_ADDITIONS) inside) so these symbols are not exposed
in non-Bun Linux builds.

Source: Coding guidelines

Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp (1)

1115-1132: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Drop the owner from the whole displaced chain before retiring it.

Similar to the pattern in resetStubAsJumpInAccess() (lines 1224-1228), when displacing the handler chain in initializeWithUnitHandler(), the code should walk all nodes in displacedHead and call removeOwner(codeBlock) on each, not just the current head at line 1117. Older nodes in the displaced chain still keep codeBlock in their owner lists, which can cause stale back-pointers after the retired chain outlives codeblock destruction.

♻️ Minimal fix sketch

         if (Options::useJSThreads()) [[unlikely]] {
             // R2-1: COPY (never move out of) m_handler...
             RefPtr<InlineCacheHandler> displacedHead = m_handler;
+            // Walk the displaced chain and remove owner from ALL nodes
+            for (auto* cursor = displacedHead.get(); cursor; cursor = cursor->next())
+                cursor->removeOwner(codeBlock);
             publishHandlerChainHead(m_handler, WTF::move(handler));
             m_handler->addOwner(codeBlock);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp` around lines 1115 -
1132, In initializeWithUnitHandler(), when displacing the handler chain into
displacedHead before calling RetiredJITArtifacts::retireHandlerChain, walk the
entire displacedHead chain and call removeOwner(codeBlock) on each
InlineCacheHandler node (not just the head) to drop the codeBlock owner from all
nodes (same pattern used in resetStubAsJumpInAccess). After iterating and
removing owners from every node in displacedHead, proceed with
publishHandlerChainHead and the retireHandlerChain(wtf::move(displacedHead))
calls so no stale owner back-pointers remain after retirement.

Source/JavaScriptCore/bytecode/GetByIdMetadata.h (1)

263-268: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Same ASSERT-only protection issue in setProtoLoadMode.

Line 268 uses ASSERT(!Options::useJSThreads()) but the comment (lines 265-267) documents this mode as unreachable under JS threads because the 16-byte record cannot be published atomically. Consider RELEASE_ASSERT for consistency with the safety requirements.

🛡️ Proposed fix

 inline void GetByIdModeMetadata::setProtoLoadMode(Structure* structure, PropertyOffset offset, JSObject* cachedSlot)
 {
     // SPEC-jit §4.3/I18: ProtoLoad's 16-byte record cannot be published as one
     // word; flag-on its sole installer (setupGetByIdPrototypeCache) is disabled
     // wholesale, so this must be unreachable.
-    ASSERT(!Options::useJSThreads());
+    RELEASE_ASSERT(!Options::useJSThreads());

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/GetByIdMetadata.h` around lines 263 - 268, The
ASSERT-only check in GetByIdModeMetadata::setProtoLoadMode is insufficient for
release builds—replace or augment ASSERT(!Options::useJSThreads()) with a
RELEASE_ASSERT(!Options::useJSThreads()) (or otherwise ensure a release-time
guard) inside setProtoLoadMode so the unreachable-by-design assumption about
ProtoLoad's 16-byte record is enforced in production; update the assertion call
in the setProtoLoadMode function accordingly to match the safety pattern used
elsewhere.

Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp (1)

3961-3964: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Block the handler-IC SetPrivateBrand path too.

Line 3964 only closes the inline-emitted path. compileOneAccessCaseHandler() still selects CommonJITThunkID::SetPrivateBrandHandler, and setPrivateBrandHandler() still writes newStructureID unconditionally, so --useJSThreads can still reach the forbidden structure-only transition through the shared handler path.

Suggested direction

// In compileOneAccessCaseHandler(...)

case AccessCase::CheckPrivateBrand:
case AccessCase::SetPrivateBrand: {
    ASSERT(!accessCase.viaGlobalProxy());
    ASSERT(accessCase.conditionSet().isEmpty());
    CommonJITThunkID thunkID = CommonJITThunkID::CheckPrivateBrandHandler;
    switch (accessCase.m_type) {
    case AccessCase::CheckPrivateBrand:
        thunkID = CommonJITThunkID::CheckPrivateBrandHandler;
        break;
    case AccessCase::SetPrivateBrand:
+       if (Options::useJSThreads())
+           break;
        thunkID = CommonJITThunkID::SetPrivateBrandHandler;
        break;

MacroAssemblerCodeRef<JITThunkPtrTag> setPrivateBrandHandler(VM&)
{
    CCallHelpers jit;
    ...
+   if (Options::useJSThreads()) [[unlikely]]
+       fallThrough.append(jit.jump());
+
    jit.transfer32(CCallHelpers::Address(GPRInfo::handlerGPR, InlineCacheHandler::offsetOfNewStructureID()),
        CCallHelpers::Address(baseJSR.payloadGPR(), JSCell::structureIDOffset()));

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp` around lines 3961 -
3964, The inline path block at the top missed the shared handler path: update
compileOneAccessCaseHandler to avoid selecting
CommonJITThunkID::SetPrivateBrandHandler when Options::useJSThreads() is enabled
(mirror the RELEASE_ASSERT gating) or alternatively modify
setPrivateBrandHandler to only write newStructureID when
!Options::useJSThreads(); in short, ensure both compileOneAccessCaseHandler
(selection of SetPrivateBrandHandler) and setPrivateBrandHandler (the write of
newStructureID) are guarded so the structure-only transition cannot occur under
useJSThreads.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/workflows/thread-fix.js:
- Around line 128-140: The code currently only validates each item's scope shape
(via safeScopePath) but does not check that scopes are disjoint across items;
add a guard after computing items (the const items = ... block / after map(...)
and before slice(0,20) or immediately after) that compares scope file paths
across all items (use the normalized scope arrays on each item) and fails fast
if any two items share a path. Implement a simple overlap detection that
collects paths per item, finds intersections, and when found throws an Error (or
processLogger/error) naming the conflicting item ids (use item.id) so the fix
fan-out never runs on overlapping file scopes.

In @.claude/workflows/thread-prep.js:
- Line 454: The README/example line uses the flag form "--useThreads=true" while
the test files expect "--useJSThreads=1"; update the invocation in
.claude/workflows/thread-prep.js (the string "Each test self-contained:
./WebKitBuild/Debug/bin/jsc --useThreads=true <file>.") to use the same flag the
tests use (replace "--useThreads=true" with "--useJSThreads=1"), or
alternatively update the tests to accept "--useThreads" consistently—ensure the
project uses one canonical flag (prefer matching the test suite's
"--useJSThreads=1").

In @.claude/workflows/thread-ungil.js:
- Around line 199-202: The current fallback pushes the result of tasks.find(...)
without checking for undefined, which can lead to a crash when all remaining
tasks are unsatisfiable; update the block around ready.push(tasks.find(...)) to
capture the candidate (e.g., const candidate = tasks.find(t => !done.has(t.id)))
and only push it if candidate is truthy, otherwise handle the stuck state
explicitly (for example: log a clear error about unsatisfiable/circular deps and
abort/return/throw or mark remaining tasks as failed) so subsequent code that
expects a valid task (batch.push(t), t.id, etc.) never receives undefined.

In `@JSTests/threads/arrays/shared-element-read-write.js`:
- Line 4: The test fails at runtime because it uses harness utilities (Lock,
joinAll, spawnN) without importing harness.js; add a
load("../resources/harness.js", "caller relative"); at the top alongside the
existing load("../resources/assert.js", "caller relative"); so that Lock,
joinAll, and spawnN are defined before they are used in the test.

In `@JSTests/threads/atomics/property-cas-storm-u28-flat.js`:
- Around line 69-83: The CAS checks in the ropeThreads loop are unreliable
because Atomics.compareExchange(ropeObj, "s", ...) currently uses
constant-folded string literals so expected and replacement collapse to the same
value and you never detect success vs failure; fix by building distinct expected
and replacement strings at runtime and compare the returned old value to the
expected variable (not a hardcoded literal). Concretely, in the Thread body
create let expected1 = "left" + "right" and let replacement1 = "le" +
String.fromCharCode(102) + "tright" (or another runtime-built variant) then call
Atomics.compareExchange(ropeObj, "s", expected1, replacement1) and test returned
=== expected1 to increment swaps; do the analogous change for the second
compareExchange (use distinct expected2/replacement2 and compare returned ===
expected2) so swaps actually reflects CAS success for the compareExchange calls.

In `@JSTests/threads/heap-access-blocking.js`:
- Line 25: The test unconditionally prints print("PASS") even when
$vm.sharedHeapTest is false; update the control flow around the
$vm.sharedHeapTest check so that when $vm.sharedHeapTest is unavailable the
script prints an explicit skip message (e.g., "SKIP: sharedHeapTest
unavailable") and returns/ends early instead of printing "PASS", otherwise
continue running assertions and only print "PASS" after the assertions succeed;
adjust the logic that currently calls print("PASS") to be reached only in the
successful, feature-present branch (referencing $vm.sharedHeapTest and the
print("PASS") call).

In `@JSTests/threads/heap-client-churn.js`:
- Around line 19-29: The test prints "PASS" regardless of whether
$vm.sharedHeapTest ran because print("PASS") is outside the if block; fix by
ensuring PASS is only printed when the tests actually ran and succeeded—either
move the print("PASS") inside the if that checks typeof $vm and typeof
$vm.sharedHeapTest, or add an else branch that fails (throw or print a failing
message) when $vm.sharedHeapTest is unavailable; update references to
shouldBeTrue and shouldBe remain unchanged and ensure the success print occurs
only after those assertions complete.

In `@JSTests/threads/heap-stop-interleavings.js`:
- Line 29: The test currently unconditionally prints "PASS" via print("PASS")
even when the prerequisite $vm.sharedHeapTest is unavailable, which hides
skipped assertions; modify the test so that when $vm.sharedHeapTest is falsey it
emits a clear skip message (e.g., print("SKIP: shared heap tests not
supported")) or calls assert.throws/fail to explicitly fail instead of printing
PASS; locate the guard around $vm.sharedHeapTest and replace the unconditional
print("PASS") with a conditional that prints the skip message (or fails) only
when appropriate so PASS is printed only after actual assertions run.

In `@JSTests/threads/invariants/no-lost-properties.js`:
- Line 12: The test is missing the harness import for thread helpers: it uses
spawnN and joinAll but only loads assert.js; update
JSTests/threads/invariants/no-lost-properties.js to also load the harness that
exports those functions (add a load call that imports harness.js or the test
harness module used across other threading tests so spawnN and joinAll are
defined) ensuring the file now imports both "../resources/assert.js" and the
harness providing spawnN/joinAll before their use.

In `@JSTests/threads/jit/bench-gates.sh`:
- Around line 48-57: The script allows any value for RUNS but later median math
assumes a positive odd count, causing biased/invalid results; after parsing RUNS
(symbol RUNS) validate it is a positive integer and preferably odd (e.g., [[
"$RUNS" =~ ^[1-9][0-9]*$ ]] and (( RUNS % 2 == 1 )) ), and die with a clear
message if not; alternatively, if you want to allow even counts, update the
median calculation (the code that indexes the sorted results around the middle)
to handle even RUNS by averaging the two middle values and explicitly reject
RUNS==0 to avoid empty-result behavior.

In `@JSTests/threads/jit/int-gate-epoch-reclaim.js`:
- Around line 48-56: The dispatcher shutdown uses a plain object stop = { value:
false } which causes races when threads poll stop.value; change stop to a
SharedArrayBuffer-backed Int32Array flag and use Atomics.load/Atomics.store for
visibility instead: have the spawnN/dispatchers loop check
Atomics.load(stopArray, 0) and the shutdown path set Atomics.store(stopArray, 0,
1) (optionally Atomics.notify if you add waits), updating references to
stop.value in dispatchers and the writer at the current stop write site so all
threads see the change reliably in parallel mode (refer to stop.value,
dispatchers, spawnN, THREADS, getF, stableP, stableQ to locate the
reads/writes).

In `@JSTests/threads/jit/shared-arraystorage-stress.js`:
- Around line 49-79: The shared plain objects wave and done are racy; replace
them with atomic-backed counters (e.g., SharedArrayBuffer + Int32Array views)
and use Atomics for all reads/writes and notifications. Specifically, create
atomic counters (e.g., waveArr[0], doneArr[0]) used by the worker function
spawned via spawnN and by the owner loop, use Atomics.store/Atomics.load (or
Atomics.add) instead of direct property access, and use
Atomics.wait/Atomics.notify (or appropriate wake semantics) so waitUntil and the
worker loop observe updates reliably when using writeAt/readAt; update all
references to wave.n and done.count to the new atomic counters.

In `@JSTests/threads/jit/spawned-thread-butterfly-stress.js`:
- Around line 64-67: The increment of the shared rendezvous counter
ready.count++ is a non-atomic cross-thread RMW and can be lost; change the
barrier to use Atomics: replace the non-atomic increment with an atomic add
(Atomics.add) against a SharedArrayBuffer-backed Int32Array used for ready, and
make waitUntil check Atomics.load(...) of that same atomic slot (so the
waitUntil check uses the atomic value rather than ready.count). Locate uses
around registry[slot].push(buildOne(...)), the ready counter update, and the
waitUntil(...) call and switch them to Atomics.add/Atomics.load on the shared
Int32Array (consistent with how other shared counters in this test are
implemented).

In `@JSTests/threads/lifecycle/current-and-id.js`:
- Around line 52-65: The test can false-positive when the thread runs before
holder.self is assigned; update the Thread callback (the function passed to new
Thread stored in variable t3, and referencing holder/h) to verify h.self is set
before calling h.self.join() — e.g., if h.self is undefined return a distinct
result (not "threw:true") or spin/wait until h.self is defined, then call join
and only treat a thrown Error as a self-join success when h.self was present;
this ensures the t3 path distinguishes missing h.self from a real join
exception.

In `@JSTests/threads/objectmodel/i03-n2-inline-add-races.js`:
- Around line 49-67: The reader thread returns sawPresent but its return value
is ignored; capture the result of reader.join() (e.g. const saw = reader.join())
and assert it observed the published value with a test like shouldBe(saw, true,
"round " + round + ": reader didn't observe sentinel (I9)"); this ensures the I9
observation path is actually exercised (references: reader, sawPresent, Thread,
reader.join, o.fresh).

In `@JSTests/threads/shared-objects/frozen-sealed.js`:
- Line 179: Replace the boolean assertion shouldBe(report, false) with the
boolean-specific helper shouldBeFalse(report) to match the file's style; update
the assertion at the location using the variable report so it calls
shouldBeFalse(report) instead of shouldBe(report, false).

In `@JSTests/threads/shared-objects/property-delete.js`:
- Line 6: The test fails at runtime because it only loads assert.js but uses
harness utilities (Lock, joinAll, spawnN); add a load of the harness harness
that defines these symbols (e.g., load("../resources/harness.js", "caller
relative")) at the top of the file (near the existing
load("../resources/assert.js", "caller relative")) so Lock, joinAll and spawnN
are available to the test.

In `@JSTests/threads/shared-objects/prototype-chain.js`:
- Around line 146-151: The test mutates Object.prototype via Thread and
currently only deletes that property after assertions, which can leak if an
assertion fails; wrap the assertions that read {}.__sharedObjectsTestTemp in a
try/finally and perform the cleanup inside the finally block by invoking the
deletion in a Thread (use new Thread(() => delete
Object.prototype.__sharedObjectsTestTemp).join()) and assert the deletion
(shouldBeTrue(cleaned)); reference the Thread construct,
Object.prototype.__sharedObjectsTestTemp, and the delete invocation to locate
and refactor the code.

In `@JSTests/threads/vmstate/structure-churn-threads.js`:
- Line 16: The test is missing the harness import that provides spawnN and
joinAll; update the test to also load the harness harness module (e.g. add a
load("../resources/harness.js", "caller relative") alongside the existing
load("../resources/assert.js", "caller relative")) so spawnN and joinAll are
defined before they are used in the test.

In `@Source/JavaScriptCore/assembler/ARM64Assembler.h`:
- Around line 2694-2703: The mrs_TPIDR_EL0 TLS helper is Bun-specific but
currently exposed under only OS(LINUX); guard its declaration with the Bun
feature macro by wrapping it with USE(BUN_JSC_ADDITIONS) (either combine into
`#if` OS(LINUX) && USE(BUN_JSC_ADDITIONS) or nest `#if` USE(BUN_JSC_ADDITIONS)
around the existing block) so mrs_TPIDR_EL0(RegisterID dst) is only compiled
when Bun additions are enabled; also update the comment to note the
Bun/JSThreads tie-in if present.

In `@Source/JavaScriptCore/bytecode/PropertyInlineCache.h`:
- Around line 389-412: The new JSThreads/Bun-specific packed-word fields and
accessors were added to PropertyInlineCache unconditionally; wrap the
Bun/JSThreads-specific declarations and definitions with the feature guard
USE(BUN_JSC_ADDITIONS) so non-Bun builds preserve the original layout and
behavior. Specifically, enclose the declarations of setInlineAccessSelfState and
clearInlineAccessSelfState, the packedInlineAccessSelfWord helper, and any
related packed-word fields, static_asserts, and repatch assertions referenced
elsewhere (also the other ranges around lines 455-562 and 769-783) in `#if`
USE(BUN_JSC_ADDITIONS) / `#endif` so these symbols (PropertyInlineCache changes,
packedInlineAccessSelfWord, setInlineAccessSelfState,
clearInlineAccessSelfState) are only present when the Bun addition flag is
enabled.

In `@Source/JavaScriptCore/CMakeLists.txt`:
- Around line 292-299: The global add_compile_options(-mcx16) under the
WTF_CPU_X86_64 branch causes MSVC failures; change this to only add -mcx16 for
GCC/Clang toolchains (handle clang-cl as Clang) and avoid adding it when
CMAKE_CXX_COMPILER_ID is MSVC, and prefer applying it with
target_compile_options on the JSC target instead of globally; locate the
add_compile_options(-mcx16) occurrence in CMakeLists.txt (WTF_CPU_X86_64
section) and replace the global call with a compiler-ID guarded conditional that
adds the flag only for GNU/Clang compilers (or via target_compile_options(JSC
PRIVATE ...)) so MSVC builds are not passed -mcx16.

---

Outside diff comments:
In `@JSTests/threads/heap-allocation-storm.js`:
- Around line 21-35: The test currently prints "PASS" even when the guarded
block that uses $vm.sharedHeapTest never runs; update the top-level guard around
$vm.sharedHeapTest to detect when $vm is undefined or $vm.sharedHeapTest is not
a function and explicitly fail the test instead of silently skipping: e.g., when
typeof $vm === "undefined" || typeof $vm.sharedHeapTest !== "function", invoke a
test failure (throw an Error or call the existing test failure helper like
shouldBeTrue(false, "...")/shouldBe to indicate the missing harness) so the
absence of $vm.sharedHeapTest is reported as a test failure rather than letting
print("PASS") run.

In `@Source/JavaScriptCore/bytecode/InlineCacheHandler.h`:
- Around line 35-61: The InlineCacheHandler class is currently inheriting
ThreadSafeRefCounted unconditionally; wrap the Bun-specific refcounting include
and inheritance with a compile-time guard: replace the unconditional `#include`
<wtf/ThreadSafeRefCounted.h> and the InlineCacheHandler : public
ThreadSafeRefCounted<InlineCacheHandler> path with a conditional compilation
that uses ThreadSafeRefCounted only when USE(BUN_JSC_ADDITIONS) is true and
falls back to RefCounted<InlineCacheHandler> (and its include) otherwise; update
the class declaration and related forward includes accordingly so non-Bun builds
keep RefCounted while Bun builds get ThreadSafeRefCounted.

In `@Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp`:
- Around line 1196-1204: The call to JSThreadsSafepoint::assertPatchingIsSafe is
only reachable when RELEASE_ASSERT(!Options::useHandlerICInFTL()) passes, which
can be confusing; update the branch to make the intended invariant explicit by
adding a guard or assertion for the threads flag: add
RELEASE_ASSERT(!Options::useJSThreads()) (or an equivalent comment) alongside
the existing RELEASE_ASSERT(!Options::useHandlerICInFTL()) before calling
JSThreadsSafepoint::assertPatchingIsSafe so the world-stopped discipline is
clearly enforced/expressed when this code path executes (references:
RELEASE_ASSERT(!Options::useHandlerICInFTL()), Options::useJSThreads(),
JSThreadsSafepoint::assertPatchingIsSafe).

---

Duplicate comments:
In `@JSTests/threads/arrays/push-resize-multithread.js`:
- Line 5: This test currently only loads assert.js but uses harness thread
helpers (Lock, spawnN, joinAll, Thread); add the missing harness import that
provides those symbols by adding a load(...) call for the thread helper harness
near the top of push-resize-multithread.js (adjacent to the existing
load("../resources/assert.js")) so Lock, spawnN, joinAll and Thread are defined
before use.

In `@JSTests/threads/jit/lint.sh`:
- Around line 92-97: The script prints a success message unconditionally after
checking for macros; change the logic in the loop that greps for
threadedButterflyReadPredicate, threadedButterflyWritePredicate, and
loadButterflyTIDTagToT4 in $ASM so that a failure prevents printing pass "I14
LLInt choke macros present". Introduce a boolean/status flag (or cause an
immediate exit) before the for-loop, set it to failure when any grep check fails
(or call the existing fail handler so it exits), and only call pass "I14 LLInt
choke macros present" if the flag indicates all macros were found; update the
loop that contains the grep checks to reference this flag and avoid the
unconditional pass.

In `@JSTests/threads/lifecycle/create-basics.js`:
- Around line 74-78: The test is using spawnN and joinAll but doesn't import the
harness helpers; add the harness.js import at the top of the file so spawnN and
joinAll are available (update the imports where assert.js is currently loaded to
also require or load "harness.js"); ensure the symbols spawnN and joinAll used
in the code remain unchanged so they resolve to the harness implementations.

In `@JSTests/threads/smoke.js`:
- Line 106: The busy-wait loop using Atomics.load(futex, "turn") is a no-op
because futex.turn is guaranteed to be 0 until later, so remove the loop at Line
106; simply delete the while (Atomics.load(futex, "turn") !== 0) { } pre-wait
and rely on the actual synchronization that happens at the later
Atomics.wait/Atomics.notify usage on the futex array (references: futex variable
and Atomics.load/Atomics.wait/Atomics.notify calls) so the test no longer
contains an ineffective busy-wait.

In `@Source/JavaScriptCore/assembler/MacroAssemblerARM64.h`:
- Around line 6441-6461: Wrap the ELF TLS helper declarations (the functions
loadFromELFTLS64 and loadFromELFTLS64NeedsMacroScratchRegister) with the
USE(BUN_JSC_ADDITIONS) feature macro so they are only compiled for Bun-specific
builds; i.e. change the current `#if` OS(LINUX) block to require both OS(LINUX)
and USE(BUN_JSC_ADDITIONS) (or nest an `#if` USE(BUN_JSC_ADDITIONS) inside) so
these symbols are not exposed in non-Bun Linux builds.

In `@Source/JavaScriptCore/assembler/X86Assembler.h`:
- Around line 199-200: The PRE_FS enum value and the fs() helper are
Bun-specific and must be guarded by the build flag; wrap the PRE_FS = 0x64 and
PRE_GS = 0x65 declarations (and the fs() method/usage) with `#if`
USE(BUN_JSC_ADDITIONS) / `#endif` so these symbols are only defined when
USE(BUN_JSC_ADDITIONS) is enabled; also apply the same guard to the other Bun
additions referenced around the same region (the code indicated at the 4121-4129
area) to prevent exposing Bun-only prefixes in non-Bun builds.

In `@Source/JavaScriptCore/bytecode/ArrayProfile.h`:
- Line 267: The method declaration mayInterceptIndexedAccesses has an accidental
double semicolon at the end; open the function declaration for
mayInterceptIndexedAccesses(const ConcurrentJSLocker&) and remove the extra
trailing semicolon so the line ends with a single semicolon after the return
expression that checks
m_arrayProfileFlags.contains(ArrayProfileFlag::MayInterceptIndexedAccesses).

In `@Source/JavaScriptCore/bytecode/GetByIdMetadata.h`:
- Around line 263-268: The ASSERT-only check in
GetByIdModeMetadata::setProtoLoadMode is insufficient for release builds—replace
or augment ASSERT(!Options::useJSThreads()) with a
RELEASE_ASSERT(!Options::useJSThreads()) (or otherwise ensure a release-time
guard) inside setProtoLoadMode so the unreachable-by-design assumption about
ProtoLoad's 16-byte record is enforced in production; update the assertion call
in the setProtoLoadMode function accordingly to match the safety pattern used
elsewhere.

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp`:
- Around line 3961-3964: The inline path block at the top missed the shared
handler path: update compileOneAccessCaseHandler to avoid selecting
CommonJITThunkID::SetPrivateBrandHandler when Options::useJSThreads() is enabled
(mirror the RELEASE_ASSERT gating) or alternatively modify
setPrivateBrandHandler to only write newStructureID when
!Options::useJSThreads(); in short, ensure both compileOneAccessCaseHandler
(selection of SetPrivateBrandHandler) and setPrivateBrandHandler (the write of
newStructureID) are guarded so the structure-only transition cannot occur under
useJSThreads.

In `@Source/JavaScriptCore/bytecode/JSThreadsSafepoint.h`:
- Around line 34-216: The header exposes Bun-only JSThreadsSafepoint APIs
unguarded; wrap the entire Bun-specific surface in a compile-time feature gate.
Surround the JSThreadsSafepoint namespace and its declarations (including
stopTheWorldAndRun, worldIsStopped(VM&)/worldIsStopped(),
AlreadyStoppedWorldWitnessScope, ClassAStopWatchdogContext,
watchdogAssertStopProgress,
gilRemovalPreconditionsMetValue/gilRemovalPreconditionsMet, and
assertPatchingIsSafe overloads if they are Bun additions) with `#if`
USE(BUN_JSC_ADDITIONS) ... `#endif` so those symbols are only exported when the
BUN_JSC_ADDITIONS feature is enabled, preserving other platform builds.

In `@Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp`:
- Around line 1115-1132: In initializeWithUnitHandler(), when displacing the
handler chain into displacedHead before calling
RetiredJITArtifacts::retireHandlerChain, walk the entire displacedHead chain and
call removeOwner(codeBlock) on each InlineCacheHandler node (not just the head)
to drop the codeBlock owner from all nodes (same pattern used in
resetStubAsJumpInAccess). After iterating and removing owners from every node in
displacedHead, proceed with publishHandlerChainHead and the
retireHandlerChain(wtf::move(displacedHead)) calls so no stale owner
back-pointers remain after retirement.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d8621faf-1978-409f-8842-1a7341e14a22

📥 Commits

Reviewing files that changed from the base of the PR and between 5851d47 and 5d1745d.

📒 Files selected for processing (300)

• .claude/workflows/thread-cve-audit.js
• .claude/workflows/thread-fix.js
• .claude/workflows/thread-fuzz.js
• .claude/workflows/thread-implement.js
• .claude/workflows/thread-prep.js
• .claude/workflows/thread-scanners.js
• .claude/workflows/thread-ungil-spec.js
• .claude/workflows/thread-ungil.js
• JSTests/threads.yaml
• JSTests/threads/api/blocking-gate.js
• JSTests/threads/api/condition-async-wait.js
• JSTests/threads/api/condition-basic.js
• JSTests/threads/api/condition-wait-termination.js
• JSTests/threads/api/lock-async-hold.js
• JSTests/threads/api/lock-basic.js
• JSTests/threads/api/lock-hold-termination.js
• JSTests/threads/api/park-no-microtask-drain.js
• JSTests/threads/api/thread-basic.js
• JSTests/threads/api/thread-ctor-errors.js
• JSTests/threads/api/thread-exc.js
• JSTests/threads/api/thread-id-bounds.js
• JSTests/threads/api/thread-lifecycle.js
• JSTests/threads/api/thread-restrict.js
• JSTests/threads/api/threadlocal-basic.js
• JSTests/threads/api/wasm-refused-sd7.js
• JSTests/threads/arrays/copy-on-write.js
• JSTests/threads/arrays/holes.js
• JSTests/threads/arrays/push-resize-multithread.js
• JSTests/threads/arrays/shared-element-read-write.js
• JSTests/threads/arrays/typed-arrays-sab.js
• JSTests/threads/atomics/property-cas-delete-undefined-sentinel-u5.js
• JSTests/threads/atomics/property-cas-dictionary-delete-u5.js
• JSTests/threads/atomics/property-cas-samevaluezero.js
• JSTests/threads/atomics/property-cas-storm-u28-flat.js
• JSTests/threads/atomics/property-cas-storm-u5-as.js
• JSTests/threads/atomics/property-errors.js
• JSTests/threads/atomics/property-load-store.js
• JSTests/threads/atomics/property-rmw.js
• JSTests/threads/atomics/property-store-missing-define-race.js
• JSTests/threads/atomics/property-wait-notify.js
• JSTests/threads/atomics/property-wait-termination.js
• JSTests/threads/atomics/property-waitasync-timeout.js
• JSTests/threads/atomics/property-wtr-isolation.js
• JSTests/threads/atomics/ta-path-unchanged.js
• JSTests/threads/atomics/ta-wait-thread-gate.js
• JSTests/threads/bench/array-element-read.js
• JSTests/threads/bench/array-element-write.js
• JSTests/threads/bench/flat-butterfly-read.js
• JSTests/threads/bench/flat-butterfly-write.js
• JSTests/threads/bench/harness.js
• JSTests/threads/bench/inline-property-read.js
• JSTests/threads/bench/inline-property-write.js
• JSTests/threads/bench/megamorphic-access.js
• JSTests/threads/bench/transition-heavy-constructor.js
• JSTests/threads/harness.js
• JSTests/threads/heap-access-blocking.js
• JSTests/threads/heap-allocation-storm.js
• JSTests/threads/heap-bench-allocation.js
• JSTests/threads/heap-client-churn.js
• JSTests/threads/heap-deferral-storm.js
• JSTests/threads/heap-epoch-reclaim.js
• JSTests/threads/heap-iss-revert.js
• JSTests/threads/heap-option-off.js
• JSTests/threads/heap-precise-storm.js
• JSTests/threads/heap-stop-interleavings.js
• JSTests/threads/invariants/delete-quarantine-dictionary.js
• JSTests/threads/invariants/delete-quarantine.js
• JSTests/threads/invariants/no-lost-elements.js
• JSTests/threads/invariants/no-lost-properties-same-name.js
• JSTests/threads/invariants/no-lost-properties.js
• JSTests/threads/invariants/no-time-travel.js
• JSTests/threads/invariants/no-torn-shapes.js
• JSTests/threads/jit/README.md
• JSTests/threads/jit/bench-gates.sh
• JSTests/threads/jit/construction-shared-constructor.js
• JSTests/threads/jit/fires-per-sec.js
• JSTests/threads/jit/ftl-osr-entry-catch-loop-amplifier.js
• JSTests/threads/jit/golden-disasm-corpus.js
• JSTests/threads/jit/golden-disasm.sh
• JSTests/threads/jit/ic-publish-reset-loops.js
• JSTests/threads/jit/int-gate-direct-call-relink.js
• JSTests/threads/jit/int-gate-epoch-reclaim.js
• JSTests/threads/jit/int-gate-fire-vs-execute.js
• JSTests/threads/jit/int-gate-jettison-vs-execute.js
• JSTests/threads/jit/int-gate-stop-budget.js
• JSTests/threads/jit/lint.sh
• JSTests/threads/jit/run-jit-tests.sh
• JSTests/threads/jit/shared-arraystorage-stress.js
• JSTests/threads/jit/spawned-thread-butterfly-stress.js
• JSTests/threads/jit/tag-discipline.js
• JSTests/threads/jit/tid-tag-3-threads.js
• JSTests/threads/lifecycle/async-join.js
• JSTests/threads/lifecycle/create-basics.js
• JSTests/threads/lifecycle/current-and-id.js
• JSTests/threads/lifecycle/exceptions-cross-join.js
• JSTests/threads/lifecycle/join-semantics.js
• JSTests/threads/lifecycle/nested-threads.js
• JSTests/threads/lifecycle/restrict-foreign-access.js.skip
• JSTests/threads/lifecycle/restrict.js
• JSTests/threads/lifecycle/return-values.js
• JSTests/threads/objectmodel/i03-array-resize-cas.js
• JSTests/threads/objectmodel/i03-as-shift-unshift.js
• JSTests/threads/objectmodel/i03-as-sparse-holes.js
• JSTests/threads/objectmodel/i03-b2-stay-flat-growth-vs-sw-flip.js
• JSTests/threads/objectmodel/i03-convert-grow-gc-read.js
• JSTests/threads/objectmodel/i03-cow-materialize-race.js
• JSTests/threads/objectmodel/i03-i37-same-shape-add-storm.js
• JSTests/threads/objectmodel/i03-n2-inline-add-races.js
• JSTests/threads/objectmodel/i03-n3-first-install-races.js
• JSTests/threads/objectmodel/i03-pa-global-races.js
• JSTests/threads/objectmodel/i03-quarantine-readd-across-gc.js
• JSTests/threads/objectmodel/i03-restart-locked-vs-conversion.js
• JSTests/threads/objectmodel/i03-selftest.js
• JSTests/threads/objectmodel/i03-shared-double.js
• JSTests/threads/objectmodel/i03-single-threaded-flag-on.js
• JSTests/threads/objectmodel/i03-single-threaded-no-change.js
• JSTests/threads/objectmodel/i03-stale-spine-reader-vs-grow.js
• JSTests/threads/objectmodel/i03-stress-force-segmented.js
• JSTests/threads/objectmodel/i03-stress-force-sw.js
• JSTests/threads/objectmodel/i03-t1-vs-sw-flip.js
• JSTests/threads/objectmodel/i03-t5-racing-growers.js
• JSTests/threads/objectmodel/i03-visit-range-outofline.js
• JSTests/threads/races/counter-atomics.js
• JSTests/threads/races/counter-lock.js
• JSTests/threads/races/join-storm.js
• JSTests/threads/races/transition-vs-read.js
• JSTests/threads/races/transition-vs-write.js
• JSTests/threads/races/wait-notify-storm.js
• JSTests/threads/resources/assert.js
• JSTests/threads/shared-objects/dictionary-mode.js
• JSTests/threads/shared-objects/frozen-sealed.js
• JSTests/threads/shared-objects/getters-setters.js
• JSTests/threads/shared-objects/property-add.js
• JSTests/threads/shared-objects/property-delete.js
• JSTests/threads/shared-objects/property-read-write.js
• JSTests/threads/shared-objects/prototype-chain.js
• JSTests/threads/smoke.js
• JSTests/threads/sync/atomics-futex-lock.js
• JSTests/threads/sync/atomics-object-basic.js
• JSTests/threads/sync/condition-notify-all-multi-waiter.js
• JSTests/threads/sync/condition-notify-all-shared-lock.js
• JSTests/threads/sync/condition-notify-all.js
• JSTests/threads/sync/condition-wait-notify.js
• JSTests/threads/sync/condition-worker-waiter.js
• JSTests/threads/sync/lock-async-hold.js
• JSTests/threads/sync/lock-hold-basic.js
• JSTests/threads/sync/lock-hold-mutual-exclusion.js
• JSTests/threads/sync/thread-local-isolation.js
• JSTests/threads/vmstate/README.md
• JSTests/threads/vmstate/all-flags-identity.js
• JSTests/threads/vmstate/exception-state-per-thread.js
• JSTests/threads/vmstate/flags-off-baseline.js
• JSTests/threads/vmstate/microtask-ordering.js
• JSTests/threads/vmstate/regexp-churn-threads.js
• JSTests/threads/vmstate/resources/workload.js
• JSTests/threads/vmstate/stack-limits-per-thread.js
• JSTests/threads/vmstate/structure-churn-dictionary.js
• JSTests/threads/vmstate/structure-churn-threads.js
• JSTests/threads/vmstate/structure-lock-single-thread.js
• JSTests/threads/vmstate/vmlite-single-thread-identity.js
• Source/JavaScriptCore/CMakeLists.txt
• Source/JavaScriptCore/Sources.txt
• Source/JavaScriptCore/assembler/ARM64Assembler.h
• Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
• Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h
• Source/JavaScriptCore/assembler/X86Assembler.h
• Source/JavaScriptCore/bytecode/ArrayProfile.cpp
• Source/JavaScriptCore/bytecode/ArrayProfile.h
• Source/JavaScriptCore/bytecode/BytecodeList.rb
• Source/JavaScriptCore/bytecode/CallLinkInfo.cpp
• Source/JavaScriptCore/bytecode/CallLinkInfo.h
• Source/JavaScriptCore/bytecode/CodeBlock.cpp
• Source/JavaScriptCore/bytecode/CodeBlock.h
• Source/JavaScriptCore/bytecode/ExecutionCounter.cpp
• Source/JavaScriptCore/bytecode/ExecutionCounter.h
• Source/JavaScriptCore/bytecode/GetByIdMetadata.h
• Source/JavaScriptCore/bytecode/GetByStatus.cpp
• Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp
• Source/JavaScriptCore/bytecode/InlineCacheCompiler.h
• Source/JavaScriptCore/bytecode/InlineCacheHandler.h
• Source/JavaScriptCore/bytecode/JSThreadsSafepoint.cpp
• Source/JavaScriptCore/bytecode/JSThreadsSafepoint.h
• Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp
• Source/JavaScriptCore/bytecode/PropertyInlineCache.h
• Source/JavaScriptCore/bytecode/Repatch.cpp
• Source/JavaScriptCore/bytecode/RetiredJITArtifacts.cpp
• Source/JavaScriptCore/bytecode/RetiredJITArtifacts.h
• Source/JavaScriptCore/bytecode/SharedJITStubSet.cpp
• Source/JavaScriptCore/bytecode/SharedJITStubSet.h
• Source/JavaScriptCore/bytecode/ValueProfile.h
• Source/JavaScriptCore/bytecode/Watchpoint.cpp
• Source/JavaScriptCore/bytecode/Watchpoint.h
• Source/JavaScriptCore/debugger/Debugger.cpp
• Source/JavaScriptCore/debugger/Debugger.h
• Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
• Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h
• Source/JavaScriptCore/dfg/DFGClobberize.h
• Source/JavaScriptCore/dfg/DFGCommonData.cpp
• Source/JavaScriptCore/dfg/DFGCommonData.h
• Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
• Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.cpp
• Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.h
• Source/JavaScriptCore/dfg/DFGJITCode.h
• Source/JavaScriptCore/dfg/DFGJumpReplacement.cpp
• Source/JavaScriptCore/dfg/DFGMayExit.cpp
• Source/JavaScriptCore/dfg/DFGOSREntry.cpp
• Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
• Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.h
• Source/JavaScriptCore/dfg/DFGOperations.cpp
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
• Source/JavaScriptCore/domjit/DOMJITEffect.h
• Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.cpp
• Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.h
• Source/JavaScriptCore/ftl/FTLJITCode.cpp
• Source/JavaScriptCore/ftl/FTLJITCode.h
• Source/JavaScriptCore/ftl/FTLJITFinalizer.cpp
• Source/JavaScriptCore/ftl/FTLLazySlowPath.cpp
• Source/JavaScriptCore/ftl/FTLLocation.cpp
• Source/JavaScriptCore/ftl/FTLLocation.h
• Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
• Source/JavaScriptCore/ftl/FTLOSREntry.cpp
• Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
• Source/JavaScriptCore/ftl/FTLOperations.cpp
• Source/JavaScriptCore/ftl/FTLSaveRestore.cpp
• Source/JavaScriptCore/ftl/FTLSaveRestore.h
• Source/JavaScriptCore/ftl/FTLState.cpp
• Source/JavaScriptCore/ftl/FTLThunks.cpp
• Source/JavaScriptCore/heap/AbstractSlotVisitorInlines.h
• Source/JavaScriptCore/heap/AllocatingScope.h
• Source/JavaScriptCore/heap/Allocator.h
• Source/JavaScriptCore/heap/BlockDirectory.cpp
• Source/JavaScriptCore/heap/BlockDirectory.h
• Source/JavaScriptCore/heap/BunV8HeapSnapshotBuilder.cpp
• Source/JavaScriptCore/heap/CellContainerInlines.h
• Source/JavaScriptCore/heap/CollectingScope.h
• Source/JavaScriptCore/heap/CompleteSubspace.cpp
• Source/JavaScriptCore/heap/CompleteSubspace.h
• Source/JavaScriptCore/heap/CompleteSubspaceInlines.h
• Source/JavaScriptCore/heap/GCActivityCallback.cpp
• Source/JavaScriptCore/heap/GCSafepointEpoch.cpp
• Source/JavaScriptCore/heap/GCSafepointEpoch.h
• Source/JavaScriptCore/heap/GCThreadLocalCache.cpp
• Source/JavaScriptCore/heap/GCThreadLocalCache.h
• Source/JavaScriptCore/heap/HandleSet.cpp
• Source/JavaScriptCore/heap/HandleSet.h
• Source/JavaScriptCore/heap/Heap.cpp
• Source/JavaScriptCore/heap/Heap.h
• Source/JavaScriptCore/heap/HeapCellInlines.h
• Source/JavaScriptCore/heap/HeapClientSet.cpp
• Source/JavaScriptCore/heap/HeapClientSet.h
• Source/JavaScriptCore/heap/HeapInlines.h
• Source/JavaScriptCore/heap/HeapIterationScope.h
• Source/JavaScriptCore/heap/HeapProfiler.h
• Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp
• Source/JavaScriptCore/heap/IncrementalSweeper.cpp
• Source/JavaScriptCore/heap/IsoCellSet.cpp
• Source/JavaScriptCore/heap/IsoSubspace.cpp
• Source/JavaScriptCore/heap/IsoSubspace.h
• Source/JavaScriptCore/heap/IsoSubspaceInlines.h
• Source/JavaScriptCore/heap/LocalAllocator.cpp
• Source/JavaScriptCore/heap/LocalAllocatorInlines.h
• Source/JavaScriptCore/heap/MachineStackMarker.cpp
• Source/JavaScriptCore/heap/MachineStackMarker.h
• Source/JavaScriptCore/heap/MarkedBlock.cpp
• Source/JavaScriptCore/heap/MarkedBlock.h
• Source/JavaScriptCore/heap/MarkedBlockInlines.h
• Source/JavaScriptCore/heap/MarkedSpace.cpp
• Source/JavaScriptCore/heap/MarkedSpace.h
• Source/JavaScriptCore/heap/PreciseAllocation.cpp
• Source/JavaScriptCore/heap/PreciseAllocation.h
• Source/JavaScriptCore/heap/PreciseSubspace.cpp
• Source/JavaScriptCore/heap/RunningScope.h
• Source/JavaScriptCore/heap/SharedHeapTestHarness.cpp
• Source/JavaScriptCore/heap/SharedHeapTestHarness.h
• Source/JavaScriptCore/heap/SlotVisitor.cpp
• Source/JavaScriptCore/heap/Strong.h
• Source/JavaScriptCore/heap/StrongInlines.h
• Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp
• Source/JavaScriptCore/heap/Subspace.cpp
• Source/JavaScriptCore/heap/SweepingScope.h
• Source/JavaScriptCore/heap/WeakBlock.cpp
• Source/JavaScriptCore/heap/WeakSet.cpp
• Source/JavaScriptCore/heap/WeakSet.h
• Source/JavaScriptCore/heap/WeakSetInlines.h
• Source/JavaScriptCore/interpreter/CLoopStack.cpp
• Source/JavaScriptCore/interpreter/CLoopStack.h
• Source/JavaScriptCore/interpreter/CLoopStackInlines.h
• Source/JavaScriptCore/interpreter/CallFrame.cpp
• Source/JavaScriptCore/interpreter/FrameTracers.h
• Source/JavaScriptCore/interpreter/Interpreter.cpp
• Source/JavaScriptCore/interpreter/InterpreterInlines.h
• Source/JavaScriptCore/interpreter/StackVisitor.cpp
• Source/JavaScriptCore/jit/AssemblyHelpers.cpp
• Source/JavaScriptCore/jit/AssemblyHelpers.h
• Source/JavaScriptCore/jit/CCallHelpers.cpp
• Source/JavaScriptCore/jit/CCallHelpers.h
• Source/JavaScriptCore/jit/ConcurrentButterflyOperations.cpp
• Source/JavaScriptCore/jit/ConcurrentButterflyOperations.h

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enforce disjoint file scopes before starting the fix fan-out.

The workflow depends on disjoint scopes for safe parallel apply, but Line 129–Line 140 only validates path shape. Without explicit overlap checks, two items can race on the same file.

Suggested guard

   const KNOWN_GATES = ['build', 'corpus', 'stub', 'tsan', 'bench']
   const items = (triage.items ?? [])
@@
     .slice(0, 20)
+
+  const normalizeScope = (p) => String(p).replace(/^\.\/+/, '').replace(/\/+/g, '/')
+  const ownerByPath = new Map()
+  for (const item of items) {
+    for (const rawPath of item.scope ?? []) {
+      const path = normalizeScope(rawPath)
+      const previous = ownerByPath.get(path)
+      if (previous && previous !== item.id)
+        throw new Error(`non-disjoint fix scopes: "${path}" appears in both "${previous}" and "${item.id}"`)
+      ownerByPath.set(path, item.id)
+    }
+  }
+
   log(`Round ${round}: ${items.length} fix item(s): ${items.map(i => `${i.id}[${i.gate}]`).join(', ')}`)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/workflows/thread-fix.js around lines 128 - 140, The code currently
only validates each item's scope shape (via safeScopePath) but does not check
that scopes are disjoint across items; add a guard after computing items (the
const items = ... block / after map(...) and before slice(0,20) or immediately
after) that compares scope file paths across all items (use the normalized scope
arrays on each item) and fails fast if any two items share a path. Implement a
simple overlap detection that collects paths per item, finds intersections, and
when found throws an Error (or processLogger/error) naming the conflicting item
ids (use item.id) so the fix fan-out never runs on overlapping file scopes.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Same flag inconsistency in test execution.

Uses --useThreads=true but test files use --useJSThreads=1.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/workflows/thread-prep.js at line 454, The README/example line uses
the flag form "--useThreads=true" while the test files expect
"--useJSThreads=1"; update the invocation in .claude/workflows/thread-prep.js
(the string "Each test self-contained: ./WebKitBuild/Debug/bin/jsc
--useThreads=true <file>.") to use the same flag the tests use (replace
"--useThreads=true" with "--useJSThreads=1"), or alternatively update the tests
to accept "--useThreads" consistently—ensure the project uses one canonical flag
(prefer matching the test suite's "--useJSThreads=1").

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Potential crash when DAG is stuck with no resolvable tasks.

If all remaining tasks have unsatisfiable deps (circular dependency or missing task IDs), tasks.find(t => !done.has(t.id)) returns undefined, and ready.push(undefined) proceeds. The subsequent batch.push(t) on line 209 then iterates over undefined, and line 212's t.id dereferences it, causing a runtime error.

🐛 Proposed fix

  if (!ready.length) {
    log(`UNGIL: DAG stuck — ${tasks.length - done.size} task(s) blocked by unsatisfiable deps; running them sequentially`)
-   ready.push(tasks.find(t => !done.has(t.id)))
+   const fallback = tasks.find(t => !done.has(t.id))
+   if (!fallback)
+     throw new Error('UNGIL: DAG stuck but no undone tasks remain — logic error')
+   ready.push(fallback)
  }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (!ready.length) {
	log(`UNGIL: DAG stuck — ${tasks.length - done.size} task(s) blocked by unsatisfiable deps; running them sequentially`)
	ready.push(tasks.find(t => !done.has(t.id)))
	}
	if (!ready.length) {
	log(`UNGIL: DAG stuck — ${tasks.length - done.size} task(s) blocked by unsatisfiable deps; running them sequentially`)
	const fallback = tasks.find(t => !done.has(t.id))
	if (!fallback)
	throw new Error('UNGIL: DAG stuck but no undone tasks remain — logic error')
	ready.push(fallback)
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/workflows/thread-ungil.js around lines 199 - 202, The current
fallback pushes the result of tasks.find(...) without checking for undefined,
which can lead to a crash when all remaining tasks are unsatisfiable; update the
block around ready.push(tasks.find(...)) to capture the candidate (e.g., const
candidate = tasks.find(t => !done.has(t.id))) and only push it if candidate is
truthy, otherwise handle the stuck state explicitly (for example: log a clear
error about unsatisfiable/circular deps and abort/return/throw or mark remaining
tasks as failed) so subsequent code that expects a valid task (batch.push(t),
t.id, etc.) never receives undefined.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

The rope CAS phase currently can't tell success from failure.

Atomics.compareExchange() returns the old value, and every string in this block collapses to "leftright". That means the swaps checks pass whether the CAS actually succeeds or fails, so this phase doesn't validate the value-equality path it is meant to cover.

Suggested fix

 const ropeObj = { s: "left" + "right" };
 const ropeThreads = [];
 for (let t = 0; t < 2; ++t) {
     ropeThreads.push(new Thread(() => {
         let swaps = 0;
         for (let i = 0; i < 400; ++i) {
-            if (Atomics.compareExchange(ropeObj, "s", "left" + "right", "le" + "ftright") === "leftright")
+            if (Atomics.compareExchange(ropeObj, "s", "left" + "right", "left-" + "right") === "leftright")
                 ++swaps;
-            if (Atomics.compareExchange(ropeObj, "s", "leftri" + "ght", "left" + "right") === "leftright")
+            if (Atomics.compareExchange(ropeObj, "s", "left-" + "right", "le" + "ftright") === "left-right")
                 ++swaps;
         }
         return swaps >= 0;
     }));
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/atomics/property-cas-storm-u28-flat.js` around lines 69 - 83,
The CAS checks in the ropeThreads loop are unreliable because
Atomics.compareExchange(ropeObj, "s", ...) currently uses constant-folded string
literals so expected and replacement collapse to the same value and you never
detect success vs failure; fix by building distinct expected and replacement
strings at runtime and compare the returned old value to the expected variable
(not a hardcoded literal). Concretely, in the Thread body create let expected1 =
"left" + "right" and let replacement1 = "le" + String.fromCharCode(102) +
"tright" (or another runtime-built variant) then call
Atomics.compareExchange(ropeObj, "s", expected1, replacement1) and test returned
=== expected1 to increment swaps; do the analogous change for the second
compareExchange (use distinct expected2/replacement2 and compare returned ===
expected2) so swaps actually reflects CAS success for the compareExchange calls.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Test always prints PASS even when assertions are skipped.

When $vm.sharedHeapTest is unavailable (Line 20 condition false), the test skips all assertions but still prints PASS, which could mask failures in environments where the feature is not enabled.

Consider printing a skip message or failing explicitly when the required feature is unavailable.

🧪 Proposed fix to make skipping explicit

 if (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest === "function") {
     shouldBeTrue($vm.sharedHeapTest("blockedInNativeVsGC", 4, 2000), "blockedInNativeVsGC");
     shouldBeTrue($vm.sharedHeapTest("syncRequesterStorm", 4, 8), "syncRequesterStorm");
     shouldBeTrue($vm.sharedHeapTest("noEnteredVMsGC", 3, 8), "noEnteredVMsGC");
+    print("PASS");
+} else {
+    print("SKIPPED: $vm.sharedHeapTest not available");
 }
-print("PASS");

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	print("PASS");
	if (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest === "function") {
	shouldBeTrue($vm.sharedHeapTest("blockedInNativeVsGC", 4, 2000), "blockedInNativeVsGC");
	shouldBeTrue($vm.sharedHeapTest("syncRequesterStorm", 4, 8), "syncRequesterStorm");
	shouldBeTrue($vm.sharedHeapTest("noEnteredVMsGC", 3, 8), "noEnteredVMsGC");
	print("PASS");
	} else {
	print("SKIPPED: $vm.sharedHeapTest not available");
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/heap-access-blocking.js` at line 25, The test unconditionally
prints print("PASS") even when $vm.sharedHeapTest is false; update the control
flow around the $vm.sharedHeapTest check so that when $vm.sharedHeapTest is
unavailable the script prints an explicit skip message (e.g., "SKIP:
sharedHeapTest unavailable") and returns/ends early instead of printing "PASS",
otherwise continue running assertions and only print "PASS" after the assertions
succeed; adjust the logic that currently calls print("PASS") to be reached only
in the successful, feature-present branch (referencing $vm.sharedHeapTest and
the print("PASS") call).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Protect the Object.prototype mutation with finally.

If any assertion in this block fails before Line 149, __sharedObjectsTestTemp stays on Object.prototype and contaminates the rest of the suite.

Suggested fix

 {
     new Thread(() => { Object.prototype.__sharedObjectsTestTemp = 123; }).join();
-    shouldBe({}.__sharedObjectsTestTemp, 123);
-    const cleaned = new Thread(() => delete Object.prototype.__sharedObjectsTestTemp).join();
-    shouldBeTrue(cleaned);
-    shouldBe({}.__sharedObjectsTestTemp, undefined);
+    try {
+        shouldBe({}.__sharedObjectsTestTemp, 123);
+    } finally {
+        const cleaned = new Thread(() => delete Object.prototype.__sharedObjectsTestTemp).join();
+        shouldBeTrue(cleaned);
+        shouldBe({}.__sharedObjectsTestTemp, undefined);
+    }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/shared-objects/prototype-chain.js` around lines 146 - 151,
The test mutates Object.prototype via Thread and currently only deletes that
property after assertions, which can leak if an assertion fails; wrap the
assertions that read {}.__sharedObjectsTestTemp in a try/finally and perform the
cleanup inside the finally block by invoking the deletion in a Thread (use new
Thread(() => delete Object.prototype.__sharedObjectsTestTemp).join()) and assert
the deletion (shouldBeTrue(cleaned)); reference the Thread construct,
Object.prototype.__sharedObjectsTestTemp, and the delete invocation to locate
and refactor the code.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing harness.js import for spawnN and joinAll.

This test uses spawnN (Line 22) and joinAll (Line 47), but only loads assert.js. The pattern in other threading tests suggests these functions are exported from harness.js. This would cause a ReferenceError at runtime.

🔧 Proposed fix

-load("../resources/assert.js", "caller relative");
+load("../harness.js", "caller relative");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/vmstate/structure-churn-threads.js` at line 16, The test is
missing the harness import that provides spawnN and joinAll; update the test to
also load the harness harness module (e.g. add a load("../resources/harness.js",
"caller relative") alongside the existing load("../resources/assert.js", "caller
relative")) so spawnN and joinAll are defined before they are used in the test.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard this Bun-only TLS helper with USE(BUN_JSC_ADDITIONS).

The comment ties mrs_TPIDR_EL0() to the JSThreads/Bun TLS path, but the declaration is exposed to every Linux ARM64 JSC build. This should stay behind the Bun feature guard instead of widening the generic assembler surface.

As per coding guidelines, Source/JavaScriptCore/**/*.{cpp,h} must "Guard Bun-specific features with USE(BUN_JSC_ADDITIONS) and event-loop integration with USE(BUN_EVENT_LOOP)".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/assembler/ARM64Assembler.h` around lines 2694 - 2703,
The mrs_TPIDR_EL0 TLS helper is Bun-specific but currently exposed under only
OS(LINUX); guard its declaration with the Bun feature macro by wrapping it with
USE(BUN_JSC_ADDITIONS) (either combine into `#if` OS(LINUX) &&
USE(BUN_JSC_ADDITIONS) or nest `#if` USE(BUN_JSC_ADDITIONS) around the existing
block) so mrs_TPIDR_EL0(RegisterID dst) is only compiled when Bun additions are
enabled; also update the comment to note the Bun/JSThreads tie-in if present.

Source: Coding guidelines

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Guard the JSThreads-only IC changes with USE(BUN_JSC_ADDITIONS).

These packed-word fields, static assertions, and repatching assertions are Bun/JSThreads-specific, but they currently change PropertyInlineCache for every JSC build. That leaks the feature into non-Bun configurations instead of preserving the preexisting layout and behavior there.

As per coding guidelines, Source/JavaScriptCore/**/*.{cpp,h} must "Guard Bun-specific features with USE(BUN_JSC_ADDITIONS) and event-loop integration with USE(BUN_EVENT_LOOP)".

Also applies to: 455-562, 769-783

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/PropertyInlineCache.h` around lines 389 - 412,
The new JSThreads/Bun-specific packed-word fields and accessors were added to
PropertyInlineCache unconditionally; wrap the Bun/JSThreads-specific
declarations and definitions with the feature guard USE(BUN_JSC_ADDITIONS) so
non-Bun builds preserve the original layout and behavior. Specifically, enclose
the declarations of setInlineAccessSelfState and clearInlineAccessSelfState, the
packedInlineAccessSelfWord helper, and any related packed-word fields,
static_asserts, and repatch assertions referenced elsewhere (also the other
ranges around lines 455-562 and 769-783) in `#if` USE(BUN_JSC_ADDITIONS) / `#endif`
so these symbols (PropertyInlineCache changes, packedInlineAccessSelfWord,
setInlineAccessSelfState, clearInlineAccessSelfState) are only present when the
Bun addition flag is enabled.

Source: Coding guidelines

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does MSVC cl.exesupport the-mcx16flag? Forclang-cl, should this be passed as /clang:-mcx16instead of-mcx16?

💡 Result:

The MSVC compiler cl.exe does not support the -mcx16 flag [1]. This flag is specific to Clang and GCC, which use it to enable the CMPXCHG16B instruction [1]. Because cl.exe does not recognize -mcx16, attempting to use it directly with cl.exe will result in an unrecognized option error (such as D4002) [2]. For clang-cl, which is a driver for Clang designed for MSVC compatibility [3][4], the -mcx16 flag can typically be passed directly to the clang-cl command line [5]. When you invoke clang-cl, it can interpret many Clang-specific flags (including -mcx16) alongside MSVC-style options [5]. Consequently, you do not strictly need to use the /clang: prefix to pass -mcx16 to clang-cl; it is designed to accept it directly [5]. However, the /clang: prefix is a valid mechanism in clang-cl to pass flags directly to the underlying Clang driver if needed [6]. In practice, users frequently set -mcx16 in the CL environment variable when building with clang-cl, as clang-cl interrogates this variable for additional options [5]. Using pragma attributes in source code is another common approach to enable this feature without altering command-line flags [7].

Citations:

• 1: https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20130930/189911.html
• 2: https://www.geoffchappell.com/studies/msvc/cl/cl/options/
• 3: https://releases.llvm.org/16.0.0/tools/clang/docs/MSVCCompatibility.html
• 4: https://releases.llvm.org/16.0.0/tools/clang/docs/UsersManual.html
• 5: Missing support for clang builds under Windows MSVC SDL-Hercules-390/hyperion#629
• 6: https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html
• 7: Always use cmpxchg16b on x64 for atomic_ref<16 bytes> microsoft/STL#4751

---

Guard -mcx16 by compiler family to prevent MSVC build failures.

add_compile_options(-mcx16) is applied globally for x86_64, but this flag is not supported by MSVC cl.exe and will cause unrecognized option errors. Restrict this flag to GCC/Clang (and ensure proper handling for clang-cl), and consider applying it at target scope rather than globally.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/CMakeLists.txt` around lines 292 - 299, The global
add_compile_options(-mcx16) under the WTF_CPU_X86_64 branch causes MSVC
failures; change this to only add -mcx16 for GCC/Clang toolchains (handle
clang-cl as Clang) and avoid adding it when CMAKE_CXX_COMPILER_ID is MSVC, and
prefer applying it with target_compile_options on the JSC target instead of
globally; locate the add_compile_options(-mcx16) occurrence in CMakeLists.txt
(WTF_CPU_X86_64 section) and replace the global call with a compiler-ID guarded
conditional that adds the flag only for GNU/Clang compilers (or via
target_compile_options(JSC PRIVATE ...)) so MSVC builds are not passed -mcx16.

[coderabbitai]

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

JSTests/threads/heap-allocation-storm.js (1)

21-35: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fail fast when $vm.sharedHeapTest is unavailable.

Line 21 currently allows the test to no-op and still print PASS. That can hide regressions in the shared-heap test hook. Make this path fail explicitly (or skip explicitly with a harness-level skip), instead of silently succeeding.

Suggested fix

-if (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest === "function") {
+if (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest === "function") {
     shouldBeTrue($vm.sharedHeapTest("allocationStorm", 4, 20000), "allocationStorm");
     shouldBeTrue($vm.sharedHeapTest("stealRace", 4, 16), "stealRace");
@@
     shouldBe(sum, 149985000);
+} else {
+    throw new Error("sharedHeapTest is unavailable with --useDollarVM=1");
 }
 print("PASS");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/heap-allocation-storm.js` around lines 21 - 35, The test
currently silently succeeds when the $vm.sharedHeapTest hook is missing; update
the top-level conditional to fail fast by checking $vm and $vm.sharedHeapTest
and throwing or calling the test harness failure when absent: replace the
existing if (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest ===
"function") { ... } with an explicit guard that if (typeof $vm === "undefined"
|| typeof $vm.sharedHeapTest !== "function") throw new Error("sharedHeapTest
hook unavailable"); otherwise invoke $vm.sharedHeapTest("allocationStorm", 4,
20000) and $vm.sharedHeapTest("stealRace", 4, 16) as before (references: $vm,
sharedHeapTest, shouldBeTrue).

JSTests/threads/heap-iss-revert.js (1)

16-29: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid reporting PASS when the test body is skipped.

Line 28 prints PASS even when Line 16’s guard is false, so this can silently pass without running assertions.

Suggested fix

 if (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest === "function") {
     shouldBeTrue($vm.sharedHeapTest("issRevertChurn", 2, 8), "issRevertChurn");
@@
     shouldBe(sum, 12497500);
+    print("PASS");
+} else {
+    print("SKIP: $vm.sharedHeapTest not available");
 }
-print("PASS");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/heap-iss-revert.js` around lines 16 - 29, The test currently
prints "PASS" unconditionally even when the guard (typeof $vm !== "undefined" &&
typeof $vm.sharedHeapTest === "function") prevents the assertions from running;
update the logic so that print("PASS") is only executed when the test body
actually ran and assertions were evaluated: wrap the print("PASS") inside the
same guard block (after the shouldBeTrue/shouldBe calls) or, alternatively, add
an else branch that prints "SKIP" or throws when the guard is false; reference
the existing symbols $vm, sharedHeapTest, shouldBeTrue, shouldBe and the current
print("PASS") to locate where to change.

♻️ Duplicate comments (6)

JSTests/threads/heap-stop-interleavings.js (1)

24-29: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Same skipped-assertions false-positive path remains.

Line 29 still prints PASS even if Line 24 guard is false, so the file can pass without executing any checks.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/heap-stop-interleavings.js` around lines 24 - 29, The file
prints "PASS" unconditionally even when the guard if (typeof $vm !== "undefined"
&& typeof $vm.sharedHeapTest === "function") is false, allowing a false-positive
pass; fix by ensuring PASS is only printed when tests actually ran — either move
print("PASS") inside that guard block after the shouldBeTrue(...) calls or
introduce a boolean (e.g., ranTests) set to true inside the guard and only print
"PASS" when ranTests is true (and consider printing a skip/fail message
otherwise); update references to the guard and the shouldBeTrue(...) invocations
accordingly.

JSTests/threads/vmstate/structure-churn-threads.js (1)

16-16: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Load the threads harness before using spawnN/joinAll.

spawnN at Line 22 and joinAll at Line 47 are used without importing ../harness.js, so this can fail with ReferenceError.

🔧 Proposed fix

-load("../resources/assert.js", "caller relative");
+load("../harness.js", "caller relative");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/vmstate/structure-churn-threads.js` at line 16, The test uses
spawnN and joinAll but never loads the threads harness; add a load call to
import the harness (e.g., load('harness.js', 'caller relative')) before any use
of spawnN and joinAll so those helpers are defined (place it above the existing
load("../resources/assert.js", "caller relative") or immediately after it);
ensure you reference the harness filename so spawnN and joinAll resolve at
runtime.

Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h (1)

7424-7443: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard this ELF TLS helper with USE(BUN_JSC_ADDITIONS).

Line 7424 still exposes a Bun-specific TLS path to every Linux JSC build. This helper is wired to butterflyTIDTagELFTLSOffset(), so it should stay behind the Bun additions feature flag.

Suggested patch

-#if OS(LINUX)
+#if OS(LINUX) && USE(BUN_JSC_ADDITIONS)
     // ELF initial-exec TLS load: one %fs-prefixed 64-bit load at a constant
     // (typically negative) offset from the thread pointer, baked as an
     // immediate at emission. The offset comes from
     // JSC::butterflyTIDTagELFTLSOffset() (jit/ConcurrentButterflyOperations.h),
@@
     static bool loadFromELFTLS64NeedsMacroScratchRegister()
     {
         return false;
     }
 `#endif`

As per coding guidelines, Source/JavaScriptCore/**/*.{cpp,h} must “Guard Bun-specific features with USE(BUN_JSC_ADDITIONS) and event-loop integration with USE(BUN_EVENT_LOOP)”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h` around lines 7424 -
7443, The ELF TLS helper functions loadFromELFTLS64 and
loadFromELFTLS64NeedsMacroScratchRegister are Bun-specific and must be enclosed
in the USE(BUN_JSC_ADDITIONS) guard; update the preprocessor around the current
OS(LINUX) block so that the declarations/definitions of loadFromELFTLS64 and
loadFromELFTLS64NeedsMacroScratchRegister (which are tied to
butterflyTIDTagELFTLSOffset()) are compiled only when USE(BUN_JSC_ADDITIONS) is
enabled, keeping the existing RELEASE_ASSERT and fs/movq_mr logic intact.

Source: Coding guidelines

Source/JavaScriptCore/assembler/MacroAssemblerARM64.h (1)

6441-6461: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add Bun feature gating to the new Linux ELF TLS helper block.

Line 6441 currently gates this Bun-specific helper with OS(LINUX) only; it also needs USE(BUN_JSC_ADDITIONS) to avoid exposing Bun-only paths in non-Bun Linux builds.

Suggested patch

-#if OS(LINUX)
+#if OS(LINUX) && USE(BUN_JSC_ADDITIONS)
@@
-#endif // OS(LINUX)
+#endif // OS(LINUX) && USE(BUN_JSC_ADDITIONS)

As per coding guidelines, “Source/JavaScriptCore/**/*.{cpp,h}: Guard Bun-specific features with USE(BUN_JSC_ADDITIONS) and event-loop integration with USE(BUN_EVENT_LOOP)”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/assembler/MacroAssemblerARM64.h` around lines 6441 -
6461, The new ELF TLS helper (functions loadFromELFTLS64 and
loadFromELFTLS64NeedsMacroScratchRegister, and the block using
m_assembler.mrs_TPIDR_EL0 and load64) is currently only gated by OS(LINUX);
guard this Bun-specific addition with USE(BUN_JSC_ADDITIONS) as well (e.g.,
change the preprocessor condition to require both OS(LINUX) and
USE(BUN_JSC_ADDITIONS)) so the helper is only exposed in Bun-enabled builds per
the project guidelines.

Source: Coding guidelines

Source/JavaScriptCore/bytecode/CodeBlock.cpp (1)

986-1007: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

dfgJITData ownership is still dropped without a retirement path under JSThreads.

Line 987 clears m_jitData, and Lines 993-1006 skip deletion when Options::useJSThreads() is on, but the pointer is never transferred to a retire container. This leaves optimizing DFGJITData permanently unreachable and leaked.

Also applies to: 1066-1079

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/CodeBlock.cpp` around lines 986 - 1007, The
code clears m_jitData and skips deleting jitData when Options::useJSThreads() is
true, but never transfers ownership to any retirement container, leaking the
DFGJITData; instead of simply nulling m_jitData and skipping delete,
move/transfer jitData into the thread-safe retire queue/holder used for delayed
teardown (e.g. push or hand off jitData to the existing retire/retainer
mechanism used for JIT data) after calling jitData->clearWatchpoints(), so that
the object remains reachable for eventual retirement; apply the same
ownership-transfer fix to the analogous block referenced at lines 1066-1079 (the
other dfgJITData cleanup site).

Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp (1)

3961-3964: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handler-IC SetPrivateBrand is still reachable in thread mode.

This only blocks the repatching stub path. compileOneAccessCaseHandler() still routes AccessCase::SetPrivateBrand to CommonJITThunkID::SetPrivateBrandHandler, and setPrivateBrandHandler() still writes the new structure ID unconditionally, so the unsafe structure-only transition is still available under --useJSThreads.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp` around lines 3961 -
3964, The SetPrivateBrand handler remains reachable under thread mode: update
compileOneAccessCaseHandler so AccessCase::SetPrivateBrand is not routed to
CommonJITThunkID::SetPrivateBrandHandler when Options::useJSThreads() is true,
and modify setPrivateBrandHandler to guard the unconditional structure-only
write with the same check (or make it a no-op/fall through to a thread-safe
transition) so structure-only transitions are never performed while useJSThreads
is enabled; locate references by name: compileOneAccessCaseHandler,
AccessCase::SetPrivateBrand, CommonJITThunkID::SetPrivateBrandHandler, and
setPrivateBrandHandler to apply the guards.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/workflows/thread-prep.js:
- Line 425: The comment uses flag names that don't match the tests: replace the
stub-phase reference to "--useThreads=true" and the parenthetical
"--useThreadGIL=true" with the test-used flag "--useJSThreads=1" (and if the
codebase expects a separate GIL flag, rename that occurrence to the test-aligned
variant or remove the misleading default), and ensure the USE_BUN_JSC_ADDITIONS
gating text still appears; update any string/flag checks or docs in the same
stub block that reference "--useThreads" or "--useThreadGIL" to use
"--useJSThreads" so names are consistent with the test corpus.

In @.claude/workflows/thread-scanners.js:
- Line 3: The SCANNERS configuration advertises ASAN but never defines or runs
an ASAN scanner; update the SCANNERS array in thread-scanners.js to include a
dedicated ASAN scanner entry (and any missing entries around the other scanner
entries at the block referenced by lines ~28-38) that: names the job "asan",
sets the runner/steps to build the threads target with -fsanitize=address (and
related flags as needed), runs the instrumented binary under the test harness,
and publishes the sanitizer logs/exit status; ensure the scanner entry follows
the same job schema as the existing TSAN/UBSAN entries and is referenced
wherever SCANNERS is iterated so ASAN actually executes.
- Around line 50-52: The current pipeline filters reports then slices to 30
before mapping, causing slice(0, 30) to drop items by input order rather than
risk; change the flow in the block that builds items (where reports, ident, and
items are used) to sort/prioritize the filtered reports by severity/risk (e.g.,
map severities to priority values and sort descending by that value, then by
other tie-breakers like date or confidence) before calling slice(0, 30), then
map to add id: ident(f.id) and scanner: ident(f.scanner) and log the resulting
items.length; ensure the sorting happens on the filtered array
(reports.filter(...)) and preserve existing ident usage.

In `@JSTests/threads/arrays/holes.js`:
- Line 4: The test fails because it uses harness utilities (Lock, joinAll,
spawnN) but only loads assert.js; add a load for harness.js at the top of the
file alongside the existing load("../resources/assert.js", "caller relative") so
the Lock, joinAll, and spawnN symbols are defined before use.

In `@JSTests/threads/heap-deferral-storm.js`:
- Around line 20-24: The test prints "PASS" unconditionally even when the
assertion guard (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest ===
"function") prevents the $vm.sharedHeapTest calls from running; move the
print("PASS") into that same guard or add a boolean flag set after running
$vm.sharedHeapTest calls (reference the guard expression and the function
$vm.sharedHeapTest("deferralVsAllocationStorm", ...) and
$vm.sharedHeapTest("structureLockVsSTW", ...)) and only print PASS when the
assertions actually ran (i.e., guard true or flag true).

In `@JSTests/threads/invariants/no-time-travel.js`:
- Around line 24-47: The test spawns writer, transitioner and reader Threads
without synchronizing their start, allowing the reader to finish before writers
run; add an explicit start barrier (e.g., a shared boolean/flag or a Barrier
object) that each Thread (the writer Thread, transitioner Thread, and reader
Thread) waits on before entering their loops, and only flip/signal the barrier
once all three Threads are created so they begin competing simultaneously;
update the Thread callbacks (the closures that set o.v, grow..., and sample o.v)
to spin/wait on that shared start flag or await the Barrier before proceeding.

In `@JSTests/threads/jit/bench-gates.sh`:
- Around line 142-148: The GEOMEAN_10 pipeline can emit RATIO lines to stderr
before detecting a NAME-MISMATCH; modify the awk invocation used with paste
<(echo "$OFF_RESULTS") <(echo "$ON10_RESULTS") so it validates all input row
name pairs before printing any per-benchmark RATIO or writing to stderr.
Concretely, in the awk block used to compute GEOMEAN_10, first accumulate rows
(e.g., store per-row ratios in arrays and maintain logsum/n) and only output the
per-benchmark "RATIO ..." lines and the final geomean after the END check
confirms there was no NAME-MISMATCH; ensure if a mismatch is detected you exit
non-zero without emitting prior RATIO lines, preserving existing stderr handling
that records FLAGON-1-0 lines.

In `@JSTests/threads/shared-objects/getters-setters.js`:
- Line 6: The test is missing the harness utilities used later; add an import
for harness so Lock, joinAll and spawnN are available. Insert a load call for
the harness (e.g., load("../resources/harness.js", "caller relative")) alongside
the existing load("../resources/assert.js", "caller relative") near the top of
getters-setters.js so that the Lock class and the joinAll/spawnN helpers resolve
at runtime.

In `@JSTests/threads/sync/condition-notify-all-shared-lock.js`:
- Line 18: The test is missing the harness import needed for spawnN and joinAll;
add a load of "harness.js" (in addition to the existing load of "assert.js")
near the top of the file so that spawnN and joinAll are defined before they are
used in the test (ensure spawnN and joinAll are available when referenced).

In `@Source/JavaScriptCore/bytecode/CodeBlock.cpp`:
- Around line 620-633: The new JSThreads/Bun-specific runtime check and call
(Options::useJSThreads() and metadata.m_structureID.set(vm, this, op.structure))
must be guarded by the repository feature macros; wrap the JSThreads-specific
branch with `#if` USE(BUN_JSC_ADDITIONS) (and also add `#if` USE(BUN_EVENT_LOOP)
where event-loop integration is required) so the code only compiles when Bun
additions are enabled, and mirror the same guards around the other affected
blocks you noted (around the code at the other locations referencing
useJSThreads()/Bun paths) to satisfy the gating policy.

In `@Source/JavaScriptCore/bytecode/ExecutionCounter.cpp`:
- Around line 71-77: deferIndefinitely() currently writes m_totalCount and
m_activeThreshold as plain non-synchronized members, which can cause C++
data-race UB in concurrent paths; change the member types in ExecutionCounter
(m_totalCount and m_activeThreshold) to std::atomic<int32_t> (or equivalent) or
ensure all accesses use the same lock, then update all reads/writes (including
other SUPPRESS_TSAN methods in ExecutionCounter.cpp and any callers) to use the
chosen synchronization with appropriate memory_order (e.g., relaxed for advisory
counts or stronger where needed), leaving storeCounterValueConcurrently(...)
behavior unchanged but auditing its interaction with the new atomic/lock to
avoid races.

In `@Source/JavaScriptCore/bytecode/GetByIdMetadata.h`:
- Around line 263-268: The ASSERT in GetByIdModeMetadata::setProtoLoadMode
(which checks Options::useJSThreads()) compiles out in release and can allow an
unsafe 16-byte write when JSThreads are enabled; replace the compile-only ASSERT
with a runtime guard that aborts or otherwise prevents execution when
Options::useJSThreads() is true (e.g., a RELEASE_ASSERT or an explicit
crash/return with clear logging), so the unreachable assumption around
setupGetByIdPrototypeCache remains enforced at runtime and the unsafe proto
write cannot occur.

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp`:
- Around line 6398-6400: The own-property path is losing the structure-check
dependency because emitDataICCheckUid() reuses scratch1GPR and then
loadHandlerImpl<ownProperty> is called with InvalidGPRReg; update the call so
the threaded ARM64 structureID register is forwarded instead of InvalidGPRReg.
Specifically, after emitDataICCheckUid() finishes, ensure you pass the
structureIDGPR (or the preserved register that holds the structure UID) into
loadHandlerImpl<ownProperty> in place of InvalidGPRReg so the structure-check
dependency remains alive for string/symbol GetByVal self loads (preserve or move
scratch1GPR into structureIDGPR if needed before the call).
- Around line 5611-5618: The getterHandlerImpl helper loses the validated
structure ID when offsetOfHolder() == 0 because scratch1GPR is overwritten
before calling loadProperty, causing loadProperty to be invoked without passing
the structureIDGPR and permitting a stale GetterSetter cell on ARM64
--useJSThreads; preserve the compared structure ID and pass it into loadProperty
by retaining the original structure ID register (don’t clobber scratch1GPR or
move the payload into it) and call the overload of loadProperty that accepts a
structureIDGPR (use the same register used in the structure check), updating the
code paths around loadPtr/moveConditionally64 to keep the structure ID live for
the subsequent loadProperty call.
- Around line 5854-5862: The current JSThreads guard uses jit.breakpoint() which
aborts instead of falling back; replace the hard trap with the same generic
slow-path/handler chaining used by the delete/replace transition handlers so
missed selection routes route to the fallback. Locate the block that checks
Options::useJSThreads() and remove the jit.breakpoint() call, and instead call
or jump to the existing generic transition fallback used by the delete/replace
handlers (mirror their chaining/emit-slow-path logic so the shared thunk
delegates to the slow path rather than trapping).

In `@Source/JavaScriptCore/bytecode/Repatch.cpp`:
- Around line 1147-1154: Wrap the direct runtime check Options::useJSThreads()
in a Bun feature guard so non-Bun builds don't reference Bun-only features:
surround the existing if (Options::useJSThreads()) [[unlikely]] return
GiveUpOnCache; (and the analogous checks at the other two sites for delete IC
creation and set-brand IC creation) with `#if` USE(BUN_JSC_ADDITIONS) ... `#endif`
so the JSThreads-specific cache bypass is compiled only when
USE(BUN_JSC_ADDITIONS) is enabled; keep the original behavior inside the guard
unchanged.

---

Outside diff comments:
In `@JSTests/threads/heap-allocation-storm.js`:
- Around line 21-35: The test currently silently succeeds when the
$vm.sharedHeapTest hook is missing; update the top-level conditional to fail
fast by checking $vm and $vm.sharedHeapTest and throwing or calling the test
harness failure when absent: replace the existing if (typeof $vm !== "undefined"
&& typeof $vm.sharedHeapTest === "function") { ... } with an explicit guard that
if (typeof $vm === "undefined" || typeof $vm.sharedHeapTest !== "function")
throw new Error("sharedHeapTest hook unavailable"); otherwise invoke
$vm.sharedHeapTest("allocationStorm", 4, 20000) and
$vm.sharedHeapTest("stealRace", 4, 16) as before (references: $vm,
sharedHeapTest, shouldBeTrue).

In `@JSTests/threads/heap-iss-revert.js`:
- Around line 16-29: The test currently prints "PASS" unconditionally even when
the guard (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest ===
"function") prevents the assertions from running; update the logic so that
print("PASS") is only executed when the test body actually ran and assertions
were evaluated: wrap the print("PASS") inside the same guard block (after the
shouldBeTrue/shouldBe calls) or, alternatively, add an else branch that prints
"SKIP" or throws when the guard is false; reference the existing symbols $vm,
sharedHeapTest, shouldBeTrue, shouldBe and the current print("PASS") to locate
where to change.

---

Duplicate comments:
In `@JSTests/threads/heap-stop-interleavings.js`:
- Around line 24-29: The file prints "PASS" unconditionally even when the guard
if (typeof $vm !== "undefined" && typeof $vm.sharedHeapTest === "function") is
false, allowing a false-positive pass; fix by ensuring PASS is only printed when
tests actually ran — either move print("PASS") inside that guard block after the
shouldBeTrue(...) calls or introduce a boolean (e.g., ranTests) set to true
inside the guard and only print "PASS" when ranTests is true (and consider
printing a skip/fail message otherwise); update references to the guard and the
shouldBeTrue(...) invocations accordingly.

In `@JSTests/threads/vmstate/structure-churn-threads.js`:
- Line 16: The test uses spawnN and joinAll but never loads the threads harness;
add a load call to import the harness (e.g., load('harness.js', 'caller
relative')) before any use of spawnN and joinAll so those helpers are defined
(place it above the existing load("../resources/assert.js", "caller relative")
or immediately after it); ensure you reference the harness filename so spawnN
and joinAll resolve at runtime.

In `@Source/JavaScriptCore/assembler/MacroAssemblerARM64.h`:
- Around line 6441-6461: The new ELF TLS helper (functions loadFromELFTLS64 and
loadFromELFTLS64NeedsMacroScratchRegister, and the block using
m_assembler.mrs_TPIDR_EL0 and load64) is currently only gated by OS(LINUX);
guard this Bun-specific addition with USE(BUN_JSC_ADDITIONS) as well (e.g.,
change the preprocessor condition to require both OS(LINUX) and
USE(BUN_JSC_ADDITIONS)) so the helper is only exposed in Bun-enabled builds per
the project guidelines.

In `@Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h`:
- Around line 7424-7443: The ELF TLS helper functions loadFromELFTLS64 and
loadFromELFTLS64NeedsMacroScratchRegister are Bun-specific and must be enclosed
in the USE(BUN_JSC_ADDITIONS) guard; update the preprocessor around the current
OS(LINUX) block so that the declarations/definitions of loadFromELFTLS64 and
loadFromELFTLS64NeedsMacroScratchRegister (which are tied to
butterflyTIDTagELFTLSOffset()) are compiled only when USE(BUN_JSC_ADDITIONS) is
enabled, keeping the existing RELEASE_ASSERT and fs/movq_mr logic intact.

In `@Source/JavaScriptCore/bytecode/CodeBlock.cpp`:
- Around line 986-1007: The code clears m_jitData and skips deleting jitData
when Options::useJSThreads() is true, but never transfers ownership to any
retirement container, leaking the DFGJITData; instead of simply nulling
m_jitData and skipping delete, move/transfer jitData into the thread-safe retire
queue/holder used for delayed teardown (e.g. push or hand off jitData to the
existing retire/retainer mechanism used for JIT data) after calling
jitData->clearWatchpoints(), so that the object remains reachable for eventual
retirement; apply the same ownership-transfer fix to the analogous block
referenced at lines 1066-1079 (the other dfgJITData cleanup site).

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp`:
- Around line 3961-3964: The SetPrivateBrand handler remains reachable under
thread mode: update compileOneAccessCaseHandler so AccessCase::SetPrivateBrand
is not routed to CommonJITThunkID::SetPrivateBrandHandler when
Options::useJSThreads() is true, and modify setPrivateBrandHandler to guard the
unconditional structure-only write with the same check (or make it a no-op/fall
through to a thread-safe transition) so structure-only transitions are never
performed while useJSThreads is enabled; locate references by name:
compileOneAccessCaseHandler, AccessCase::SetPrivateBrand,
CommonJITThunkID::SetPrivateBrandHandler, and setPrivateBrandHandler to apply
the guards.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ab720065-eaa2-4f3e-95db-aa18e4e5ee20

📥 Commits

Reviewing files that changed from the base of the PR and between 5851d47 and 5d1745d.

📒 Files selected for processing (300)

• .claude/workflows/thread-cve-audit.js
• .claude/workflows/thread-fix.js
• .claude/workflows/thread-fuzz.js
• .claude/workflows/thread-implement.js
• .claude/workflows/thread-prep.js
• .claude/workflows/thread-scanners.js
• .claude/workflows/thread-ungil-spec.js
• .claude/workflows/thread-ungil.js
• JSTests/threads.yaml
• JSTests/threads/api/blocking-gate.js
• JSTests/threads/api/condition-async-wait.js
• JSTests/threads/api/condition-basic.js
• JSTests/threads/api/condition-wait-termination.js
• JSTests/threads/api/lock-async-hold.js
• JSTests/threads/api/lock-basic.js
• JSTests/threads/api/lock-hold-termination.js
• JSTests/threads/api/park-no-microtask-drain.js
• JSTests/threads/api/thread-basic.js
• JSTests/threads/api/thread-ctor-errors.js
• JSTests/threads/api/thread-exc.js
• JSTests/threads/api/thread-id-bounds.js
• JSTests/threads/api/thread-lifecycle.js
• JSTests/threads/api/thread-restrict.js
• JSTests/threads/api/threadlocal-basic.js
• JSTests/threads/api/wasm-refused-sd7.js
• JSTests/threads/arrays/copy-on-write.js
• JSTests/threads/arrays/holes.js
• JSTests/threads/arrays/push-resize-multithread.js
• JSTests/threads/arrays/shared-element-read-write.js
• JSTests/threads/arrays/typed-arrays-sab.js
• JSTests/threads/atomics/property-cas-delete-undefined-sentinel-u5.js
• JSTests/threads/atomics/property-cas-dictionary-delete-u5.js
• JSTests/threads/atomics/property-cas-samevaluezero.js
• JSTests/threads/atomics/property-cas-storm-u28-flat.js
• JSTests/threads/atomics/property-cas-storm-u5-as.js
• JSTests/threads/atomics/property-errors.js
• JSTests/threads/atomics/property-load-store.js
• JSTests/threads/atomics/property-rmw.js
• JSTests/threads/atomics/property-store-missing-define-race.js
• JSTests/threads/atomics/property-wait-notify.js
• JSTests/threads/atomics/property-wait-termination.js
• JSTests/threads/atomics/property-waitasync-timeout.js
• JSTests/threads/atomics/property-wtr-isolation.js
• JSTests/threads/atomics/ta-path-unchanged.js
• JSTests/threads/atomics/ta-wait-thread-gate.js
• JSTests/threads/bench/array-element-read.js
• JSTests/threads/bench/array-element-write.js
• JSTests/threads/bench/flat-butterfly-read.js
• JSTests/threads/bench/flat-butterfly-write.js
• JSTests/threads/bench/harness.js
• JSTests/threads/bench/inline-property-read.js
• JSTests/threads/bench/inline-property-write.js
• JSTests/threads/bench/megamorphic-access.js
• JSTests/threads/bench/transition-heavy-constructor.js
• JSTests/threads/harness.js
• JSTests/threads/heap-access-blocking.js
• JSTests/threads/heap-allocation-storm.js
• JSTests/threads/heap-bench-allocation.js
• JSTests/threads/heap-client-churn.js
• JSTests/threads/heap-deferral-storm.js
• JSTests/threads/heap-epoch-reclaim.js
• JSTests/threads/heap-iss-revert.js
• JSTests/threads/heap-option-off.js
• JSTests/threads/heap-precise-storm.js
• JSTests/threads/heap-stop-interleavings.js
• JSTests/threads/invariants/delete-quarantine-dictionary.js
• JSTests/threads/invariants/delete-quarantine.js
• JSTests/threads/invariants/no-lost-elements.js
• JSTests/threads/invariants/no-lost-properties-same-name.js
• JSTests/threads/invariants/no-lost-properties.js
• JSTests/threads/invariants/no-time-travel.js
• JSTests/threads/invariants/no-torn-shapes.js
• JSTests/threads/jit/README.md
• JSTests/threads/jit/bench-gates.sh
• JSTests/threads/jit/construction-shared-constructor.js
• JSTests/threads/jit/fires-per-sec.js
• JSTests/threads/jit/ftl-osr-entry-catch-loop-amplifier.js
• JSTests/threads/jit/golden-disasm-corpus.js
• JSTests/threads/jit/golden-disasm.sh
• JSTests/threads/jit/ic-publish-reset-loops.js
• JSTests/threads/jit/int-gate-direct-call-relink.js
• JSTests/threads/jit/int-gate-epoch-reclaim.js
• JSTests/threads/jit/int-gate-fire-vs-execute.js
• JSTests/threads/jit/int-gate-jettison-vs-execute.js
• JSTests/threads/jit/int-gate-stop-budget.js
• JSTests/threads/jit/lint.sh
• JSTests/threads/jit/run-jit-tests.sh
• JSTests/threads/jit/shared-arraystorage-stress.js
• JSTests/threads/jit/spawned-thread-butterfly-stress.js
• JSTests/threads/jit/tag-discipline.js
• JSTests/threads/jit/tid-tag-3-threads.js
• JSTests/threads/lifecycle/async-join.js
• JSTests/threads/lifecycle/create-basics.js
• JSTests/threads/lifecycle/current-and-id.js
• JSTests/threads/lifecycle/exceptions-cross-join.js
• JSTests/threads/lifecycle/join-semantics.js
• JSTests/threads/lifecycle/nested-threads.js
• JSTests/threads/lifecycle/restrict-foreign-access.js.skip
• JSTests/threads/lifecycle/restrict.js
• JSTests/threads/lifecycle/return-values.js
• JSTests/threads/objectmodel/i03-array-resize-cas.js
• JSTests/threads/objectmodel/i03-as-shift-unshift.js
• JSTests/threads/objectmodel/i03-as-sparse-holes.js
• JSTests/threads/objectmodel/i03-b2-stay-flat-growth-vs-sw-flip.js
• JSTests/threads/objectmodel/i03-convert-grow-gc-read.js
• JSTests/threads/objectmodel/i03-cow-materialize-race.js
• JSTests/threads/objectmodel/i03-i37-same-shape-add-storm.js
• JSTests/threads/objectmodel/i03-n2-inline-add-races.js
• JSTests/threads/objectmodel/i03-n3-first-install-races.js
• JSTests/threads/objectmodel/i03-pa-global-races.js
• JSTests/threads/objectmodel/i03-quarantine-readd-across-gc.js
• JSTests/threads/objectmodel/i03-restart-locked-vs-conversion.js
• JSTests/threads/objectmodel/i03-selftest.js
• JSTests/threads/objectmodel/i03-shared-double.js
• JSTests/threads/objectmodel/i03-single-threaded-flag-on.js
• JSTests/threads/objectmodel/i03-single-threaded-no-change.js
• JSTests/threads/objectmodel/i03-stale-spine-reader-vs-grow.js
• JSTests/threads/objectmodel/i03-stress-force-segmented.js
• JSTests/threads/objectmodel/i03-stress-force-sw.js
• JSTests/threads/objectmodel/i03-t1-vs-sw-flip.js
• JSTests/threads/objectmodel/i03-t5-racing-growers.js
• JSTests/threads/objectmodel/i03-visit-range-outofline.js
• JSTests/threads/races/counter-atomics.js
• JSTests/threads/races/counter-lock.js
• JSTests/threads/races/join-storm.js
• JSTests/threads/races/transition-vs-read.js
• JSTests/threads/races/transition-vs-write.js
• JSTests/threads/races/wait-notify-storm.js
• JSTests/threads/resources/assert.js
• JSTests/threads/shared-objects/dictionary-mode.js
• JSTests/threads/shared-objects/frozen-sealed.js
• JSTests/threads/shared-objects/getters-setters.js
• JSTests/threads/shared-objects/property-add.js
• JSTests/threads/shared-objects/property-delete.js
• JSTests/threads/shared-objects/property-read-write.js
• JSTests/threads/shared-objects/prototype-chain.js
• JSTests/threads/smoke.js
• JSTests/threads/sync/atomics-futex-lock.js
• JSTests/threads/sync/atomics-object-basic.js
• JSTests/threads/sync/condition-notify-all-multi-waiter.js
• JSTests/threads/sync/condition-notify-all-shared-lock.js
• JSTests/threads/sync/condition-notify-all.js
• JSTests/threads/sync/condition-wait-notify.js
• JSTests/threads/sync/condition-worker-waiter.js
• JSTests/threads/sync/lock-async-hold.js
• JSTests/threads/sync/lock-hold-basic.js
• JSTests/threads/sync/lock-hold-mutual-exclusion.js
• JSTests/threads/sync/thread-local-isolation.js
• JSTests/threads/vmstate/README.md
• JSTests/threads/vmstate/all-flags-identity.js
• JSTests/threads/vmstate/exception-state-per-thread.js
• JSTests/threads/vmstate/flags-off-baseline.js
• JSTests/threads/vmstate/microtask-ordering.js
• JSTests/threads/vmstate/regexp-churn-threads.js
• JSTests/threads/vmstate/resources/workload.js
• JSTests/threads/vmstate/stack-limits-per-thread.js
• JSTests/threads/vmstate/structure-churn-dictionary.js
• JSTests/threads/vmstate/structure-churn-threads.js
• JSTests/threads/vmstate/structure-lock-single-thread.js
• JSTests/threads/vmstate/vmlite-single-thread-identity.js
• Source/JavaScriptCore/CMakeLists.txt
• Source/JavaScriptCore/Sources.txt
• Source/JavaScriptCore/assembler/ARM64Assembler.h
• Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
• Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h
• Source/JavaScriptCore/assembler/X86Assembler.h
• Source/JavaScriptCore/bytecode/ArrayProfile.cpp
• Source/JavaScriptCore/bytecode/ArrayProfile.h
• Source/JavaScriptCore/bytecode/BytecodeList.rb
• Source/JavaScriptCore/bytecode/CallLinkInfo.cpp
• Source/JavaScriptCore/bytecode/CallLinkInfo.h
• Source/JavaScriptCore/bytecode/CodeBlock.cpp
• Source/JavaScriptCore/bytecode/CodeBlock.h
• Source/JavaScriptCore/bytecode/ExecutionCounter.cpp
• Source/JavaScriptCore/bytecode/ExecutionCounter.h
• Source/JavaScriptCore/bytecode/GetByIdMetadata.h
• Source/JavaScriptCore/bytecode/GetByStatus.cpp
• Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp
• Source/JavaScriptCore/bytecode/InlineCacheCompiler.h
• Source/JavaScriptCore/bytecode/InlineCacheHandler.h
• Source/JavaScriptCore/bytecode/JSThreadsSafepoint.cpp
• Source/JavaScriptCore/bytecode/JSThreadsSafepoint.h
• Source/JavaScriptCore/bytecode/PropertyInlineCache.cpp
• Source/JavaScriptCore/bytecode/PropertyInlineCache.h
• Source/JavaScriptCore/bytecode/Repatch.cpp
• Source/JavaScriptCore/bytecode/RetiredJITArtifacts.cpp
• Source/JavaScriptCore/bytecode/RetiredJITArtifacts.h
• Source/JavaScriptCore/bytecode/SharedJITStubSet.cpp
• Source/JavaScriptCore/bytecode/SharedJITStubSet.h
• Source/JavaScriptCore/bytecode/ValueProfile.h
• Source/JavaScriptCore/bytecode/Watchpoint.cpp
• Source/JavaScriptCore/bytecode/Watchpoint.h
• Source/JavaScriptCore/debugger/Debugger.cpp
• Source/JavaScriptCore/debugger/Debugger.h
• Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
• Source/JavaScriptCore/dfg/DFGCallArrayAllocatorSlowPathGenerator.h
• Source/JavaScriptCore/dfg/DFGClobberize.h
• Source/JavaScriptCore/dfg/DFGCommonData.cpp
• Source/JavaScriptCore/dfg/DFGCommonData.h
• Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
• Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.cpp
• Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.h
• Source/JavaScriptCore/dfg/DFGJITCode.h
• Source/JavaScriptCore/dfg/DFGJumpReplacement.cpp
• Source/JavaScriptCore/dfg/DFGMayExit.cpp
• Source/JavaScriptCore/dfg/DFGOSREntry.cpp
• Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
• Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.h
• Source/JavaScriptCore/dfg/DFGOperations.cpp
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
• Source/JavaScriptCore/domjit/DOMJITEffect.h
• Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.cpp
• Source/JavaScriptCore/ftl/FTLForOSREntryJITCode.h
• Source/JavaScriptCore/ftl/FTLJITCode.cpp
• Source/JavaScriptCore/ftl/FTLJITCode.h
• Source/JavaScriptCore/ftl/FTLJITFinalizer.cpp
• Source/JavaScriptCore/ftl/FTLLazySlowPath.cpp
• Source/JavaScriptCore/ftl/FTLLocation.cpp
• Source/JavaScriptCore/ftl/FTLLocation.h
• Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
• Source/JavaScriptCore/ftl/FTLOSREntry.cpp
• Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp
• Source/JavaScriptCore/ftl/FTLOperations.cpp
• Source/JavaScriptCore/ftl/FTLSaveRestore.cpp
• Source/JavaScriptCore/ftl/FTLSaveRestore.h
• Source/JavaScriptCore/ftl/FTLState.cpp
• Source/JavaScriptCore/ftl/FTLThunks.cpp
• Source/JavaScriptCore/heap/AbstractSlotVisitorInlines.h
• Source/JavaScriptCore/heap/AllocatingScope.h
• Source/JavaScriptCore/heap/Allocator.h
• Source/JavaScriptCore/heap/BlockDirectory.cpp
• Source/JavaScriptCore/heap/BlockDirectory.h
• Source/JavaScriptCore/heap/BunV8HeapSnapshotBuilder.cpp
• Source/JavaScriptCore/heap/CellContainerInlines.h
• Source/JavaScriptCore/heap/CollectingScope.h
• Source/JavaScriptCore/heap/CompleteSubspace.cpp
• Source/JavaScriptCore/heap/CompleteSubspace.h
• Source/JavaScriptCore/heap/CompleteSubspaceInlines.h
• Source/JavaScriptCore/heap/GCActivityCallback.cpp
• Source/JavaScriptCore/heap/GCSafepointEpoch.cpp
• Source/JavaScriptCore/heap/GCSafepointEpoch.h
• Source/JavaScriptCore/heap/GCThreadLocalCache.cpp
• Source/JavaScriptCore/heap/GCThreadLocalCache.h
• Source/JavaScriptCore/heap/HandleSet.cpp
• Source/JavaScriptCore/heap/HandleSet.h
• Source/JavaScriptCore/heap/Heap.cpp
• Source/JavaScriptCore/heap/Heap.h
• Source/JavaScriptCore/heap/HeapCellInlines.h
• Source/JavaScriptCore/heap/HeapClientSet.cpp
• Source/JavaScriptCore/heap/HeapClientSet.h
• Source/JavaScriptCore/heap/HeapInlines.h
• Source/JavaScriptCore/heap/HeapIterationScope.h
• Source/JavaScriptCore/heap/HeapProfiler.h
• Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp
• Source/JavaScriptCore/heap/IncrementalSweeper.cpp
• Source/JavaScriptCore/heap/IsoCellSet.cpp
• Source/JavaScriptCore/heap/IsoSubspace.cpp
• Source/JavaScriptCore/heap/IsoSubspace.h
• Source/JavaScriptCore/heap/IsoSubspaceInlines.h
• Source/JavaScriptCore/heap/LocalAllocator.cpp
• Source/JavaScriptCore/heap/LocalAllocatorInlines.h
• Source/JavaScriptCore/heap/MachineStackMarker.cpp
• Source/JavaScriptCore/heap/MachineStackMarker.h
• Source/JavaScriptCore/heap/MarkedBlock.cpp
• Source/JavaScriptCore/heap/MarkedBlock.h
• Source/JavaScriptCore/heap/MarkedBlockInlines.h
• Source/JavaScriptCore/heap/MarkedSpace.cpp
• Source/JavaScriptCore/heap/MarkedSpace.h
• Source/JavaScriptCore/heap/PreciseAllocation.cpp
• Source/JavaScriptCore/heap/PreciseAllocation.h
• Source/JavaScriptCore/heap/PreciseSubspace.cpp
• Source/JavaScriptCore/heap/RunningScope.h
• Source/JavaScriptCore/heap/SharedHeapTestHarness.cpp
• Source/JavaScriptCore/heap/SharedHeapTestHarness.h
• Source/JavaScriptCore/heap/SlotVisitor.cpp
• Source/JavaScriptCore/heap/Strong.h
• Source/JavaScriptCore/heap/StrongInlines.h
• Source/JavaScriptCore/heap/StructureAlignedMemoryAllocator.cpp
• Source/JavaScriptCore/heap/Subspace.cpp
• Source/JavaScriptCore/heap/SweepingScope.h
• Source/JavaScriptCore/heap/WeakBlock.cpp
• Source/JavaScriptCore/heap/WeakSet.cpp
• Source/JavaScriptCore/heap/WeakSet.h
• Source/JavaScriptCore/heap/WeakSetInlines.h
• Source/JavaScriptCore/interpreter/CLoopStack.cpp
• Source/JavaScriptCore/interpreter/CLoopStack.h
• Source/JavaScriptCore/interpreter/CLoopStackInlines.h
• Source/JavaScriptCore/interpreter/CallFrame.cpp
• Source/JavaScriptCore/interpreter/FrameTracers.h
• Source/JavaScriptCore/interpreter/Interpreter.cpp
• Source/JavaScriptCore/interpreter/InterpreterInlines.h
• Source/JavaScriptCore/interpreter/StackVisitor.cpp
• Source/JavaScriptCore/jit/AssemblyHelpers.cpp
• Source/JavaScriptCore/jit/AssemblyHelpers.h
• Source/JavaScriptCore/jit/CCallHelpers.cpp
• Source/JavaScriptCore/jit/CCallHelpers.h
• Source/JavaScriptCore/jit/ConcurrentButterflyOperations.cpp
• Source/JavaScriptCore/jit/ConcurrentButterflyOperations.h

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Same flag inconsistency in stub phase.

Line 425 references --useThreads=true and --useThreadGIL=true but the test corpus uses --useJSThreads=1. Align the flag names with what the tests actually use.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/workflows/thread-prep.js at line 425, The comment uses flag names
that don't match the tests: replace the stub-phase reference to
"--useThreads=true" and the parenthetical "--useThreadGIL=true" with the
test-used flag "--useJSThreads=1" (and if the codebase expects a separate GIL
flag, rename that occurrence to the test-aligned variant or remove the
misleading default), and ensure the USE_BUN_JSC_ADDITIONS gating text still
appears; update any string/flag checks or docs in the same stub block that
reference "--useThreads" or "--useThreadGIL" to use "--useJSThreads" so names
are consistent with the test corpus.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

ASAN is advertised but never executed in the scanner battery.

Line 3 promises ASAN coverage, but SCANNERS has no ASAN run. This creates a false hardening guarantee and drops a key memory-safety signal.

Also applies to: 28-38

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/workflows/thread-scanners.js at line 3, The SCANNERS configuration
advertises ASAN but never defines or runs an ASAN scanner; update the SCANNERS
array in thread-scanners.js to include a dedicated ASAN scanner entry (and any
missing entries around the other scanner entries at the block referenced by
lines ~28-38) that: names the job "asan", sets the runner/steps to build the
threads target with -fsanitize=address (and related flags as needed), runs the
instrumented binary under the test harness, and publishes the sanitizer
logs/exit status; ensure the scanner entry follows the same job schema as the
existing TSAN/UBSAN entries and is referenced wherever SCANNERS is iterated so
ASAN actually executes.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Triage currently drops findings by scan order, not risk.

slice(0, 30) is applied before prioritization, so high-severity findings can be discarded arbitrarily when report volume is high.

Proposed fix

-const items = reports.filter(f => f.severity !== 'low').slice(0, 30)
-  .map(f => ({ ...f, id: ident(f.id), scanner: ident(f.scanner) }))
+const rank = { high: 0, medium: 1, low: 2 }
+const items = reports
+  .filter(f => f.severity !== 'low')
+  .sort((a, b) => rank[a.severity] - rank[b.severity])
+  .slice(0, 30)
+  .map(f => ({ ...f, id: ident(f.id), scanner: ident(f.scanner) }))

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const items = reports.filter(f => f.severity !== 'low').slice(0, 30)
	.map(f => ({ ...f, id: ident(f.id), scanner: ident(f.scanner) }))
	log(`Triage: ${items.length} medium/high findings (of ${reports.length} total)`)
	const rank = { high: 0, medium: 1, low: 2 }
	const items = reports
	.filter(f => f.severity !== 'low')
	.sort((a, b) => rank[a.severity] - rank[b.severity])
	.slice(0, 30)
	.map(f => ({ ...f, id: ident(f.id), scanner: ident(f.scanner) }))
	log(`Triage: ${items.length} medium/high findings (of ${reports.length} total)`)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/workflows/thread-scanners.js around lines 50 - 52, The current
pipeline filters reports then slices to 30 before mapping, causing slice(0, 30)
to drop items by input order rather than risk; change the flow in the block that
builds items (where reports, ident, and items are used) to sort/prioritize the
filtered reports by severity/risk (e.g., map severities to priority values and
sort descending by that value, then by other tie-breakers like date or
confidence) before calling slice(0, 30), then map to add id: ident(f.id) and
scanner: ident(f.scanner) and log the resulting items.length; ensure the sorting
happens on the filtered array (reports.filter(...)) and preserve existing ident
usage.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add an explicit start barrier so interleavings are guaranteed.

Line 24 and Line 66 spawn competing threads without synchronizing start. The reader can complete most/all samples before mutators run, which can produce false-green results for both invariants.

Proposed fix

+function makeStartBarrier(participants) {
+    const gateLock = new Lock();
+    const gateCond = new Condition();
+    let ready = 0;
+    let open = false;
+    return () => gateLock.hold(() => {
+        if (++ready === participants) {
+            open = true;
+            gateCond.notifyAll();
+            return;
+        }
+        while (!open)
+            gateCond.wait(gateLock);
+    });
+}
+
 for (let round = 0; round < ROUNDS; ++round) {
     const o = { v: 0 };
+    const awaitStart = makeStartBarrier(3);
@@
     const writer = new Thread(() => {
+        awaitStart();
         for (let i = 1; i <= WRITES; ++i)
             o.v = i;
     });
@@
     const transitioner = new Thread(() => {
+        awaitStart();
         for (let i = 0; i < TRANSITIONS; ++i)
             o["grow" + round + "_" + i] = i;
     });
@@
     const reader = new Thread(() => {
+        awaitStart();
         let last = 0;
         for (let s = 0; s < SAMPLES; ++s) {
@@
 for (let round = 0; round < ROUNDS; ++round) {
     const a = [0];
+    const awaitStart = makeStartBarrier(3);
     const writer = new Thread(() => {
+        awaitStart();
         for (let i = 1; i <= WRITES; ++i)
             a[0] = i;
     });
     const grower = new Thread(() => {
+        awaitStart();
         for (let i = 1; i <= 512; ++i)
             a[i] = -i; // out-of-bounds store: forces vector growth
     });
     const reader = new Thread(() => {
+        awaitStart();
         let lastV = 0, lastLen = 1;

Also applies to: 66-89

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/invariants/no-time-travel.js` around lines 24 - 47, The test
spawns writer, transitioner and reader Threads without synchronizing their
start, allowing the reader to finish before writers run; add an explicit start
barrier (e.g., a shared boolean/flag or a Barrier object) that each Thread (the
writer Thread, transitioner Thread, and reader Thread) waits on before entering
their loops, and only flip/signal the barrier once all three Threads are created
so they begin competing simultaneously; update the Thread callbacks (the
closures that set o.v, grow..., and sample o.v) to spin/wait on that shared
start flag or await the Barrier before proceeding.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Potential name mismatch in geomean paste not detected before output.

The awk script prints "NAME-MISMATCH" to stdout on mismatch (line 144), but GEOMEAN_10 is only checked after the entire pipeline completes. If names mismatch for a single benchmark, the RATIO lines for prior benchmarks have already been printed via stderr redirection, then the pipeline exits with "NAME-MISMATCH" as the final value.

Consider failing fast or restructuring to avoid partial output before detecting the mismatch.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@JSTests/threads/jit/bench-gates.sh` around lines 142 - 148, The GEOMEAN_10
pipeline can emit RATIO lines to stderr before detecting a NAME-MISMATCH; modify
the awk invocation used with paste <(echo "$OFF_RESULTS") <(echo
"$ON10_RESULTS") so it validates all input row name pairs before printing any
per-benchmark RATIO or writing to stderr. Concretely, in the awk block used to
compute GEOMEAN_10, first accumulate rows (e.g., store per-row ratios in arrays
and maintain logsum/n) and only output the per-benchmark "RATIO ..." lines and
the final geomean after the END check confirms there was no NAME-MISMATCH;
ensure if a mismatch is detected you exit non-zero without emitting prior RATIO
lines, preserving existing stderr handling that records FLAGON-1-0 lines.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Same ASSERT-only protection issue in setProtoLoadMode.

Line 268 uses ASSERT(!Options::useJSThreads()) which compiles out in release. If this path is reachable with JSThreads enabled, it will silently proceed with an unsafe 16-byte write.

🛡️ Proposed fix

-    ASSERT(!Options::useJSThreads());
+    RELEASE_ASSERT(!Options::useJSThreads());

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	inline void GetByIdModeMetadata::setProtoLoadMode(Structure* structure, PropertyOffset offset, JSObject* cachedSlot)
	{
	// SPEC-jit §4.3/I18: ProtoLoad's 16-byte record cannot be published as one
	// word; flag-on its sole installer (setupGetByIdPrototypeCache) is disabled
	// wholesale, so this must be unreachable.
	ASSERT(!Options::useJSThreads());
	inline void GetByIdModeMetadata::setProtoLoadMode(Structure* structure, PropertyOffset offset, JSObject* cachedSlot)
	{
	// SPEC-jit §4.3/I18: ProtoLoad's 16-byte record cannot be published as one
	// word; flag-on its sole installer (setupGetByIdPrototypeCache) is disabled
	// wholesale, so this must be unreachable.
	RELEASE_ASSERT(!Options::useJSThreads());

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/GetByIdMetadata.h` around lines 263 - 268, The
ASSERT in GetByIdModeMetadata::setProtoLoadMode (which checks
Options::useJSThreads()) compiles out in release and can allow an unsafe 16-byte
write when JSThreads are enabled; replace the compile-only ASSERT with a runtime
guard that aborts or otherwise prevents execution when Options::useJSThreads()
is true (e.g., a RELEASE_ASSERT or an explicit crash/return with clear logging),
so the unreachable assumption around setupGetByIdPrototypeCache remains enforced
at runtime and the unsafe proto write cannot occur.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve the checked structure ID in getter/setter self handlers.

offsetOfHolder() == 0 still makes these helpers serve own-property cases, but by then scratch1GPR no longer holds the just-compared structure ID and loadProperty() is called through the overload without structureIDGPR. In --useJSThreads on ARM64 that drops the R7/F7 dependency between the structure check and the GetterSetter-cell load, so a stale accessor cell can be observed after a matching structure check.

Also applies to: 6104-6112

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp` around lines 5611 -
5618, The getterHandlerImpl helper loses the validated structure ID when
offsetOfHolder() == 0 because scratch1GPR is overwritten before calling
loadProperty, causing loadProperty to be invoked without passing the
structureIDGPR and permitting a stale GetterSetter cell on ARM64 --useJSThreads;
preserve the compared structure ID and pass it into loadProperty by retaining
the original structure ID register (don’t clobber scratch1GPR or move the
payload into it) and call the overload of loadProperty that accepts a
structureIDGPR (use the same register used in the structure check), updating the
code paths around loadPtr/moveConditionally64 to keep the structure ID live for
the subsequent loadProperty call.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't turn a missed JSThreads gate into a hard trap.

This file still has local transition-handler selection paths. If one slips through under --useJSThreads, jit.breakpoint() aborts the process instead of chaining to the generic slow path the way the delete/replace handlers do. Please route this to fallback rather than trapping.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp` around lines 5854 -
5862, The current JSThreads guard uses jit.breakpoint() which aborts instead of
falling back; replace the hard trap with the same generic slow-path/handler
chaining used by the delete/replace transition handlers so missed selection
routes route to the fallback. Locate the block that checks
Options::useJSThreads() and remove the jit.breakpoint() call, and instead call
or jump to the existing generic transition fallback used by the delete/replace
handlers (mirror their chaining/emit-slow-path logic so the shared thunk
delegates to the slow path rather than trapping).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the structure-check dependency alive for string/symbol GetByVal self loads.

After emitDataICCheckUid() reuses scratch1GPR, the own-property path always passes InvalidGPRReg into loadHandlerImpl(). That defeats the new structureIDGPR threading exactly on the threaded ARM64 case this helper was added for, so the property load can be decoupled from the structure comparison.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp` around lines 6398 -
6400, The own-property path is losing the structure-check dependency because
emitDataICCheckUid() reuses scratch1GPR and then loadHandlerImpl<ownProperty> is
called with InvalidGPRReg; update the call so the threaded ARM64 structureID
register is forwarded instead of InvalidGPRReg. Specifically, after
emitDataICCheckUid() finishes, ensure you pass the structureIDGPR (or the
preserved register that holds the structure UID) into
loadHandlerImpl<ownProperty> in place of InvalidGPRReg so the structure-check
dependency remains alive for string/symbol GetByVal self loads (preserve or move
scratch1GPR into structureIDGPR if needed before the call).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard JSThreads-specific cache bypasses behind Bun feature macros

Line 1153, Line 1538, and Line 1898 add Options::useJSThreads() checks in Source/JavaScriptCore without USE(BUN_JSC_ADDITIONS) guards. This can break non-Bun JSC configurations and violates the repository contract for Bun-specific code paths.

Suggested patch

+#if USE(BUN_JSC_ADDITIONS)
                 if (Options::useJSThreads()) [[unlikely]]
                     return GiveUpOnCache;
+#endif

Apply the same guard pattern at all three locations (new-property transition, delete IC creation, and set-brand IC creation).

As per coding guidelines, Source/JavaScriptCore/**/*.{cpp,h}: Guard Bun-specific features with USE(BUN_JSC_ADDITIONS) and event-loop integration with USE(BUN_EVENT_LOOP).

Also applies to: 1535-1540, 1895-1900

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Source/JavaScriptCore/bytecode/Repatch.cpp` around lines 1147 - 1154, Wrap
the direct runtime check Options::useJSThreads() in a Bun feature guard so
non-Bun builds don't reference Bun-only features: surround the existing if
(Options::useJSThreads()) [[unlikely]] return GiveUpOnCache; (and the analogous
checks at the other two sites for delete IC creation and set-brand IC creation)
with `#if` USE(BUN_JSC_ADDITIONS) ... `#endif` so the JSThreads-specific cache
bypass is compiled only when USE(BUN_JSC_ADDITIONS) is enabled; keep the
original behavior inside the guard unchanged.

Source: Coding guidelines

[claude]

🟡 Off-by-one in the post-loop exhaustion check: if the gates go green on the final iteration (round == MAX_ROUNDS) and break, the subsequent if (round >= MAX_ROUNDS) is still true, so the workflow logs "Stopped … without all-green" and returns { fixed: false } despite having just logged "All gates green". The same pattern bites thread-ungil.js:528-532 (the !(lastReport?.allGreen) guard is defeated because lastReport = verify is assigned after the success break) and thread-implement.js:436 (misleading log only). Track an explicit green flag set before break, or in thread-ungil.js move lastReport = verify above the allGreen check.

Extended reasoning...

What the bug is

In thread-fix.js the main loop is:

const MAX_ROUNDS = 6
let round = 0
while (round < MAX_ROUNDS) {
  round++
  // … triage …
  if (triage.allGreen) { log(`All gates green after ${round - 1} fix round(s)`); break }
  // … fix + verify …
  if (verify.allGreen) { log(`All gates green after ${round} fix round(s)`); break }
  lastReport = verify
}
if (round >= MAX_ROUNDS) {
  log(`Stopped after ${MAX_ROUNDS} rounds without all-green — needs human attention`)
  return { fixed: false, rounds: round, lastReport }
}
return { fixed: true, rounds: round }

The post-loop check uses round >= MAX_ROUNDS as a proxy for "loop exhausted without success", but that proxy is wrong: a successful break on the final iteration leaves round at exactly MAX_ROUNDS, which is indistinguishable from natural loop exhaustion. There is no separate success flag.

Step-by-step proof (thread-fix.js)

With MAX_ROUNDS = 6:

1. round = 0. Iterations 1–5 run; each finds gates still broken, sets lastReport = verify, loops.
2. Iteration 6: round < 6 (5 < 6) is true; enter body; round++ → round = 6.
3. Triage runs, finds items, fixes are applied, then verify runs and reports verify.allGreen === true.
4. Line 203 logs "All gates green after 6 fix round(s)" and breaks. lastReport still holds round-5's non-green report.
5. Control reaches line 208: round >= MAX_ROUNDS → 6 >= 6 → true.
6. Logs "Stopped after 6 rounds without all-green — needs human attention" and returns { fixed: false, rounds: 6, lastReport: <round-5 report> }.
7. The { fixed: true } return at line 212 is unreachable for this case.

The same applies if triage.allGreen (line 126) breaks on round 6.

Why nothing prevents it

The only discriminator between "broke out green" and "ran out of rounds" is the value of round, and on the final permitted iteration both outcomes leave round == MAX_ROUNDS. No boolean is set on the success path.

thread-ungil.js (528–532)

This file appears to guard the case:

if (verify.allGreen) { log(`LADDER GREEN …`); break }
lastReport = verify
…
if (round >= MAX_ROUNDS && !(lastReport?.allGreen)) {
  return { ungil: false, … }
}

But the guard is defeated: the success branch breaks before assigning lastReport = verify, so when round-8 succeeds, lastReport is still round-7's report (which had allGreen === false — otherwise the loop would have ended on round 7). Thus 8 >= 8 && !(false) → true → { ungil: false }. Moving lastReport = verify above the allGreen check, or tracking a separate flag, fixes it.

thread-implement.js (436)

The build loop has the same structure (MAX_ROUNDS = 30); a green build on round 30 still triggers the Stopped at 30 build rounds without a green build log. Here it's only a misleading log line — the return value at lines 438–447 doesn't carry a success/failure boolean — but the log contradicts the immediately-preceding "Build green after 30 round(s)".

Impact

These are dev-orchestration workflow scripts under .claude/workflows/, not engine code. The actual file edits applied to the working tree are correct either way — only the structured return value and the trailing log line lie. A human reading the narrator log would see "All gates green …" immediately followed by "Stopped … without all-green", which is confusing but recoverable. The trigger is the edge case of success on exactly the last permitted round (1-in-MAX_ROUNDS at worst). Hence nit, not blocking.

Fix

Set an explicit flag before each success break:

let green = false
while (round < MAX_ROUNDS) {
  round++
  …
  if (verify.allGreen) { green = true; log(…); break }
  lastReport = verify
}
if (!green) {
  log(`Stopped after ${MAX_ROUNDS} rounds without all-green …`)
  return { fixed: false, rounds: round, lastReport }
}
return { fixed: true, rounds: round }

For thread-ungil.js, alternatively move lastReport = verify above the if (verify.allGreen) check so the existing guard works as intended.

[claude]

🟡 If success (allGreen/build.success) lands on the final loop iteration, round already equals MAX_ROUNDS after the break, so the post-loop if (round >= MAX_ROUNDS) fires and the workflow returns {fixed: false} (or logs "did not converge") despite succeeding. The same off-by-one shape recurs at thread-implement.js:436, thread-ungil.js:366, and thread-ungil.js:528 (where lastReport still holds the prior round's non-green report, so the !(lastReport?.allGreen) guard does not help). Track an explicit success flag set at the break sites and gate the failure return on that instead of round >= MAX_ROUNDS.

Extended reasoning...

What the bug is

These workflow scripts use the pattern let round = 0; while (round < MAX_ROUNDS) { round++; ...; if (success) break; } followed by a post-loop check if (round >= MAX_ROUNDS) { return failure }. Because round is incremented at the top of the loop body, on the final iteration round === MAX_ROUNDS inside the body. If the success condition triggers break on that iteration, control leaves the loop with round === MAX_ROUNDS, and the post-loop >= check cannot tell "succeeded on the last try" from "exhausted all tries."

Step-by-step proof (thread-fix.js, MAX_ROUNDS=6)

1. Round 5 completes; verify is not allGreen → lastReport = verify (line 204), loop continues.
2. Loop condition: round < 6 → 5 < 6 → true. Enter body, round++ → round === 6.
3. Triage runs (line 112). Suppose triage.allGreen === true on this 6th attempt.
4. Line 126: log("All gates green after 5 fix round(s)"); break.
5. Post-loop, line 208: if (round >= MAX_ROUNDS) → 6 >= 6 → true.
6. Returns { fixed: false, rounds: 6, lastReport } — the wrong result.

The same trace applies if verify.allGreen (line 203) becomes true on round 6: break exits with round === 6, line 208 fires, {fixed: false} is returned.

The other three sites

thread-ungil.js:528 (Ladder, MAX_ROUNDS=8): The post-loop guard is if (round >= MAX_ROUNDS && !(lastReport?.allGreen)), which looks like it handles this — but lastReport is only assigned on the non-green path (line 462, after the if (verify.allGreen) break at line 461). So on a round-8 success, lastReport still holds round 7's non-green report, !(lastReport?.allGreen) is true, and {ungil: false} is returned despite the ladder being green.

thread-implement.js:436 (Build, MAX_ROUNDS=30) and thread-ungil.js:366 (Build, MAX_ROUNDS=20): Same shape, but the consequence is only a misleading log() ("Stopped at N build rounds without a green build") emitted right after the correct "Build green after N round(s)" log — the return value is not affected.

Why nothing prevents it

The post-loop checks compare only round against MAX_ROUNDS, with no separate flag recording why the loop exited. The one site that tries (!(lastReport?.allGreen) at thread-ungil.js:528) reads stale state because lastReport is updated after the success-break, not before.

Impact

Low. These are .claude/workflows/ orchestration scripts — internal dev tooling that has already been run to produce this PR, not engine code or shipped artifacts. The bug only manifests when success lands on exactly the boundary iteration. In the two return-affecting cases (thread-fix.js, thread-ungil.js ladder), the immediately-preceding log() correctly says "All gates green" / "LADDER GREEN", so a human reading the run output would see the contradiction and re-run rather than be silently misled. The other two sites only produce a confusing log line.

How to fix

Set an explicit let success = false before the loop, assign success = true at each break site, and change the post-loop check to if (!success). Alternatively, in thread-fix.js return {fixed: true} directly at the break sites; in thread-ungil.js:528, assign lastReport = verify before the allGreen check so the guard reads fresh state.

[claude]

🟡 Several cloned workflow scripts retain stale content from their templates: thread-bughunter2/3/4.js all share meta.name = 'thread-bughunter' (would collide in the named-workflow registry) plus the original butterfly-corruption meta.description, and their Evidence-phase prompt items 2–4 still instruct decoding '1003008 vs 1003017' and toggling forceSegmentedButterflies — irrelevant to the GC under-marking / STW watchdog / W≥16 libpas targets each file's COMMON block actually hunts. Likewise thread-ab17c/d/e.js, thread-closeout.js, and thread-cve-close.js still emit log('ab17b review clean') / log('ab17b VERIFIED GREEN') and carry a stale PINNED_VERIFY V5 parenthetical ('was +10.59% entering this round and family 1 exists to fix it') even where the bench state differs and no such family exists. Low impact — these are spent one-shot bring-up scripts, not engine code — but if they're being kept as the honest bring-up record, at minimum the duplicate meta.name values should be made unique (e.g. thread-bughunter2).

Extended reasoning...

What the bug is

Two families of .claude/workflows/ scripts were created by copy-editing a template and retargeting only part of it:

Family A — thread-bughunter{2,3,4}.js (cloned from thread-bughunter.js):

• All four files export meta.name = 'thread-bughunter' and the identical meta.description ('Hypothesis-driven hunt for the butterfly-stress silent corruption (named property reads a WRONG VALUE ~1/120 …)').
• Each clone's COMMON block was correctly retargeted to a different bug — bughunter2 hunts GC under-marking, bughunter3 hunts the STW watchdog abort, bughunter4 hunts the W≥16 libpas crash family — and Evidence-prompt item 1 was retargeted to match.
• But Evidence-prompt items 2–4 were copied verbatim from the butterfly original: 'decode 1003008 vs 1003017 precisely: stale-by-9-writes? cross-property?', '--useFTLJIT=0? --useDFGJIT=0?', 'forceSegmentedButterflies=1? verifyConcurrentButterfly=1?'. None of this applies to a GC under-marking bug or a stop-the-world watchdog timeout.

Family B — thread-ab17{c,d,e}.js, thread-closeout.js, thread-cve-close.js (cloned from thread-ab17b.js):

• The Review/Verify loops still emit log('ab17b review clean (round N)'), log('ab17b review round N: …'), and log('ab17b VERIFIED GREEN …').
• The PINNED_VERIFY V5 line still reads 'transition-heavy-constructor was +10.59% entering this round and family 1 exists to fix it' — copied from ab17c (where bench was +10.59% and F1-bench-flagoff was the first family). In ab17d the same file's COMMON says bench is at +0.41% and the families are S1/S2; in ab17e it's +3.05% with item T1; in closeout/cve-close bench is explicitly parked and there is no bench-fix family at all.

Step-by-step concrete example

Take thread-bughunter2.js:

1. Lines 1–9 export meta = { name: 'thread-bughunter', description: 'Hypothesis-driven hunt for the butterfly-stress silent corruption (named property reads a WRONG VALUE ~1/120 under load, no crash) …' }.
2. Lines 40–56 (COMMON) describe a completely different bug: 'shared-GC-heap UNDER-MARKING corruption … live cells are swept and re-allocated while in use … DETERMINISTIC REPRO EXISTS: Tools/threads/scalebench/js/repro-bigint-shared-ingest.js'.
3. Lines 60–66 (Evidence item 1) correctly tell the agent to reproduce repro-bigint-shared-ingest.js on Release with W=4.
4. Lines 67–76 (Evidence items 2–4) then tell the agent to 'decode 1003008 vs 1003017 precisely: stale-by-9-writes? cross-property?', test forceSegmentedButterflies=1, and try verifyConcurrentButterfly=1 — there is no '1003008 vs 1003017' value pair in the GC under-marking repro, and segmented-butterfly stress flags don't bisect a GC root-coverage bug.
5. If this script were ever invoked by name via Workflow({name: 'thread-bughunter'}), the registry would resolve four files claiming the same name.

And thread-ab17d.js:

1. Line 35 (COMMON) states 'bench gate green at +0.41% worst'.
2. Lines 49–73 define FAMILIES = [['S1-ic-publish-uaf', …], ['S2-n3-first-install', …]] — no bench family.
3. Line 121 (PINNED_VERIFY V5, passed to the verify agent) says 'transition-heavy-constructor was +10.59% entering this round and family 1 exists to fix it'. Both halves contradict the same file's COMMON and FAMILIES.
4. Lines 101/102/139 emit log('ab17b review clean') / log('ab17b VERIFIED GREEN …') for a workflow named ab17d.

Why nothing prevents it

These are hand-cloned one-shot scripts; there's no lint or test for meta.name uniqueness across .claude/workflows/, and the agent prompts are free-text. The retargeted COMMON block dominates each prompt, so the stale items 2–4 didn't stop the workflows from succeeding — but that's luck (the agent self-correcting against contradictory instructions), not design.

Impact (and why this is a nit, not a blocker)

This is dev-tooling hygiene only, not engine code:

• These scripts are checked into .claude/workflows/, not Source/ or JSTests/. Nothing here ships.
• Per the PR's own bring-up log and per the clones' own COMMON text (e.g. bughunter4: 'the GC under-marking fix … and the watchdog fire-under-lock fix are LANDED and verified'), the bugs each script hunted are already solved — these are spent artifacts.
• The 'ab17b' strings in log() are narrator output to the workflow progress UI; they never reach an agent prompt and have zero functional effect.
• The stale PINNED_VERIFY V5 parenthetical is in an agent prompt, but the operative instruction — '>1% = FAIL with a scoped item, no exceptions, do not hide it' — is unchanged and correct, and the verify agent runs bench-gate.sh and reports the actual number regardless of what the parenthetical claims.
• The meta.name collision was never hit in practice: these scripts are filename-distinguished (thread-bughunter2.js, etc.) and were invoked via Workflow({scriptPath}), not {name}.

Addressing the 'not actionable' objection

Two reviewers argued this isn't worth a comment because the scripts are throwaway debugging-session artifacts that already ran. That's a fair characterization of the impact — hence nit. But the PR description explicitly checks these in as the honest bring-up record ('nothing is retconned'), and the workflow registry does key on meta.name. If the intent is to keep them as documentation, then (a) documentation that contradicts itself between meta and COMMON undercuts that purpose, and (b) four files sharing one meta.name is a latent footgun for anyone who later tries Workflow({name: 'thread-bughunter'}). The fix is trivial enough (one line per file) that it's worth noting even at nit severity.

How to fix

Minimal:

• thread-bughunter{2,3,4}.js: change meta.name to 'thread-bughunter2' / '3' / '4', update meta.description to a one-liner matching each file's COMMON target, and either delete Evidence-prompt items 2–4 or rewrite them for the actual bug class (GC root coverage / stop-protocol / allocator double-free).
• thread-ab17{c,d,e}.js, thread-closeout.js, thread-cve-close.js: s/ab17b/${actualName}/ in the three log() strings, and update the V5 parenthetical to the bench number stated in each file's own COMMON (or drop the parenthetical entirely — the operative '>1% = FAIL' clause already says everything needed).

Alternatively, if the position is that these are frozen historical artifacts not meant to be re-run or read as current, a one-line header comment saying so would resolve it just as well.

[xdevfaheem]

wth

[eran-broder]

I need this so bad. The moment this is adopted i am throwing away all my shitty python code and coming back to TS\JS
PLEASE MAKE THIS A REALITY!!!!

[9Morello]

@Jarred-Sumner any chance this could be released/made available in Bun through an experimental feature flag?

[robobun]

Per-hunk review of the engine diff (part 1 of 2). A maintainer asked for a pushback review and a per-hunk pass, so this review reads every hunk under Source/ (523 files, 2985 hunks, +79,320/-4,190 at 3a14f2a) and asks two questions: should the change exist in this shape, and is each hunk right. Nothing here was built or run. Every inline comment is a static claim with the cited lines. Please treat each one as a reading that can be wrong, and push back where it is.

Disposition: restructure, do not merge as is

The PR body already says this branch "may never merge" and "exists so the design and the code can be read and argued with". That framing is correct, and the PR should be a Draft. Five facts drive the disposition:

1. It is not mergeable. A three-way merge against main today conflicts in 117 files (5 of them files main has since deleted). The branch is 5,782 commits behind, and the fork takes an upstream merge about weekly. The largest edits sit in the files that churn most: Heap.cpp (+5,239), FTLLowerDFGToB3.cpp (+1,598), DFGSpeculativeJIT.cpp, VM.cpp/.h, JSObject.cpp, Structure.h, LowLevelInterpreter64.asm.
2. About 72% of the added lines are not engine code, and much of that should never be in the repo: a 2.3 MB compiled Go binary (Tools/threads/scalebench/out-run1/bench-go), 18 .class files, 3.8 MB of gzipped logs, 121 core-dump backtraces, 290 .out files, crash dumps with /root/WebKit paths, 26 .claude/workflows scripts hardcoded to /root/WebKit, and docs/aot (4.6k lines), which its own generating script calls unrelated. docs/threads is 75.5k lines, of which about 6k are the normative specs.
3. The flag-off invariant is not established by anything in the tree. Tools/threads/v5a-identity.sh runs the same binary with --useJSThreads=false and with no flag; the default is false, so it compares a binary with itself. bench-gate.sh compares against baseline.json, recorded 2026-06-05 on a tree that already had the threads code (the merge base is 2026-06-04; INTEGRATE-ungil.md:1931 says no reference binary exists). The golden-disassembly gate has no recorded golden file. The in-tree ledger records the serial gate red on transition-heavy-constructor at +3.9% to +7.6% across sessions; the PR body's "3-4%" is the oldest number.
4. Real parallelism is behind a flag the code labels DEVELOPMENT ONLY. Options.cpp:813-816 forces useThreadGIL=1 unless useThreadGILOffUnsafe=1, because "blocker-grade activation items remain open". GIL-off also force-disables useWasm and useLOLJIT, runs only on Linux x86-64/arm64 (macOS fail-stops in AssemblyHelpers.cpp:237-261, Windows CRASH()es at option parsing, Options.cpp:1055). The PR body's status section describes the GIL-off shape as the thing that passes the suite. The body should say which configuration that is.
5. The review found 3 blockers and 152 majors in the engine code (below). About 40 of the findings, majors and minors together, are deterministic with --useJSThreads=1 and the default GIL, so the GIL-on mode the PR calls a verified fallback is not one yet.

Blockers

• runtime/StructureInlines.h:167: flag-on, forEachProperty runs the functor under the non-recursive Structure m_lock. FastStringifier's functor recurses into forEachProperty for a nested object of the same Structure. JSON.stringify({a:{a:1}}) self-deadlocks with --useJSThreads=1 alone.
• dfg/DFGSpeculativeJIT64.cpp:8780: EnumeratorPutByVal's out-of-line store masks the tag and stores through the word with no write predicate. For a segmented object (any out-of-line add by a non-owner thread, GIL-on included) the store lands in the spine. Baseline and FTL already refuse this path; the comment's own FIXME defers the predicate.
• runtime/ConcurrentButterfly.cpp:640: convertToSegmentedButterfly reads flat->vectorLength() for every structure with an indexing header. A typed-array view's header holds an ArrayBuffer*, so the high 32 bits of a pointer size the fragments, the hole fill writes past the 16-byte butterfly cell, and the GC aliasing range can skip live fragments.

Themes across the 558 findings (counts include the minors posted in part 2)

• Deterministic flag-on failures (43): the three blockers, for (k in o) o[k] = v storing base[index] for odd indices in Baseline (jit/JITPropertyAccess.cpp:2229), RELEASE_ASSERT(enteredVMs <= 1) in the GIL-on stop stub that aborts the process when a second VM (a Bun Worker) is entered (bytecode/JSThreadsSafepoint.cpp:518), heap snapshots hanging once the heap is a shared server (heap/Heap.cpp:3008).
• Flag-off is not unchanged (7 majors, 62 minors): WriteBarrierBase<Unknown> slot accessors are unconditional relaxed atomics (runtime/JSCJSValue.h:1289), which stops clang from turning a slot-clear loop into memset or vectorizing a copy loop. Every C++ write barrier and DeferGC gains an Options test (heap/Heap.h:1049). StringImpl hashing became lock-prefixed RMWs and deref() gained a global load and branch (WTF/text/StringImpl.h:1263). ArrayBuffer ref/deref became atomic RMW (WTF/DeferrableRefCounted.h:48). The LLInt is assembled at build time and now runs about 45 gate sequences plus 12 fast-path byte tests flag-off. Two flag-off defects: a Weak cleared into a freed WeakBlock at VM teardown (heap/GCSafepointEpoch.cpp:56) and a Structure lock held across a GC-allocating install (runtime/StructureRareData.cpp:282). CMakeLists.txt:1214 omits four newly included headers from the exported list, so Bun does not compile against this tree.
• Leaks, hard caps, teardown order (28): call-link records pin their callee CodeBlock and the only unpin runs at epoch expiry, which a dying caller never reaches, so compiled code is never reclaimed (bytecode/CallLinkInfo.cpp:239, heap/Heap.cpp:5126). The baked scratch registry hands out indices per compiled exit and never frees them, then RELEASE_ASSERTs at 16384 (runtime/VMLite.h:371). Several use-after-free paths in ~VM/~Heap ordering (bytecode/RetiredJITArtifacts.cpp:242, runtime/VMLite.h:267, runtime/DeferredWorkTimer.cpp:504).
• Check-then-reload in the shared object model, GIL-off (61): a mayBeSegmentedButterfly() guard followed by a fresh butterfly() load, or an indexing type read before an allocation that can park and trusted after it. The consequences are out-of-bounds writes and doubles decoded as cells, from legal racy programs that THREAD.md promises can only yield stale values (runtime/JSArray.cpp:2290, runtime/JSObject.h:366, runtime/ArrayPrototype.cpp:630, dfg/DFGSpeculativeJIT.cpp:9850).
• Watchpoint, IC and JIT protocol races, GIL-off (57): WatchpointSet::add re-arms a set a concurrent fire already invalidated (bytecode/Watchpoint.cpp:203). The epoch jettison that the hoisted-fact model depends on walks from the raw VM-block topCallFrame, which nothing writes GIL-off, so it jettisons nothing (bytecode/JSThreadsSafepoint.cpp:1019, runtime/VMTraps.cpp:718). On ARM64, loadVMLite into the macro memory temp self-clobbers for TLS offsets above 32760 (jit/AssemblyHelpers.cpp:613).
• Shared heap server (36), VM state and parking (65), ArrayBuffer detach and string lifetime (22): Heap::protect/unprotect mutate an unlocked HashCountedSet from N threads (heap/Heap.cpp:907). The TLC snapshot is keyed by thread, not server, so a nested foreign-VM entry allocates from the wrong heap (heap/GCThreadLocalCache.cpp:130). VM::notifyNeedTermination, which Bun uses, is never seen by parked threads (runtime/VMTraps.cpp:1368). The main carrier keeps heap access across parks when the embedder pre-acquired it, which is Bun's pattern (runtime/JSLock.cpp:1297).

Bun integration

The PR does not say what Bun needs to ship this, so here is the list from reading both trees. Bun takes the JSLock once in Run::start and never drops it, so in the GIL-on mode a spawned thread only runs while the main thread is inside join(). Bun holds heap access for the process lifetime (ZigGlobalObject.cpp:484) and idles in epoll_wait, which GIL-off treats as a wedged mutator and aborts after the 30 s watchdog (bytecode/JSThreadsSafepoint.cpp:623). Bun's per-class IsoSubspace has one LocalAllocator per VM, so any Bun object allocated on a spawned thread pops one FreeList from N threads. The blocking-policy hook is isAtomicsWaitAllowedOnCurrentThread, which Bun answers true. SPEC-nativeaffinity.md is a draft with no engine implementation, so nothing stops a spawned thread from entering Bun natives where VirtualMachine::get() is unset. Bun forwards BUN_JSC_* to Options::setOption, so a WebKit bump alone makes BUN_JSC_useJSThreads=1 reachable.

What this review did not do

It read only Source/. It did not read Tools/threads, JSTests/threads, the spec histories, or .claude/. It did not build jsc on any platform, run the corpus, TSAN, or either gate. Average confidence of the posted findings is 0.7. The 20 findings a second pass refuted are not posted.

Suggested next steps

1. Mark the PR Draft. Strip the binaries, logs, crash dumps, checked-in .diff candidates, docs/aot and .claude/workflows.
2. Land the flag-independent pieces as their own PRs with tests: the for-of TDZ exit-profile fix in DFGFixupPhase (gated on useJSThreads today, so flag-off users keep the bug), CodeBlock::jitCode() by raw pointer, the SharedAtomStringTable with its TestWebKitAPI test.
3. Build a same-toolchain pre-threads jsc and run bench-gate.sh as an interleaved A/B, plus an LLInt-only pass. Record a golden-disassembly baseline from that binary.
4. Build flag-on in debug, GIL-on, and add three tests that need no race: nested JSON.stringify, for (k in o) o[k] = v with out-of-line keys, a property add to a typed-array view from a second thread.
5. Rebase, add a compile-time ENABLE() gate, and re-land as a layered series along the existing Option seams (heap server, VM state, object model, API, JIT tiers, GIL removal).

Part 2 of this review carries the 190 minor inline comments (bugs, safety, flag-off, spec) and lists the 213 leftover and question items.

[robobun]

Major: Destructor frees the record without releasing the publish-time CodeBlock pin, making the named CodeBlock immortal (flag-on)

publishRecord pins codeBlockToTransfer at publish time (CallLinkInfo.cpp:140 -> Heap::pinRetiredCallLinkRecordCodeBlock, Heap.cpp:927) and the only unpin is ~RetiredCallLinkRecordWithPin at epoch expiry (RetiredJITArtifacts.cpp:340-345). This inline delete, the gilOff arm at line 230 and ~DirectCallLinkInfo (CallLinkInfo.h:640) free the record without that unpin, so the HashCountedSet entry stays forever. The marking constraint (Heap.cpp:5122-5127) marks every entry whose address is still in codeBlockSet, and codeBlockSet only drops unmarked blocks (CodeBlockSet.cpp:55-56), so a pinned callee never leaves it: the CodeBlock, its JITCode, ownerExecutable and global object (CodeBlock.cpp:2207-2208) are immortal. This runs flag-on whenever a DFG caller dies holding a live monomorphic or direct record: ~CodeBlock -> retireOptimizedJITCode -> ~CommonData destroys m_callLinkInfos and m_directCallLinkInfos (DFGCommonData.h:139-140). The leaked MetadataTable and DFG JITData CLIs (CodeBlock.cpp:1123, 1259) never free their records, so their pins leak the same way; Heap.cpp:934-935 concedes the retention. Fix: in ~CodeBlock flag-on, clearRecord(vm) every owned CLI (metadata, CommonData, JITData) before the teardown, as unlinkOrUpgradeIncomingCalls -> reset -> clearRecord already does for incoming records from the same context; the retired holder then unpins at epoch expiry.

[robobun]

Major: GIL-off ARM64: polymorphic thunk can pair the new record with a stale displaced m_stub

The polymorphic thunks load m_stub off the CallLinkInfo (LowLevelInterpreter.asm:3391, :3426; ThunkGenerators.cpp:390), not through the record the fast path just loaded (LowLevelInterpreter64.asm:3049-3068 hands the thunk only t2). setStub stores m_stub with release (CallLinkInfo.cpp:657) and publishes m_record afterwards (:684), but the reader has no address dependency or acquire between the two loads, so ARM64 under GIL-off can pair the new always-call record with the old m_stub. That stale routine is not benign: reset() keeps it published while mode leaves Polymorphic (CallLinkInfo.cpp:553-567, :260), stub() returns null (CallLinkInfo.h:279) so visitWeak never clears its slots again, and unlinkForcefully (PolymorphicCallStubRoutine.cpp:226) already delisted its nodes, so its CallSlot callee/target/codeBlock words dangle after the next GC or jettison. A match on a recycled callee cell jumps to freed code with a dead CodeBlock in the callee frame. This is the recorded IT-8 residual (INTEGRATE-ungil.md:1025), but the comment here calls the stale pointer safe. Close it: null the displaced routine's slot callee words in reset(), carry the stub pointer in the record, or make the thunk's m_stub load acquire under gilOff. x86 TSO and GIL-on are unaffected.

[robobun]

Major: Flag-on, every dead CodeBlock leaks its MetadataTable, JITData and OpCatch buffers, gated on useJSThreads rather than on the gilOff hazard

With useJSThreads on, every swept CodeBlock now permanently leaks its linked MetadataTable (this ref escape, which also pins the UnlinkedMetadataTable through linkingData().unlinkedMetadata), its BaselineJITData (line 1159), its DFG::JITData (line 1110) and its OpCatch ValueProfileAndVirtualRegisterBuffers (line 1004). The gate is Options::useJSThreads(), so the supported GIL-on shape pays it too, although there the straggler these leaks defend against cannot exist: spawned threads execute JS only under the VM's JSLock (ThreadObject.cpp:217-219), ~CodeBlock runs on a JSLock-holding mutator or inside a stopped world, and a parked sibling's frames keep the CodeBlock marked through the conservative scan. The PR's own GIL-off analysis closes the window too: TSAN-TRIAGE.md section 17.2 row 17 concludes dead-cell entry is impossible after the End-phase unlink and calls rows 7/8/16 defense-in-depth, and RetiredJITArtifacts.cpp:262-298 frees the optimized machine code inline on exactly that argument. Leaking the data behind freed code protects nothing. Old-age jettison, reoptimization and eval/new Function churn turn this into unbounded growth in a long-running process. Drop the four leaks, or at most gate them on vm.gilOff().

[robobun]

Major: GIL-on, every jettison now reaches the interim STW stub, which RELEASE_ASSERTs that no second VM is entered

With useJSThreads on and the GIL on, every jettison except old age now goes through this call, and a requester that is not already world-stopped takes the interim stub in JSThreadsSafepoint::stopTheWorldAndRun (JSThreadsSafepoint.cpp:476-518), which counts entered VMs across the whole process with VMManager::forEachVM and RELEASE_ASSERTs enteredVMs <= 1. VM::isEntered() is !!entryScope GIL-on (VM.h:400-405), so any other VM running JS on another thread counts. Bun creates a VM per Worker on its own thread, so a reoptimization, OSR-exit or watchpoint-fire jettison while a Worker executes JS aborts the process; GC-driven jettisons hit the same process-wide count through AlreadyStoppedWorldWitnessScope (JSThreadsSafepoint.cpp:167-194). The stub comment defers to the M7 entry tripwire, but M7 was deleted rather than applied (VMEntryScope.cpp:45-55), and SPEC-ungil U0b expects other VMs to keep running GIL-on beside the shared VM, so by the design's own terms this is a supported configuration. Another VM's mutators can never execute this VM's code, so either scope the count to the requesting VM (trivially satisfied under its JSLock) or refuse useJSThreads when a second VM is constructed.

[robobun]

Major: GIL-on stub RELEASE_ASSERT(enteredVMs <= 1) crashes any process with a second entered VM (Worker) under useJSThreads; the M7 guard this comment relies on was deleted

Under useJSThreads=1 the shipping shape is GIL-on: Options.cpp:813-815 forces useThreadGIL=1 unless useThreadGILOffUnsafe is set, so vm.gilOff() is false for every VM and this stub is the live path for every CodeBlock::jettison other than OldAge (CodeBlock.cpp:2834-2902) and every Class-A watchpoint fire (Watchpoint.cpp:381). The GIL is the per-VM JSLock (ThreadManager.h:57-59), not a process lock, so two VMs on two threads (a Bun Worker beside the main VM, or the jsc shell's $.agent.start at jsc.cpp:2586, which has no useJSThreads refusal) are both isEntered() (VM.h:400-405), and the first reoptimization jettison in either one fails this RELEASE_ASSERT; the GC path reaches the same count through AlreadyStoppedWorldWitnessScope and assertAlreadyStoppedEvidenceCoversEveryMutator (lines 167-195 and 197-209) on the first collection that jettisons dead DFG code while the other VM is entered. The comment above names manifest M7 in VMEntryScope as the structural guard, but VMEntryScope.cpp:45-56 records that tripwire as deleted, its replacement (park at entry during a stop) exists only on the gilOff thread-granular path, and VMManager.cpp:587-593 states that the stub and its entered-VM tripwire remain GIL-on-only. Nothing refuses a second VM at construction or entry under GIL-on, while docs/threads/INTEGRATE-jit.md:2101-2112 still lists M7 as REQUIRED before any flag-on configuration in which a second VM can be entered. GIL-off has the same hole in the other direction: the loser VM that SPEC-ungil U0b (docs/threads/SPEC-ungil.md:36-43) says keeps the GIL-on protocol takes this stub and counts the designated VM via isAnyThreadEntered (VM.cpp:3078), so it crashes whenever any thread of the winner is entered. Either refuse a second VM deterministically at VM construction or VMEntryScope under useJSThreads (and refuse Thread use once one exists), or give the GIL-on stub a real exclusion argument for the N-separate-VMs config (CodeBlocks are per-VM and each VM's JSLock already serializes its own mutators, so the count can be scoped to VMs that share code or a heap, the way assertAlreadyStoppedEvidenceCoversEveryMutator scopes it), and in either case fix the stale M7 references here and at JSThreadsSafepoint.h:126-133.

[robobun]

Major: std::atomic defer counters add seq_cst RMWs and stores on flag-off paths; the cross-thread justification is stale

At the merge base these were plain scalars; they are now std::atomic with default seq_cst and no useJSThreads gate. Flag-off, every DeferTraps scope now executes two seq_cst stores (VMTrapsInlines.h:85 and :90; xchg on x86-64, stlrb on arm64) and every DeferTermination scope two locked RMWs (VMTrapsInlines.h:48 and :63). DeferTraps wraps Interpreter::executeCallImpl/executeConstruct/executeProgram (Interpreter.cpp:1231, 1327, 1428), every JS microtask (JSMicrotask.cpp:135), the LLInt call slow path (LLIntSlowPaths.cpp:2352) and JIT call linking (JITOperations.cpp:237, RepatchInlines.h:184). The justification in the comment, that N GIL-off mutators reach this one instance through vm.traps(), is stale: DeferTermination.h:53/58 and DeferTraps (VMTrapsInlines.h:82) resolve vm.trapsForCurrentThread(), and every reader does the same (VMTraps.cpp:747, 752, 988; the slow paths at VMTraps.cpp:1272-1299 run on that instance), so in every mode these fields are touched only by their owning thread. The fences buy nothing and their cost is unmeasured. Revert to plain fields, and drop the matching stale note at VMTraps.cpp:735-739 that says deferTermination is reached via vm.traps().

[robobun]

Major: WasmGC module instantiation under useSharedGCHeap aborts the process; SPEC-ungil section I requires a LinkError precheck

SPEC-ungil section I (docs/threads/SPEC-ungil.md:703) and UNGIL-HANDOUT.md:2414-2420 supersede the manifest-11 RELEASE_ASSERT with a hasGCObjectTypes() precheck before instance construction that throws WebAssembly.LinkError in both GIL modes, keeping the assert only on non-JS-reachable paths. The tree has only the assert: nothing in JSWebAssemblyInstance::tryCreate (:310-345), constructJSWebAssemblyInstance or the module constructor tests hasGCObjectTypes() against useSharedGCHeap; the only other use is CompleteSubspace.cpp:380, the same assert. It is reachable from the main thread in a supported shape: --useJSThreads=1 --useSharedGCHeap=1 keeps useThreadGIL forced on (Options.cpp:813-816), so useWasm stays enabled (Options.cpp:843 disables it only GIL-off) and new WebAssembly.Instance(anyWasmGCModule) aborts the process after tryAllocateCell. The U17 positive arm (LinkError, no abort) has no test; wasm-refused-sd7.js covers only the spawned-thread TypeError. Add the precheck in tryCreate before allocation, plus the compile-side CompileError the spec names, and keep the assert as the backstop.

[robobun]

Major: SD7 spawned-thread wasm refusal is bypassed by JSPI resumption: a pinball reaction drained on a spawned thread resumes wasm frames there

The contract this assert documents is bypassed by JSPI. WebAssemblySuspending.cpp:187 attaches the pinball fulfill/reject handlers via performPromiseThen to the promise the Suspending import returned, and a spawned thread can hold that promise's resolver through the shared heap. GIL-on, the spawned thread's resolve enqueues the reaction on the single VM queue (VM.cpp:842) and the same thread drains it at fn-return (ThreadObject.cpp:349) or on depth-0 JSLock release (JSLock.cpp:1514). The handler is a host function, not a WebAssemblyFunction, so callWebAssemblyFunction's refusal and the jsCallICEntrypoint nullptr never run; pinballHandlerImplantSlice (PinballCompletion.cpp:171) re-implants the wasm frames on the spawned stack and continues wasm there. No JSPI file checks isJSThreadCurrent; the only wasm-side checks are JSWebAssemblyHelpers.h:63 and this line. GIL-off is unreachable (useWasm forced off) and GIL-on stack limits follow the JSLock holder, so the damage is: debug builds trip this ASSERT on the first prologue slow path, the normative SD7 refusal is void in release, and vm.topJSPIContext, kept VM-level on the carrier-only premise (VMLite.h:137), can chain across two stacks. Gate WebAssembly.promising and Suspending under useJSThreads like the other surfaces, or refuse in pinballHandlerInitContextForFulfill/Reject.

[robobun]

Major: matchLimit does not bound the poll-free Yarr region for paren-free patterns; GIL-off a polynomial ReDoS regex starves the 30s STW watchdog into a process abort

The comment and static_assert claim remainingMatchCount bounds the poll-free Yarr region to about 1.8s. That holds only for patterns with nested parentheses. The interpreter decrements once on entry to matchDisjunction (YarrInterpreter.cpp:1776), which runs once for the whole body (:2264); greedy backtracking and body-alternative retries are the MATCH_NEXT/BACKTRACK goto loop inside that single call (:1729-1730, :2099-2118). The JIT initializes the counter only when m_containsNestedSubpatterns (YarrJIT.cpp:7135) and decrements only in allocateParenContext (:773). Measured on stock JSC: /aaaab/.test("a".repeat(300)) takes 15s and n=500 exceeds two minutes, while the nested /^(a+)+$/ used by mc-safe-regexp-tts-watchdog.js hits the limit in 0.6s. Flag-off and GIL-on this is the pre-existing hang (the matcher holds the JSLock, so no sibling can request a stop). GIL-off, a sibling's Class-A stop or GC trips watchdogAssertStopProgress after 30s (JSThreadsSafepoint.cpp:623, :885) and aborts the process on user input, so the MC-SAFE S3 downgrade (CVE-AUDIT-RESULTS.md:307) rests on a false premise. Add a stop-word poll to the backtrack path of both engines, or drop the bound claim and reopen S3.

[robobun]

Major: Unconditional atomic RMW on the ArrayBuffer refcount sits on a hot flag-off path, not a cold one

The comment calls this a cold path, but every typed array view created over an existing buffer pays one ref plus deref on this count with the flag off: TypedArray.prototype.subarray (JSGenericTypedArrayViewPrototypeFunctions.h:2111), new TypedArray(buffer, ...) (JSGenericTypedArrayViewConstructorInlines.h:126), DataView, and Bun's Buffer.prototype.slice (JSBuffer.cpp:2254) all take a RefPtr that ConstructionContext does not keep. On a Xeon 8375C with clang 17 -O2, relaxed fetch_add plus acq_rel fetch_sub costs 11.5 ns per pair against 1.4 ns plain, and still adds about 9 ns interleaved with ordinary stores; a release build creates such a view in roughly 40 ns, so this is a 20 percent regression on Buffer.slice style code, well outside the 1 percent flag-off budget. The same path now also takes the GCIncomingRefCountedSet lock (GCIncomingRefCountedSetInlines.h:55), TSAN-TRIAGE.md 3.27 says a regression on these paths forces the mode split, and JSTests/threads/bench has no view-creation benchmark. The atomic is only needed GIL-off: flag-off and GIL-on serialize every ref/deref under the JSLock as upstream does. Gate it on a process-latched WTF::Config byte set alongside gilOffProcess (JSCConfig.h:100, the VM.h:734 pattern) and keep the plain increment when the byte is clear.

[robobun]

Per-hunk review of the engine diff (part 2 of 2). Part 1 is #249 (review) and carries the disposition, the 3 blockers and the 152 major findings.

This part carries the 190 minor findings of kind bug, safety, flag-off regression, or spec mismatch as inline comments. Each one is a static reading with the cited lines; none was built or run.

The remaining 213 minor findings are leftovers and questions: dead scaffolding (__has_include probes for headers that are in the tree, #else arms that never compile), kill switches that nothing reads (useThreadedDFG, useThreadedFTL, useThreadedLLIntICs), unused functions, never-incremented counters, instrumentation marked not for landing, and comments that state the opposite of the code (mechanisms described as pending that were deleted or landed, byte-identical claims on paths that are not). They are listed below by file instead of as inline comments, to keep the inline set to things that change behavior. A separate observation that is not listed per instance: about 49% of the added lines under Source/ are comments, and roughly a thousand of them cite bring-up artifacts (review rounds, task ids, TSAN report numbers, bench sections) that mean nothing once the process record leaves the tree.

213 leftover and question items, by file

assembler/AbstractMacroAssembler.h

• L100 initializeRandom() is dead code kept only so its definition still compiles

bytecode/BytecodeIntrinsicRegistry.cpp

• L155 Stale rationale: gilOffProcess byte is already latched before this registry is built; JSCConfig.h include is unused

bytecode/CallLinkInfo.h

• L146 Recursive-lock rationale is stale and blesses a GC-cell allocation that would deadlock against the refill stripe

bytecode/CodeBlock.cpp

• L1092 DFG arm rationale says the optimized machine code is leaked, but retireOptimizedJITCode now frees it inline
• L2589 FunctionExecutable::codeBlockWithEntrypointFor has no callers

bytecode/InlineCacheCompiler.cpp

• L4067 Comment's EOR zeroing-idiom claim is false and contradicts the shared dependency helper

bytecode/JSThreadsSafepoint.cpp

• L59 __has_include probes and their #else arms are dead scaffolding from the pre-integration split
• L623 30s watchdog turns a long host call on any sibling thread into a process crash
• L668 Dtor comment and AUDIT row claim exit-edge bump coverage for fireAllUnderClassAStop branch (1), which never publishes a context

bytecode/JSThreadsSafepoint.h

• L170 Header describes a stub-only stopTheWorldAndRun and an unwired, dormant watchdog; the .cpp has a real gilOff conductor and the watchdog is live

bytecode/PropertyInlineCache.cpp

• L279 Dictionary funnel is the documented R1-2 rule, but Repatch.cpp's T4 comment still claims cacheable dictionaries get cached
• L1320 Stale comment: resetStubAsJumpInAccess is reached from JIT slow paths with the world running, not only world-stopped

bytecode/PropertyInlineCache.h

• L288 'KNOWN RESIDUAL WRITER ... when that file is in scope' leaves the m_identifier conversion half done

bytecode/Repatch.cpp

• L1481 Stale justification: TTL/WTL watchpoint sets exist in-tree, yet put-transition ICs stay disabled for all useJSThreads runs

bytecode/RetiredJITArtifacts.cpp

• L53 __has_include gating on in-tree headers is always true; the leak arms and stub predicate behind it are dead scaffolding
• L211 Lazy makeGCAware promotion is unreachable in every configuration and the comments contradict each other

bytecode/SharedJITStubSet.h

• L162 find() turns a weak m_stubs entry into a strong ref; what guarantees the stub's zero-crossing cannot race it GIL-off?

bytecode/UnlinkedFunctionExecutable.cpp

• L96 Fifth local extern declaration of gilOffCompilationLock(); the keep-in-sync lists already disagree

bytecode/UnlinkedFunctionExecutable.h

• L143 Residual recordParse race rationale is wrong: the racy fields fit in the cell's tail padding

bytecode/UnlinkedMetadataTableInlines.h

• L139 TSAN-only zeroing in link() suppressed the metadata-buffer lifetime race and is now dead code

bytecode/Watchpoint.cpp

• L244 BUGHUNT instrumentation marked NOT FOR LANDING ships in the engine
• L381 GIL-on: a Class-A fire while a second VM (Worker) is entered aborts in the stub
• L654 fireEarlyForGILOff has no callers; the deferred-fire ORDERING comment claims callers use it

bytecode/Watchpoint.h

• L718 fireEarlyForGILOff and hasClassAFirePending have no callers; the in-code comments describe them as wired

bytecompiler/BytecodeGenerator.cpp

• L79 gilOffCompilationLock() declared out-of-header in six .cpp files with a copy-pasted locker

debugger/Debugger.cpp

• L602 GIL-off: deleteAllCode's whenIdle deferral and the !vm.entryScope assert key on a member GIL-off never writes

dfg/DFGAbstractValue.cpp

• L222 Zero-StructureID bail masks a cell freed while a live plan referenced it; the guard is also incomplete

dfg/DFGCommonData.cpp

• L113 Stale comment: the usePollingTraps forcing it calls 'deferred' has already landed

dfg/DFGJumpReplacement.cpp

• L55 Stale comment: M2b (usePollingTraps forced under useJSThreads) has already landed

dfg/DFGOSRExitCompilerCommon.cpp

• L49 DW-1 sort-comparator stash record is bring-up diagnostics left compiled in, and it is never disarmed on exits that do not return through the trampoline

dfg/DFGOSRExitCompilerCommon.h

• L45 Free-function loadVMLite redeclared in a public header with a comment that contradicts AssemblyHelpers.h

dfg/DFGPlan.cpp

• L106 Bring-up scaffolding: bare extern for gilOffCompilationLock() plus fifth verbatim copy of GILOffCompilationLocker, with stale keep-in-sync lists
• L645 KNOWN RESIDUAL comment records an open GIL-off race (compiler thread vs sibling mutator profile stores) only here, not in the integration record

dfg/DFGSpeculativeJIT.cpp

• L99 __has_include guard and #else fallback constants are dead scaffolding; the header is already included via JSObject.h
• L494 GIL-on: inline-allocated butterfly is installed with TID 0 while the C++ slow path tags it with the creator's TID
• L2411 validateDFGClobberize is disabled for every GIL-off compile; per-lite didEnterVM left as an open obligation
• L11713 __has_include fallback arms with hard-coded layout constants are dead bring-up scaffolding
• L12046 First-foreign-write exit in the segmented-aware PutByVal flat arm has no profile feedback
• L12307 useThreadedDFG kill switch exists but is consulted nowhere; the prescribed wiring would be unsound

dfg/DFGThunks.cpp

• L244 Stale comment: no Release fallback to the VM-block destination word exists any more

ftl/FTLJITCode.h

• L59 FTL::JITData::offsetOfDummyArrayProfile() has no caller

ftl/FTLLazySlowPath.cpp

• L87 GIL-off: no consumer-side instruction-stream sync for the data-published stub on arm64

ftl/FTLLowerDFGToB3.cpp

• L156 Dead __has_include fallback duplicates the butterfly tag encoding
• L6206 writeThreadLocal watchpoints are registered for read-only KnownNonArrayStorage plans, coupling reader code to the first foreign write
• L6432 E1 comment claims the owner compare routes segmented words slow; it does not in the !E2 arms
• L6457 useThreadedFTL kill switch exists in OptionsList.h but is read nowhere; comment says it has not landed
• L13617 useThreadedFTL kill switch exists in OptionsList.h but is never read
• L20102 Comment calls the butterfly mask a no-op, but the operation path already TID-tags the word
• L21525 Handler-IC GetById sites emit an unreachable slow-path call sequence
• L23893 Inline-allocation butterfly TID tag is gated on gilOff, but the C++ ctor stamps under useJSThreads (GIL-on mismatch)

ftl/FTLOSRExitCompiler.cpp

• L55 Stale 'no header owns this form yet' comment; loadVMLite is already declared by included headers

ftl/FTLSaveRestore.cpp

• L46 Stale bring-up comment and redundant free-function self-declaration of loadVMLite

ftl/FTLThunks.cpp

• L105 Baked scratch index space is monotonic, never freed, and hard-capped at 16384: what bounds it in a long-running gilOff process?

heap/BlockDirectory.cpp

• L145 findOwnEmptyBlockForRefill's stated population does not exist; it can only return weak-declined blocks, which it declines again
• L550 Mutator-concurrent sweep runs cell destructors while siblings run, but the same PR says siblings may still read dead-verdicted cells
• L761 Shared-mode assertion gating every lock-free m_bits read is weaker than its comment claims under per-directory stripes
• L787 Comments claim the IncrementalSweeper is disabled when shared; it was re-enabled, so assertSweeperIsSuspended's rationale is wrong

heap/CompleteSubspace.h

• L221 GCClient::CompleteSubspaceView is dead code with a live bind hook

heap/CompleteSubspaceInlines.h

• L76 Stale justification: growTable no longer stamps the TLS snapshot under the option

heap/GCThreadLocalCache.cpp

• L238 allocatorFor(BlockDirectory&), stopAllocating(), prepareForAllocation() and server() have no callers

heap/GCThreadLocalCache.h

• L137 GCCellLockDepth has no writers; the CG-I18 asserts it feeds are vacuous

heap/HandleSet.cpp

• L71 Process-global strong-lock side table is justified by a file-ownership constraint this PR no longer has

heap/HandleSet.h

• L129 m_strongLock does not exist; the lock lives in a process-global side table whose stated justification no longer applies

heap/Heap.cpp

• L150 Seam forward declarations duplicate a header this file already includes; the comment explaining why is stale
• L2300 Concurrent marking window is on by default under GIL-off with no flag
• L2723 Stale NOTE: worldIsStopped() header accessor is already an atomic load
• L2743 TEMPORARY O(blocks) diagnostic walk per stop window still compiled into release
• L4241 Deferred LambdaFinalizer is invoked with a nullptr cell; the contract change is undocumented at the public declaration and the sole-caller justification is inaccurate
• L5914 ISS flip can orphan a legacy needFinalizeBit; the deferred finalize() then runs after clients have allocated
• L6588 SharedGCWindowOpen::TicketDrainSuccessor is never constructed; the F28 comments describe a state the code never enters
• L6840 F28 TicketDrainSuccessor open is described as wired but has no caller
• L7271 Shared stop has no push for a non-entered access holder; Heap.h:1116-1120 still promises one
• L7507 Dead namespace-scope allocationClientForCurrentThread duplicates the Heap.h resolver and has already drifted
• L8492 Auto cap is a dead 'cap = 0'; the sibling-assist mechanism is off by default and the option text says otherwise

heap/Heap.h

• L80 Self-defined feature-test macro JSC_HEAP_HAS_STW_FORBIDDEN_SCOPE is a bring-up shim
• L750 allocationClientForJITCodegen is dead code kept by comment
• L769 STW-forbidden scope is described as debug-only but drives release behavior
• L999 Pin comment describes a superseded design (pin at retire, permanent flag-on leak, leaf lock)
• L1413 sharedGCWindowedStagesEnabled() is unreferenced; Heap.cpp keeps a divergent mirror
• L1813 Unused 1-byte Lock pad that changes no offset; the layout guards do not check what their comment claims
• L2573 bindCompleteSubspaceClients() and the five CompleteSubspaceView members are written but never read
• L2758 Stale rationale: the gilOffProcess latch now precedes the m_gilOff designation in VM.cpp
• L2838 allocationClientForJITCodegen is dead code (zero callers), as is the duplicate free function in Heap.cpp

heap/HeapClientSet.cpp

• L69 verifyStickySharedServerDesignation() call documented as mandatory before this noteSharedServerSticky() is missing

heap/IsoSubspace.h

• L155 Out-of-line justification for GCClient::IsoSubspace::tlcSlot is false

heap/IsoSubspaceInlines.h

• L45 GCClient::IsoSubspace::allocateForClient is dead code

heap/MarkedBlock.cpp

• L502 allocBit leg and section-13 last-era skip are unreachable from the only caller

heap/MarkedBlock.h

• L261 Hint safety argument cites MSPL exclusion that does not exist
• L661 isMarkedRaw comment contradicts its callers and their locking

heap/MarkedSpace.cpp

• L449 isPagedOut comment says the full-GC activity callback never fires once shared; it does

heap/MarkingConstraint.cpp

• L105 MarkingConstraint::m_lock is now dead but still declared

heap/SharedHeapTestHarness.cpp

• L1040 Harness is the only caller of the blocking JSThreadsStopScope ctor, so it exercises a GCL path production never takes
• L1262 run() installs FIXME-marked TEMPORARY GC diagnostics that persist for the process lifetime

interpreter/CLoopStack.cpp

• L121 Per-lite CLoopStack instances reserve an unused 5 MB segment and bump the global cache epoch on every VMLite teardown

interpreter/CachedCall.h

• L106 Foreign-skip is gated on the process byte but the owner-side absorb is gated on vm.gilOff()

interpreter/FrameTracers.h

• L223 Stale NOTE says prepareCallOperation is unconverted and this assert is expected to fire under gilOff
• L247 Pre-resolved tracer overloads and VM::group3Primitives(preResolved) have zero callers

jit/AssemblyHelpers.cpp

• L229 Hand-duplicated self-declaration of loadVMLite and macro-guarded member are bring-up scaffolding
• L261 GIL-off on Darwin fail-stops in the LLInt/JIT instead of being refused at option validation

jit/AssemblyHelpers.h

• L704 VM&-keyed copyLLIntBaselineCalleeSaves overload duplicates a now-dead function body in another TU

jit/CCallHelpers.cpp

• L43 Partial-tree __has_include scaffolding and unused includes left in CCallHelpers.cpp

jit/ConcurrentButterflyOperations.cpp

• L47 __has_include scaffolding and TID-0 shim are dead now that VMLite.h and ConcurrentButterfly.h are in-tree
• L293 Seven unreferenced JIT operations with RELEASE_ASSERT_NOT_REACHED bodies compiled into the engine

jit/GCAwareJITStubRoutine.cpp

• L353 Stateless shared stubs become GC-aware with the first CodeBlock as m_owner

jit/JITOpcodes.cpp

• L2270 Duplicated callee-save spooler body defined in the wrong TU; the original overload is now dead

jit/JITOperations.cpp

• L4972 operationReallocateButterflyAndTransition is unreachable flag-on; the useJSThreads Ref is dead and the body has no fail-stop if the Repatch gate is ever relaxed

jit/JITThunks.h

• L263 Lock-discipline comment contradicts ctiStubImpl, which holds m_lock across thunk generation

jit/JITWorklist.cpp

• L38 Unused <wtf/NeverDestroyed.h> include and history note left from the removed file-local finalizingKeys() set

llint/LLIntSlowPaths.cpp

• L136 useThreadedLLIntICs kill switch is declared but not wired; comment says it does not exist
• L2581 Bring-up tripwire (thread_local echo plus unconditional RELEASE_ASSERTs) left in the flag-off varargs path
• L3278 DW-1 instrumentation: release-build dataLogLn on a non-crash path, and the comment overstates what the RELEASE_ASSERT checks

llint/LowLevelInterpreter.asm

• L569 CLoop lite arms depend on the debugging-only cloopDo hook and are unreachable because Options refuses GIL-off on CLoop
• L636 GIL-off on Darwin crashes (RELEASE_ASSERT or a bare break) instead of being refused at option validation like CLoop and 32-bit
• L3339 virtualThunkFor comment still presents the slot recompare as the defense against a racing installCode; the JIT twin was corrected and the recompare is dead for script executables under GIL-off

llint/LowLevelInterpreter64.asm

• L1862 Stale comment: the Darwin TLS-key slot exists, yet every LLInt threaded write fast path off Linux jumps straight to the slow path
• L3059 Register-contract comment names the wrong ARM64 register for t6 and the wrong clobbering instruction

runtime/AtomicsObject.cpp

• L48 Cross-TU prototype of isArrayBufferDetachedGILOff with a stale justification; the detached-flag member it stands in for never landed

runtime/CodeCache.cpp

• L88 Fifth copy-pasted GILOffCompilationLocker plus another local extern of gilOffCompilationLock()

runtime/CommonSlowPaths.h

• L63 Comment claims the maxNumberOfFastIterationModes bound survives racing merges; it does not

runtime/ConcurrentButterfly.cpp

• L1587 segmentedTransition / structureOnlyTransition drivers and settledStructure have no callers
• L2261 getDirectConcurrent/putDirectConcurrent and the void transition drivers are exported dead code
• L2576 Mode (b) rarity comment assumes a VL rounding that is gated on useSharedGCHeap, not useJSThreads
• L2672 Mode (b) fragment liveness under GIL-off rests on Wlr retention, not on this DeferGC
• L2837 Dead availableOldLength computation and 17-line comment about a removed branch
• L4043 Ledger entry I25 cites a non-null-fragment RELEASE_ASSERT in validateConsistency that does not exist

runtime/ConcurrentButterfly.h

• L80 Dead __has_include shim would silently return TID 0 for every thread if it ever activated
• L133 SFINAE option probes are dead scaffolding; the options exist
• L667 Exported drivers segmentedTransition/structureOnlyTransition (and two spine accessors) have no callers
• L742 Stub witness is described as redundant and pre-M4 but is live, process-global evidence that bypasses the r33 non-conductor guard

runtime/DeferredWorkTimer.cpp

• L344 Stale comment: runRunLoop GIL-on/flag-off path is no longer lock-free

runtime/FunctionExecutable.h

• L145 codeBlockWithEntrypointFor and replaceCodeBlockWith are dead code

runtime/GetterSetter.h

• L67 Hand-expanded set() is redundant; the comment's claim about setEarlyValue is stale

runtime/JSArray.cpp

• L522 Flag-on branches in unshiftCountSlowCase are unreachable

runtime/JSCell.h

• L222 Comment claims only structure() clears the nuke bit; StructureID::decode/tryDecode always do, so the added decontaminate() is a no-op

runtime/JSCellInlines.h

• L295 32-line comment documents a withdrawn mechanism with no code in the tree

runtime/JSGenericTypedArrayViewPrototypeFunctions.h

• L716 Only the TSAN-reported arms are routed through relaxed lane accessors; the stated UB rationale applies to the rest of this file

runtime/JSGlobalObject.cpp

• L728 Disposition table rules import() and ShadowRealm refused on spawned threads, but no gate exists and nothing tracks the gap
• L1442 threadAsyncContextData per-lite path is dead code; GIL-off still shares one AsyncLocalStorage cursor across threads
• L2167 Stale rationale: the park-capable LazyProperty waiter has already landed

runtime/JSInternalFieldObjectImpl.h

• L80 atomicInternalField() has no callers; the two claim/publish host hooks open-code the cast

runtime/JSLock.cpp

• L432 VMEntryTokenRecord::spAtEntry is write-only
• L814 mayBlockSynchronously is dead code; the §G re-point it documents never landed
• L1518 Stale comment: per-lite queue is described as never created, but gilOff enqueues are already rerouted into it
• L1670 Stale comment: claims the GILDroppedSection GIL-off split has not landed and that any spawned park aborts here

runtime/JSLock.h

• L132 'Sole caller: GILDroppedSection' is stale; reacquireParkedCarrierAndServiceWatchdogCheck also calls unlockAllForThreadParking

runtime/JSMicrotask.cpp

• L626 publishAsyncGeneratorResume is dead code; the comment that justifies keeping it is wrong

runtime/JSObject.cpp

• L4727 Comment claims renumberPropertyOffsets does not bump concurrentEditCount; it does
• L7472 FIXME is half stale and records a still-unguarded flat-only butterfly deref in DFGOperations

runtime/JSObject.h

• L949 butterflyRegime() and isSharedArrayStorage() have no callers
• L1224 FIXME marks a known abort (i03-i37) whose identified root cause is only half fixed

runtime/JSPropertyNameEnumerator.h

• L142 Stale NOTE says the writer-side relaxed stores are still owed; they already exist in JSPropertyNameEnumerator.cpp

runtime/JSString.cpp

• L194 resolveRopeInternalNoSubstring is now dead code

runtime/KeyAtomStringCacheInlines.h

• L36 Unused debugging includes left in KeyAtomStringCacheInlines.h

runtime/LazyPropertyInlines.h

• L261 LazyProperty abandonment restore is unreachable; the comment promises re-run semantics the code cannot provide

runtime/Options.cpp

• L901 Shadow gilOffProcess latch comment is stale: the JSCConfig byte has landed, but this latch is still load-bearing

runtime/PropertyTable.cpp

• L84 Copy-constructor comment contradicts the code and the destructor's safety argument
• L200 seal()/freeze() premise is false; if it were true the plain setAttributes stores would break the writer protocol

runtime/PropertyTable.h

• L181 Stale comment: inline offsets are quarantined too, not fed straight to Reusable

runtime/RaceAmplifier.h

• L48 RaceAmplifier.h integration plan describes call sites that never landed

runtime/SamplingProfiler.h

• L378 WhileTargetSuspendedScope is never instantiated; header PENDING notes contradict the .cpp

runtime/ScriptExecutable.cpp

• L110 GILOffCompilationLocker and the lock/conductor declarations are copy-pasted across translation units
• L340 GlobalExecutable::replaceCodeBlockWith and FunctionExecutable::replaceCodeBlockWith are now dead code

runtime/Structure.cpp

• L2147 New DeferredWatchpointFire* parameter has no caller
• L2804 Comment says the clear runs without m_lock; flag-on clearCachedPrototypeChain takes it

runtime/Structure.h

• L868 transitionThreadLocalTIDOffset() has no JIT consumer; it exists so Heap.cpp can poke a private field by offset
• L1004 New DeferredWatchpointFire* parameter is never passed by any caller
• L1083 55-line perf investigation log embedded in the header, including an admitted unmeasured flag-off gate

runtime/StructureInlines.h

• L245 Comments claim WriteBarrier::set() is a plain store, but this PR already made setEarlyValue a relaxed atomic store

runtime/ThreadAtomics.cpp

• L84 g_threadAtomicsSlotCASRetries is never incremented; the dump always reports the hook as not wired
• L1675 Property waitAsync ticket is not a counted keepalive registration, contrary to the U-T9-INT1 gate list

runtime/ThreadAtomics.h

• L66 g_threadAtomicsSlotCASRetries is a never-incremented stub compiled into the engine

runtime/ThreadManager.cpp

• L311 ThreadManager::currentTID() and the VM-blind allocateSpawnedThreadState() overload have no callers
• L746 addThreadWaitDeadline has no callers, so the waitDeadlines expire/harvest machinery is dead and SD16 is not wired
• L1122 Stale reviewer NOTE: the 9.2-6 hooks are applied and thread-restrict.js is no longer skipped

runtime/ThreadManager.h

• L69 Stale note: the useThreads alias option was already removed
• L179 Documented keepalive decrement site (2) 'VM-shutdown cancelPendingWork' does not exist
• L795 addThreadWaitDeadline, ThreadWaitDeadline and ThreadState::waitDeadlines are dead code

runtime/ThreadObject.cpp

• L283 AB18-I diagnostic dataLogLn and speculative exception fold left in threadMain

runtime/VM.cpp

• L253 g_jscCurrentVMLite rationale block is stale and contradicts VMLite.h/VMLite.cpp and the LLInt
• L2333 Stale comment: JSPromise.cpp call sites already use the cross-thread-aware seam
• L3075 verifyStickySharedServerDesignation is a tautology at its only call site; the site it was written for is documented as unwired
• L3248 Lock-rank comment on the registry lock is wrong about the code under it

runtime/VM.h

• L379 Dead infrastructure: gilOffMultiMutator()/everHadSecondMutator() and the preResolved group3Primitives overload have no callers and are documented as unsafe to use
• L2028 Review-process transcript embedded in VM.h member block

runtime/VMEntryScope.cpp

• L369 Dead N-entry refusal walk keyed on a constexpr true; status log records an open GIL-off crash

runtime/VMLite.cpp

• L104 Darwin VMLite TLS mirror compiles away; GIL-off on macOS fail-stops at the first JIT emission
• L429 verifyVMLiteTLCAddressingChain checks a chain no emitter uses; the status block contradicts the tree
• L829 Lazy-init owner side table has no consumers; LazyPropertyInlines.h landed its own copy
• L922 lazyInit* owner side table is dead code; LazyPropertyInlines.h ships its own implementation
• L1264 WS1.2 'verified still in violation' list is half stale and its line references have drifted

runtime/VMLite.h

• L284 Dead per-lite fields: sizeOfLastScratchBuffer is retired and regExpAllocator has no users
• L481 Phase A and HARD GATE comments contradict the landed GIL-off routing

runtime/VMLiteInlines.h

• L30 File banner says these helpers are unit-test only; the same file says the opposite

runtime/VMLiteShared.cpp

• L64 Recursive StructureAllocationLocker acquisition hangs silently; the comment calls it a fail-stop
• L76 Stale N7 TODO and #if defined shim around the STW-forbidden scope calls
• L153 everHadSecondMutator scan has zero consumers and an acknowledged unlanded companion store

runtime/VMLiteShared.h

• L136 Header still documents VMLiteRegistry::lock as a leaf; the code nests several locks under it

runtime/VMManager.cpp

• L416 numberOfEnteredThreads is dead code
• L703 ParkingLot queue key is the WTF::Lock's own parking address
• L770 30s stop watchdog turns an unbracketed sibling native wait into a release-build abort; should the bound be option-gated and shared with the GC barrier?
• L1217 Nested notifyVMStop from a STW callback is now reachable (bit re-fired while Stopped) and trips the stopped<=active assert

runtime/VMTraps.cpp

• L734 Stale comment: DeferTermination no longer writes the VM-level instance's flags

runtime/VMTraps.h

• L575 Acceptance record in the header is stale and documents known-broken GIL-off behavior
• L762 JSThreadsSafepoint epoch API declared in VMTraps.h for task-ownership reasons

runtime/WaiterListManager.cpp

• L67 Park-site helpers redeclared as bare prototypes in five .cpp files instead of a header
• L174 SD6 recorded as incomplete and still is: D8 gate and 1a gate remain, spawned arm unreachable

runtime/Watchdog.h

• L61 Stale WIRING STATUS block claims the W1 park-site wiring is missing and GIL-off must not ship; the code has landed it

tools/JSDollarVM.cpp

• L4521 sharedHeapTest should be allowIfNotFuzz like the other crash-on-misuse $vm entries; harness still carries TEMPORARY diagnostic hooks

wasm/WasmMemory.cpp

• L393 GIL-off relocating-grow stop-the-world arm is unreachable and untested, yet claims to close CVE-AUDIT B4

WTF/Threading.h

• L391 Comment describes the old bit-field layout and the wrong racing parties

WTF/text/AtomStringTable.cpp

• L37 Placement comments claim SharedAtomStringTable.cpp is not compiled; it is
• L59 RELEASE_ASSERT at thread death for pre-initialize atomization: is the Bun startup audit closed?

WTF/text/SharedAtomStringTable.h

• L96 Placement rationale is stale: SharedAtomStringTable.cpp is in the build

[robobun]

Minor: if (@gilOffProcess) is not folded at bytecode generation; flag-off builtins execute an extra jfalse per branch in LLInt and Baseline

The comment above says @gilOffProcess is a bytecode-time constant so every tier folds the branch, and AsyncGeneratorPrototype.js:117-119 says flag-off bytecode is the landed inline path verbatim. Neither holds below the DFG. @gilOffProcess is a BytecodeIntrinsicNode of Type::Constant (parser/ASTBuilder.h:207-209) whose emitter is emitLoad of jsBoolean(false) (bytecompiler/NodesCodegen.cpp:2463-2470); BytecodeIntrinsicNode has no emitBytecodeInConditionContext override (parser/Nodes.h:1077-1105, only ConstantNode folds at NodesCodegen.cpp:109-129), so IfElseNode::emitBytecode (NodesCodegen.cpp:4575-4590) falls into ExpressionNode::emitBytecodeInConditionContext (NodesCodegen.cpp:87-94) and emits op_jfalse on the constant register, and emitJumpIfFalse (BytecodeGenerator.cpp:1607-1655) has no constant fold, only compare-fusion peepholes. LLInt executes that jfalse through loadConstantOrVariable plus a dispatch (llint/LowLevelInterpreter64.asm:2646-2666) and Baseline materializes the constant and tests it (jit/JITOpcodes.cpp:391-420); only DFG CFA plus CFGSimplification remove it (dfg/DFGCFGSimplificationPhase.cpp:93-118). With useJSThreads off, Generator.prototype.next/return/throw and IteratorHelper.prototype.next/return each execute three extra jfalse per call (two in the builtin, one in generatorResume at line 36), AsyncGenerator next/return/throw one each, and the dead try arm plus its exception handler entry are materialized in each of these UnlinkedCodeBlocks. SPEC-ungil.md:90-93 does permit a flag-off delta for the N.5 twin intrinsics, so this is not a violation of the byte-identical rule, but it is a larger delta than that carve-out describes and the comment misstates what happens. Either give BytecodeIntrinsicNode an emitBytecodeInConditionContext that folds Type::Constant entries the way ConstantNode does and let IfElseNode skip the dead arm when the condition is a known constant, or reword both comments to say only the optimizing tiers fold the branch.

[robobun]

Minor: m_mode.store() defaults to seq_cst, adding a full barrier flag-off per call op at CodeBlock creation and per link transition

m_mode was a plain bit-field and is now Atomic<uint8_t>; every write uses store() with WTF's default std::memory_order_seq_cst (wtf/Atomics.h:67), which compiles to xchg on x86-64 and stlrb on ARM64 regardless of the flag. This line runs once per call op for every CodeBlock created (DataOnlyCallLinkInfo::initialize from finishCreation, CodeBlock.cpp:497); lines 355, 566, 621 and 690 run on every link transition. All readers are relaxed (mode(), CallLinkInfo.h:452) and the header's own rationale (CallLinkInfo.h:515-518) says mode readers tolerate staleness because publishRecord's exchange carries the ordering, so storeRelaxed() is equivalent flag-on and restores the plain byte move flag-off. The m_flags RMWs (CallLinkInfo.h:294-329) also default to seq_cst although the header calls them a relaxed-atomic flag byte; passing std::memory_order_relaxed only helps ARM64 there (an x86-64 fetch_or is lock-prefixed at any order) and setSeen runs once per call site, so the five m_mode stores are the part worth changing.

[robobun]

Minor: First Polymorphic transition: ARM64 GIL-off reader can pair the new always-call record with a stale null m_stub

The comment at lines 643-644 says a racing reader observes the old or the new routine, both alive. On a CallLinkInfo's first Init/Monomorphic -> Polymorphic transition the old m_stub is null (clearStub never nulls it afterwards flag-on, lines 260-261). The polymorphic thunk is reached only through the always-call record published at line 684, then loads CallLinkInfo::m_stub through the CLI pointer and indexes into it unconditionally (ThunkGenerators.cpp:390-391, LowLevelInterpreter.asm:3391-3392). The fast path's m_record load (emitFastPathImpl, LowLevelInterpreter64.asm:3049) and the thunk's m_stub load are independent relaxed loads with no address dependency, so an ARM64 GIL-off reader can see the new record with the pre-store null and fault in the thunk. x86-64 TSO and GIL-on are unaffected, and CallLinkInfo.h:274-277 records the load pair as the IT-8 residual, but with a null old stub the outcome is a crash, not a stale dispatch. An acquire on the m_stub load does not close it (it orders later accesses, not the earlier record load); carry the stub pointer in the CallLinkRecord so the thunk reads it through r, or make the fast path's m_record load acquire flag-on.

[robobun]

Minor: Size claim is false: the field split adds +8B on top of m_record, +16B per CallLinkInfo flag-off

The claim is false on the Itanium ABI. In the merge-base layout the bit-field word sat in CallLinkInfoBase's tail padding (bytes 17-18), so m_maxArgumentCountIncludingThisForVarargs was at 19, m_slowPathCount at 20, m_codeBlock at 24 and sizeof(CallLinkInfo) was 80. The new layout needs four separate bytes (m_flags, m_callTypeAndType, m_mode, m_maxArgumentCountIncludingThisForVarargs) at 17-20, which pushes m_slowPathCount to 24 and m_codeBlock to 32; with m_record appended, sizeof is 96 (clang and gcc agree on a field-for-field mockup for x86_64 and arm64 Linux and macOS; the old layout plus m_record alone is 88). So the repack costs +8 bytes on top of the +8 for m_record, +16 per DataOnlyCallLinkInfo in the 12 call-family metadata structs and per OptimizingCallLinkInfo, paid with the flag off, and double the "+8B per call op" D7 budget at docs/threads/SPEC-jit.md:143. Either fold a byte back (carry the write-once Type bit in m_flags, or pack mode with callType behind the same atomic RMWs) to return to 88, or correct this comment and the D7 budget. MSVC does not reuse base tail padding, so Windows is size-neutral either way.

[robobun]

Minor: computeFromLLInt reads LLInt cache structureIDs with plain loads while flag-on writers use unlocked relaxed atomic stores

Flag-on, these caches are published and cleared with relaxed 64-bit atomic stores and no m_lock (LLIntCachedIdAndOffset::clear/setConcurrently, GetByIdMetadata.h:65-74, called from slow_path_try_get_by_id, slow_path_get_by_id_direct and CodeBlock::finalizeLLIntInlineCaches). This read and the one at line 95 are plain 32-bit loads of the same word on a DFG compiler thread; the ConcurrentJSLocker in computeFor (line 192) does not exclude those writers, and useJSThreads forces useConcurrentJIT on (Options.cpp:1013). Mixed atomic and non-atomic access is a data race TSAN reports; the PR's own TSAN corpus caught this exact pair for PutByStatus::computeFromLLInt (Tools/threads/tsan/families-r0.txt:1325) and it was fixed with relaxed WTF::atomicLoad (PutByStatus.cpp:86-94), as were performLLIntGetByID and JITPropertyAccess.cpp:614. Soundness is unaffected (offset recomputed via getConcurrently, stale id tolerated) and flag-off is not a regression since the base already cleared these fields outside the lock, so this is a TSAN-gate and consistency leftover. Use WTF::atomicLoad(..., relaxed) here, at line 95, and for the mode/defaultMode.structureID reads at lines 69-71, 81-83, 113-115, 123-131, 141-149, which race clearToDefaultModeWithoutCache/setDefaultModeCacheConcurrently/setArrayLengthMode the same way. INTEGRATE-jit.md:1659 claims these reads are single aligned u64 loads; they are not.

[robobun]

Minor: GIL-off: VM::notifyNeedTermination() bypasses fireTrapVMWide, so the carrier shield can swallow it and parked threads never see it

The never-swallowed guarantee covers only raises that go through fireTrapVMWide. VM::notifyNeedTermination() (VM.h:1794), the embedder termination API (WorkerOrWorkletScriptController.cpp:146, WorkletGlobalScope.cpp:107, Bun's JSC__VM__notifyNeedTermination), still calls plain fireTrap (VMTraps.h:256), which ORs the VM word only and neither fans into per-lite words nor clears m_carrierTookSharedTermination. GIL-off, with the shield up (a carrier consumed a VM-wide termination while siblings were entered, VMTraps.cpp:893-895 and :908, then its host cleared and re-entered), a fresh host raise is a no-op OR on the already-set bit: handleTraps masks it on the carrier while any sibling is entered (VMTraps.cpp:793-794) and retires it with nothing thrown once they exit (VMTraps.cpp:795-799). Also GIL-off, park sites poll only their own per-lite word plus hasTerminationRequest (VMTraps.cpp:1348-1368; join, Lock.hold, Condition.wait, Atomics.wait), and fireTrap sets neither, so a host termination of a VM whose threads are all parked waits for some mutator to run and fan it out. Route notifyNeedTermination through fireTrapVMWide, as requestStop already does (VM.h:1808).

[robobun]

Minor: GIL-off notify settles through a raw VM after dropping list->lock; the VM can be destroyed in the window*

pending.second is the raw VM* of a waiter dequeued under list->lock (:519-527), but this scheduleWorkSoon runs after the lock is dropped and nothing keeps that VM alive: Waiter::m_vm is a raw pointer (WaiterListManager.h:109) and TicketData holds no VM reference (DeferredWorkTimer.h:54-83). The base is safe only because the settle ran under list->lock (notifyWaiterImpl :553-569 via Waiter::scheduleWorkAndClear :594-602) and ~VM calls WaiterListManager::unregister(this) under the same lock (VM.cpp:1131) before stopRunningTasks (:1134): the waiter is either still listed and cancelled, or its settle has completed. In an isGILOffProcess embedding with two VMs sharing a SharedArrayBuffer (U0b allows this), a notifier on VM B's thread can dequeue VM A's async waiter, release the lock, and A's teardown can run to completion in the window; unregister no longer finds the waiter and this line reads deferredWorkTimer from freed memory. timeoutAsyncWaiter's arm (:482) is safe because its timer runs on the waiter VM's run loop. Keep the settle under list->lock for waiters whose VM is not the caller's (the rank argument only concerns same-VM spawned settles), or make unregister(VM*) wait out in-flight settles.

[robobun]

Minor: ParkedCarrier verdict skips the CPU re-arm that annex W W1 says must be identical to an entered carrier; header wiring block is stale

SPEC-ungil-history.md:2631 (annex W W1, marked BINDING) and UNGIL-HANDOUT.md:442 say a parked carrier services shouldTerminate with callback semantics and CPU re-arm identical to an entered carrier; Watchdog.h:52 repeats it. This arm deliberately does the opposite for CallerState::ParkedCarrier: it skips the CPU-budget re-arm and clears m_cpuDeadline (:135-143), and the comment explains why the spec'd variant livelocks (a parked carrier accrues no CPU). The code is right; the normative text is not. Separately, the WIRING STATUS block at Watchdog.h:61-73 says the park sites do NOT yet call serviceCheckFromReacquiredParkedCarrier and that GIL-off must not ship until they split NeedWatchdogCheck out of the park predicate, but all five sites now drive it through reacquireParkedCarrierAndServiceWatchdogCheck (WaiterListManager.cpp:251, ThreadObject.cpp:543, LockObject.cpp:1002, ConditionObject.cpp:185 and :256, ThreadAtomics.cpp:1560) and the split predicate exists (VMTraps.cpp:1348-1371). Amend annex W W1 (SPEC-ungil §A.2.8 item 8) and the handout to the wall-clock-authoritative verdict, and delete or rewrite the stale WIRING STATUS block.

[robobun]

Minor: Flag-off watchdog entry/exit now take m_lock; the header's byte-identical wording is inaccurate

With useJSThreads off, exitedVM (Watchdog.cpp:299) and the hasTimeLimit arm of enteredVM (:279) now take m_lock, as do setTimeLimit (:81) and shouldTerminate (:98, :166); in the base (wk-base Watchdog.cpp:115-127) m_lock only guarded m_vm for the timer callback. The cost is negligible rather than a measurable regression: one uncontended Lock CAS pair per top-level VMEntryScope, only when an embedder created a Watchdog (Bun never calls ensureWatchdog; the entry path already pays a CPUTime::forCurrentThread syscall in startTimer), and isActive() is reached only from the NeedWatchdogCheck trap (VMTraps.cpp:973). The JIT-output claim is untouched. What is inaccurate is the wording: Watchdog.h:95 says every path is byte-identical flag-off, while Watchdog.h:161 concedes the GIL-on lock take as behavior-neutral; the comment at :48 refers to the asserts and is accurate. Either gate the Locker on m_gilOff (startTimer/stopTimer carry WTF_REQUIRES_LOCK, so that needs assertIsHeld or a split) or reword the header to behavior-identical.

[robobun]

Minor: GIL-off exitedVM decrements an unsigned depth without a guard; wraps if the watchdog was created while entered

m_carrierEnteredDepth is unsigned and the decrement at :307 is unguarded. A carrier whose top-level VMEntryScope was already live when the watchdog was created (JSContextGroupSetExecutionTimeLimit from a host function, API/JSContextRef.cpp:96-106; the initLater lambda at VM.cpp:475-479 requests the sticky Watchdog service but never calls enteredVM) never incremented, yet tearDownSlow still runs exitedVM (VMEntryScope.cpp:443-444). In release builds the ASSERT at :306 is compiled out and the depth wraps to UINT_MAX: isActive() is permanently true, timerDidFire always takes the carrier branch at :350, and W3 never fires for spawned-only execution. This sequence already violates a pre-existing contract, though: the base's exitedVM asserted m_hasEnteredVM (wk-base Watchdog.cpp:124), so mid-entry creation trips a debug assertion there as well, and no caller in tree does it. The difference is that the base's bool self-heals in release while the GIL-off counter does not, and GIL-off is development-only. A one-line if (m_carrierEnteredDepth) guard, or having the initLater lambda call enteredVM() when the creating thread is already entered, closes it.

[robobun]

Part 2 addendum: these 9 inline comments belong to part 2 (#249 (review)) and did not attach when that review was submitted. Same set, same caveats.

[robobun]

Minor: GIL-on: watchdog CPU budget armed on one thread and checked on another is the landed multi-thread JSLock behavior, now reachable via new Thread

The mechanism is real: GIL-on, a spawned Thread holds the JSLock and services NeedWatchdogCheck (VMTraps.cpp:947-973; the carrier-only mask at :726-733 is gilOff-only), so shouldTerminate compares its own CPUTime::forCurrentThread() (Watchdog.cpp:129-133) against an m_cpuDeadline armed from the main thread's clock (Watchdog.cpp:324), and a spinning thread can be granted up to the arming thread's accumulated CPU time. But this is not new to the PR. The GIL-on path is the unchanged landed code (base Watchdog.cpp:55-108 and 129-155), and any embedder that alternates OS threads on one JSLock (DropAllLocks on thread A, JS on thread B under a nested VMEntryScope, a supported JSC API pattern) has had the same cross-thread clock comparison since the CPU budget was added. SPEC-ungil annex W and SD14 state 'GIL-on (and flag-off) unchanged' deliberately, and Bun never arms the watchdog. new Thread does make the scenario reachable from script in the shipping GIL-on shape, so either note the limitation in the GIL-on paragraph here, or record the arming thread in startTimer and take the wall-clock-authoritative arm (the existing CallerState::ParkedCarrier path) when the servicing thread differs.

[robobun]

Minor: Relaxed-atomic WriteBarrier slot accessors defeat memset in flag-off fill loops (growth slow paths; new Array(n) unaffected)

The codegen claim is correct: a relaxed store is monotonic in LLVM, which LoopIdiomRecognize and the loop vectorizer both reject. Compiling the equivalent slot with clang 17 -O2, the plain clear() loop becomes a memset tail call and the relaxed one stays a scalar movq loop, about 3.5x slower for 64 to 4096 slots and equal once memory-bound. So the 'codegen-identical to plain accesses' comment here is false for loops. The blast radius is smaller than it looks, though. new Array(n) is unaffected: JSArray::tryCreate (JSArrayInlines.h:66) fills through Butterfly::clearRange, an explicit memset on JSVALUE64 (ButterflyInlines.h:347-348). JSArray.cpp:168-169 and 189-190 clear only the slack between initialLength and the size-class-rounded vectorLength, and eagerlyInitializeButterfly (216-217) runs only for Array subclasses (JSArray.cpp:3127-3128). What remains are growth slow paths (ensureLengthSlow JSObject.cpp:7033-7034, JSArray.cpp:502-509 and 2603-2607, ClonedArguments.cpp:97, DirectArguments.cpp:81) whose fill is amortized O(1) per element beside a realloc and memcpy. Cheap fix: route those unbounded-count loops through Butterfly::clearRange or clearArray (ArrayConventions.h:126-136), which already memset flag-off, and correct the comment.

[robobun]

Minor: GIL-off inspection reads lite->primitives directly, which is not the live Group-3 storage for a GIL-on (loser) VM in a gilOff process

In a gilOff process a VM that lost the designation race keeps m_gilOff == 0 (VM.cpp:421-447) and JSLock installs m_mainVMLite as its current lite (JSLock.cpp:1278-1285). For that VM group3Primitives() (VM.h:746-754) selects the VM block because gilOffWithProcessGate() returns m_gilOff, and every writer of m_stackPointerAtVMEntry, m_stackLimit and topCallFrame goes through it (VM.cpp:1793, VM.cpp:1883-1891, FrameTracers.h:147-261), so m_mainVMLite->primitives is never written and stays zero. This branch keys on the process-level predicate and reads lite->primitives directly, so stackBottom and stackTop are null, the range test fails, and vmForCallFrame returns null for every frame of that VM; $vm.dumpRegisters (JSDollarVM.cpp:2884) then prints "Cannot find callFrame on any VM stack." The header note at lines 59-62 calls the process-level key merely conservative, but for a GIL-on VM it refuses everything. Second VMs are reachable in this shape ($.agent.start runs runJSC with a fresh VM, jsc.cpp:4438; embedder Workers). Read lite->vm->group3Primitives() here and at line 430 instead; it selects the right storage in both modes.

[robobun]

Minor: Spawned-thread wasm gate calls ThreadManager::isJSThreadCurrent() without a useJSThreads pre-check on every cold JS->wasm call

ThreadManager::isJSThreadCurrent() is out-of-line (ThreadManager.cpp:327-331) and resolves threadStateSlot(): a std::call_once check plus a pthread_getspecific lookup (ThreadManager.cpp:257-270, WTF ThreadSpecific.h:130). The helper calls it unconditionally, so with useJSThreads off every callWebAssemblyFunction invocation (WebAssemblyFunction.cpp:76) pays a call plus TLS lookup that did not exist before. That entry is not always cold: it is the only JS->wasm path for virtual call sites, C++-originated calls, --useJIT=0 or forceICFailure, and signatures with v128/exnref (JSToWasm.cpp:548-566 returns no IC). With the flag off no spawned ThreadState can exist (ThreadState::create(tid, true) is reached only from ThreadObject.cpp:447, and the Thread global is installed only under useJSThreads, JSGlobalObject.cpp:2156) and vm.gilOff() is always false, so the whole body can be skipped. The sibling gate in jsCallICEntrypoint (WebAssemblyFunction.h:91-92) already uses the single Options byte test. Wrap the body in if (Options::useJSThreads()) [[unlikely]] so the flag-off path is one predicted-false byte test; the other 13 constructor/compile call sites are cold and unaffected either way.

[robobun]

Minor: Stack-check condition flipped from signed LessThanOrEqual to unsigned BelowOrEqual (here and at all nine branchPtrAgainstSoftStackLimit sites); flag-off emission differs and the comment misstates the landed form

The merge base emits branchPtr(LessThanOrEqual, AbsoluteAddress(softStackLimit), regT0) here (base YarrJIT.cpp:7281), a signed compare. The replacement passes BelowOrEqual, and branchPtrAgainstSoftStackLimit (AssemblyHelpers.h:185-204) forwards the condition unchanged on its gilOff()==false arm, so every JIT'd RegExp prologue now encodes jbe/b.ls instead of jle/b.le regardless of useJSThreads. The trailing justification, "unsigned, matching the landed AbsoluteAddress form", is wrong: all nine landed sites were signed (base JIT.cpp:781, DFGSpeculativeJIT.cpp:143, FTLLowerDFGToB3.cpp:260 and 14401, AssemblyHelpers.cpp:1879, SetupVarargsFrame.cpp:84, ThunkGenerators.cpp:1412 and 1576, LOLJIT.cpp:149 all use GreaterThan or LessThanOrEqual), and the helper comment at AssemblyHelpers.h:163-170 plus the VMEntryScope.cpp:180-182 note repeat the same false premise. For canonical user-space addresses signed and unsigned compares agree, so this is a violation of the flag-off emission invariant (SPEC-jit.md:180, I1: identical instruction sequences modulo field-offset immediates) rather than a behavior change. Either restore the signed conditions at all nine call sites or record the flip as an intentional I1 exception and fix the comments.

[robobun]

Minor: BitSet() relaxed-atomic zeroing is not TSAN-gated and the identical-codegen claim is false for multi-word sets

The identical-codegen claim is false for any multi-word set, and unlike concurrentCopyFrom in this same file (BitSet.h:449) the change is not gated on TSAN_ENABLED. With clang 17 -O2, escaping construction of BitSet<1024> (MarkedBlock::Header m_marks and m_newlyAllocated, IsoCellSet::addSlow's makeUnique at IsoCellSet.cpp:113) goes from 8 movups to 16 scalar movq, and the BitSet<32767> local at Dominators.h:346 goes from one memset to a 512-store loop ahead of the clearAll that follows. Relaxed atomic stores are also never dead-store-eliminated: the per-block local live in specializedSweep (MarkedBlockInlines.h:283) compiled to no zeroing stores before, because the copy that follows overwrites it, and now emits 16 dead stores on every block sweep with the flag off, in the same prologue the concurrentCopyFrom comment gates to keep unchanged. The TSAN rationale is weaker than stated: the cited IsoCellSet case publishes the set with a release store (IsoCellSet.cpp:115) that the TSAN build reads with an acquire load (IsoCellSetInlines.h:55), which already orders the constructor's plain stores. Use the same #if TSAN_ENABLED gate and keep the brace-initialized std::array otherwise.

[robobun]

Minor: SPEC-vmstate 4.8 (normative) states the opposite migration order from the code

docs/threads/SPEC-vmstate.md:230-236 (section 4.8, marked normative) says enableSharedAtomStringTable() MUST, after the latch, (1) migrate the initializing thread's table into the shards and (2) clear it. The code does migrate (lines 62-94), then latch (line 101), then clear (line 108), and this comment cites section 4.8 as the authority for that order. SharedAtomStringTable.h:59-61 and INTEGRATE-vmstate.md:33 also describe migrate-then-latch, and SPEC-vmstate-history.md never records the reorder, so the spec text is the stale side. The code order is the safer one: with latch-first, a final deref of a not-yet-migrated atom would take derefSharedZero -> removeDeadAtom, shard-miss and destroy the string while the per-thread table still points at it, and the migration loop would then read freed memory. Update section 4.8 to migrate, latch, clear so a reader checking the code against the spec does not conclude one of them is wrong.

[robobun]

Minor: cost() does a lock-prefixed RMW on every fresh JSString with the flag off

This fetch_or runs with useJSThreads and useSharedAtomStringTable both off, and cost() sits on every JSString::create(vm, Ref&&) (JSString.h:229, 239) and every rope resolution (JSString.cpp:339, 363), so each fresh StringImpl that becomes a JS value pays a lock or on x86-64 (an LSE atomic on arm64) where the base (StringImpl.h:1137) did load/or/store. The same pattern is in setIsAtom() (1295, 1297) and setHash() (1319). SPEC-vmstate R3(a) accepts this only as a bench-gated delta, but the bench gate (Tools/threads/bench-gate.sh, JSTests/threads/bench/) is eight property/array/transition loops with no string-allocating benchmark, so the carve-out has never been measured for this site, and the history's justification (SPEC-vmstate-history.md:499-503) treats fetch_or as a setIsAtom slow-path cost, which cost() is not. deref() already branches on the latch to keep the legacy path verbatim (StringImpl.h:1376). Do the same here: hashAndFlags() was just loaded, so when g_sharedAtomStringTableEnabled is false do m_hashAndFlags.store(flags | s_hashFlagDidReportCost, relaxed), keeping fetch_or for shared mode where racing flag publication is real. Or add a string-allocation benchmark to the gate and record the number.

[robobun]

Minor: Symbol hash counter mutated both under this lock and with no lock (GIL-off data race)

RegisteredSymbolImpl::create runs under s_symbolRegistryLock here, and its SymbolImpl constructor calls SymbolImpl::nextHashForSymbol() (SymbolImpl.h:93, 102, 111), which does s_nextHashForSymbol += 1 << s_flagCount on a plain static unsigned (SymbolImpl.cpp:39-42). The same counter is bumped with no lock by every Symbol() call (SymbolConstructor.cpp:84 -> Symbol.cpp:40 -> createNullSymbol) and by PrivateName (PrivateName.h:42, 48), so the registry lock covers only one of the writers. Under GIL-off two JS threads creating symbols race on it: a C++ data race that TSAN will report (nothing in Tools/tsan/suppressions.txt covers SymbolImpl), and a lost increment gives two live SymbolImpls the same m_hashForSymbolShiftedWithFlagCount, so they collide in every PropertyTable and symbolImplToSymbolMap bucket for their lifetime. Pointer identity keeps this a perf degradation rather than type confusion, and the race already exists in the base when two Worker VMs create symbols concurrently, but this PR makes it reachable inside one VM from plain JS while Symbol.cpp:68-79 fixed the neighbouring symbolImplToSymbolMap race. Make the counter a std::atomic with fetch_add(relaxed) in SymbolImpl.cpp; symbol creation is cold enough that the flag-off delta is negligible.