fix(runtime-transpiler): don't read this after publishing TranspilerJob to main thread#29128
Conversation
…rJob to main thread
|
Updated 1:39 PM PT - Apr 10th, 2026
❌ @Jarred-Sumner, your commit d86130f has 2 failures in
🧪 To try this PR locally: bunx bun-pr 29128That installs a local version of the PR into your bun-29128 --bun |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
WalkthroughCapture Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
There was a problem hiding this comment.
LGTM — the local snapshot of vm before queue.push(this) correctly closes the use-after-free window.
Extended reasoning...
Overview
Single-function change in dispatchToMainThread() (3 lines): introduces a local vm variable snapshotted from this.vm before publishing the job to the queue, then uses that local for both the queue push and the subsequent enqueueTaskConcurrent call.
Security risks
None. This is a pure correctness fix with no auth, crypto, or permission surface.
Level of scrutiny
Low. The race condition is well-described in the PR body, the fix is the canonical solution (snapshot before publish), and no other call sites or data flows are affected. The diff is 3 lines with no ambiguity.
Other factors
No bugs found by the automated system. No prior reviews on this PR. The PR description provides a thorough analysis of the crash (ReleaseSafe 0xAA poison, reloaded this.vm after xchg, preemption window) and links two crash issue reports. The fix exactly addresses the root cause with minimal blast radius.
There was a problem hiding this comment.
LGTM — minimal, correct fix for a well-analyzed use-after-publish race.
Extended reasoning...
Overview
The PR modifies a single 4-line function in . The change snapshots and into locals before calling , and never touches afterwards.
Security risks
None. This is a memory-safety fix with no security surface exposure.
Level of scrutiny
Low. The change is mechanically simple: snapshot pointers before publishing the object, avoid dereferencing the published object. The root cause analysis in the PR description is thorough and accurate — makes the job visible to the main thread, which can recycle the slot (writing 0xAA in ReleaseSafe), and the original code then reloaded from poisoned memory. The fix is minimal and correctly addresses the race.
Other factors
No bugs found by automated analysis. No existing reviewer comments outstanding. The fix is consistent with standard use-after-publish idioms and the added comment clearly documents the invariant.
What does this PR do?
dispatchToMainThread()runs on a WorkPool thread and does:After
queue.push(this)publishes the job, the main thread can be woken by aConcurrentTaskenqueued by a different worker, callRuntimeTranspilerStore.runFromJSThread()→queue.popBatch()(which drains all pushed jobs, including this one) →runFromJSThread()→store.put(this).HiveArray.put()writesvalue.* = undefinedacross the slot, so by the time this worker executes the second line and reloadsthis.vm, it reads0xAApoison andvm.eventLoop()dereferences a non-canonical pointer.In practice this only crashes on Windows x64 release builds: those are ReleaseSafe, so
= undefinedactually writes0xAA(ReleaseFast is a no-op), and the generated code reloadsthis.vmafter thexchginqueue.pushrather than CSE'ing it. The window between the two instructions is a handful of cycles, so it only fires when the worker is preempted right after the push on an oversubscribed machine.Crash reports show up as
Segmentation fault at address 0xFFFFFFFFFFFFFFFFwith framesVirtualMachine.zig:eventLoop/arena_allocator.zig:deinit/atomic.zig:fetchSub(the innermost inline at each PC; the outermost frames aredispatchToMainThread/TranspilerJob.run/ThreadPool.Thread.run).Fix: snapshot
vminto a local beforequeue.push(this)and never touchthisafter publishing.Fixes #15805
Fixes #14681