[Jarred-Sumner]

What

unsafe impl<T: ?Sized> Sync for RacyCell<T> made any payload shareable across threads while the single-thread contract lived only in comments. A static RacyCell reachable from a worker thread was one (already-required) unsafe deref away from a silent data race, and nothing checked the comment.

RacyCell<T> is now { owner: AtomicU64, value: UnsafeCell<T> }:

• Checked accessors (get / read / write): the first thread to call one claims the cell; any later call from a different thread panics — in release builds too. The unsafe impl Sync is now justified by this runtime check: safe code can never observe the payload from two threads.
• Unchecked accessors (unsafe fn get_unsync / read_unsync / write_unsync): for state whose synchronization is external and can't be expressed in the type system (write-once-before-thread-spawn, Once-published, mutex-guarded, address-only). Each call site documents its happens-before edge under the existing // SAFETY: comment.

Every RacyCell static in the tree was audited and classified (table below). The two layout-sensitive FFI statics get dedicated #[repr(transparent)] wrapper types because RacyCell is no longer repr(transparent).

Named cost

• One TLS read + one Relaxed atomic load + compare per checked access (plus a one-time CAS on the first access). The unchecked accessors are zero-cost.
• 8 bytes (AtomicU64) per RacyCell static.
• Cross-thread access to a thread-confined static is now a deterministic panic instead of UB. This is the intended behavior change.

Audit

Kept on the checked accessors (thread-confined; the comment is now enforced)

Static	File	Owning thread
TEMP_HOSTNAME, SOCKET_ASYNC_HTTP_ABORT_TRACKER, SHARED_REQUEST_HEADERS_BUF, SHARED_RESPONSE_HEADERS_BUF, SINGLE_PACKET_SMALL_BUFFER	http/lib.rs	HTTP client thread
CACHE	http/h3_client/AltSvc.rs	HTTP client thread
CUSTOM_SSL_CONTEXT_MAP	http/HTTPThread.rs	HTTP client thread
WTF8_ENV_BUF, ORIG_ENVIRON	sys/windows/env.rs	startup, write-only
FAILURE_REASON_DATA	install/windows-shim/bun_shim_impl.rs	bunx fast-path / standalone shim
ARENA (install_runner_arena)	install/lib.rs	CLI dispatch thread
CWD_BUF, ROOT_PACKAGE_JSON_PATH_BUF, ROOT_PACKAGE_JSON_PATH	install/PackageManager.rs	install main thread
HANDLES_BEFORE	resolver/fs.rs	startup
RUNNER (the .write() in test_command.rs / .read() in snapshot.rs)	runtime/test_runner/jest.rs	test-runner thread
HOME_DIR_BUF, URL_, APP_NAME_BUF, GITHUB_REPOSITORY_URL_BUF, NPM_REGISTRY_URL_BUF, THREAD	runtime/cli/create_command.rs	main CLI thread
ARENA (exec_arena)	runtime/cli/exec_command.rs	CLI dispatch thread
CLI_ARENA, CMD, LOG_, CONTEXT_DATA	runtime/cli/mod.rs	main CLI thread
WORKER_FRAME, WORKER_CMDS	runtime/cli/test/parallel/runner.rs	worker-process main thread
PREV_INT, PREV_TERM	runtime/cli/test/parallel/Coordinator.rs	coordinator main thread
SHELL_BUF, RUN, DIRECT_LAUNCH_BUFFER, ENVIRONMENT_BUFFER	runtime/cli/run_command.rs	main CLI thread
LIB_DIR_Z	runtime/ffi/mod.rs	(no readers or writers exist)
CHILD_SINGLETON	runtime/node/node_cluster_binding.rs	main VM thread (cluster-child IPC)
INSTANCE	io/ParentDeathWatchdog.rs	event-loop install path (ZST)

These need no call-site changes; the existing unsafe { *X.get() } shapes are unchanged and the "X-thread-only" comment is now a checked invariant.

Moved to the unsafe *_unsync accessors (genuinely cross-thread; left unsafe, listed per rule)

Static	File	Why it crosses threads / what synchronizes it
ARGV	bun_core/util.rs	written under a Once / during startup, read from any thread via argv()
STDOUT_STREAM, STDERR_STREAM	bun_core/output.rs	write-once at startup, read by every spawned thread in configure_thread
BUFFERED_STDIN	bun_core/output.rs	prompt() is exposed on Worker global scopes, so the shared stdin buffer is reachable from more than one thread (see findings)
SCOPED_FILE_WRITER	bun_core/output.rs	write-once at startup, read by debug logging on any thread
EMPTY_MASKS_DATA	collections/bit_set.rs	never written through; address taken from any thread
SELF_DEBUG_INFO	crash_handler/lib.rs	accessed by whichever thread crashes; serialized by the panicking latch
SIGALTSTACK	crash_handler/lib.rs	address-only; the kernel writes the bytes during signal delivery
HARDLINK_QUEUE	install/PackageInstall.rs	written on the install main thread, read by thread-pool workers (publication via the task queue)
SHARED_ENV	install/repository.rs	lazily written during enqueue, re-read from git task threads
THREAD_POOL	bundler/ThreadPool.rs	mutex + refcount guarded init/drop, address handed to multiple threads
EMPTY_OBJECT/ARRAY/STRING	parsers/json.rs	never written through; address taken from parser worker threads
CALIBRATION	perf/hw_timer.rs	Once-published, read by now_ns() on any thread
INSTANCE (FileSystem)	resolver/lib.rs	init at startup, read from resolver/bundler worker threads
MODULES, MAP	resolver/node_fallbacks.rs	Once-published, read from resolver threads
BIN_FOLDERS	resolver/resolver.rs	lock-guarded writes, flag-guarded reads from resolver threads
DATA (stdin tty)	io/source.rs	lock-guarded init; address compared from any thread; bytes owned by libuv
BUN_PATH_BUF	runtime/cli/create_command.rs	written on the spawned git thread and the main thread, sequenced by the join
TEMPDIR_FD	runtime/cli/upgrade_command.rs	open/close may run on different VM threads (test-only helper)
RUNNER (the reads in Jest::runner() / runner_ptr())	runtime/test_runner/jest.rs	reachable from non-test VM threads (see findings)

Replaced with a dedicated wrapper type (layout-sensitive FFI)

Static	File	New type
ValueUndefined, ValueTrue	jsc/FFI.rs	#[repr(transparent)] ImmutableJSValue — never mutated, no interior mutability at all; unsafe impl Sync over immutable POD
Bun__FFI__offsets extern static	runtime/ffi/ffi_body.rs	#[repr(transparent)] ForeignOffsets(UnsafeCell<Offsets>) — written once by C++ before any Rust read; every access is already unsafe

Left as-is

• install/windows-shim/main.rs has its own minimal RacyCell copy for the standalone (freestanding, ntdll-only, single-threaded) bun_shim_impl.exe build. There is no thread-id API available there and no second thread ever exists, so the Sync impl is never exercised across threads. Its read/write are now unsafe fn so every payload access requires unsafe; get() already only returns a raw pointer.

Multi-thread findings (pre-existing races surfaced by the audit)

1. Jest::RUNNER is reachable from Worker threads. console_on_before_print (invoked by every console.log) calls Jest::runner(), which returns &'static mut TestRunner derived from the main test thread's pointer. A Worker that logs during bun test gets an aliasing &mut to the test runner state on a different thread. The owner check caught this immediately in a smoke test (worker console.log panicked); the reads in runner()/runner_ptr() are now read_unsync to preserve current behavior, but the underlying cross-thread &mut TestRunner is a real bug that needs a JS-thread-affinity guard or a None fast-path for non-owner threads.
2. BIN_FOLDERS reads race its writes. bin_dirs() reads const_slice() after an Acquire flag load but without taking BIN_FOLDERS_LOCK, while append() mutates the array under the lock on another resolver thread. A reader can observe a torn length/contents pair.
3. BUFFERED_STDIN is reachable from Worker threads. prompt() is exposed on Worker global scopes and reads the single process-global 4 KiB buffered-stdin struct. prompt() on the main thread followed by prompt() in a Worker hands two threads a &mut to the same buffer (concurrent calls would interleave reads and corrupt the buffer state). The owner check caught this in a smoke test; buffered_stdin() is get_unsync to preserve the existing behavior.
4. THREAD_POOL refcount: the pre-existing PORT NOTE in acquire() already documents that a racing acquirer's reference is not counted (mirrored from Zig). Not introduced or fixed here.

Before/after unsafe count

rg -o -e 'unsafe \{' -e 'unsafe fn' -e 'unsafe extern' -e 'unsafe impl' -e 'unsafe trait' <touched files> | wc -l: 1048 → 1080.

The count goes up because the fix converts wrongly-safe surface into correctly-unsafe surface and adds checked machinery: 3 new unsafe fn accessors + their internal blocks, 2 new narrowly-scoped unsafe impl Sync wrappers for the FFI statics, the windows-shim read/write becoming unsafe fn, ~7 call sites that previously called the safe get() outside any unsafe block and now call unsafe { get_unsync() }, and the new #[test]s. The deliverable is not a lower token count — it is that no safe call path can reach an unsynchronized payload.

Tests / Miri

• New #[test]s in bun_core::util::racy_cell_tests cover same-thread access, cross-thread panic, the unsync escape hatch, and first-accessor-claims semantics. They type-check (cargo check -p bun_core --all-targets) but bun_core's test target does not link standalone (it depends on FFI symbols supplied by higher-tier crates), and bun_core is not in the Miri allowlist for the same reason — so, like the existing atomic_cell.rs tests, they do not run in CI. Miri does not cover this crate.
• bun bd builds; smoke-tested bun run, Bun.serve + fetch, Worker + console.log from a worker, bun install, bun build, and bun test (with snapshots and an in-test Worker).
• bun bd test test/js/web/workers/worker.test.ts and test/js/web/fetch/fetch.test.ts show the same pass/fail set as the unmodified main build (the failures in both are pre-existing/network-dependent).
• bun run rust:check-all: 10/10 targets pass.

[coderabbitai]

Warning

Rate limit exceeded

@Jarred-Sumner has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 2 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cdfa7749-3606-4d0d-b4ac-1eafb113fb9b

📥 Commits

Reviewing files that changed from the base of the PR and between 5d8fb88 and 1c25170.

📒 Files selected for processing (2)

• src/bun_core/output.rs
• src/bun_core/util.rs

Walkthrough

This PR refactors RacyCell<T>, a synchronization primitive for single-threaded write-once patterns, to distinguish between checked and unsynchronized accessors. The core type now tracks ownership, provides panic-on-cross-thread for checked methods, and new *_unsync raw variants for callers that provide external synchronization. All call sites throughout the resolver, install, I/O, and CLI subsystems are updated to use the appropriate accessor variant with updated safety documentation.

Changes

RacyCell core refactoring and widespread call-site updates

Layer / File(s)	Summary
RacyCell reimplementation and tests src/bun_core/util.rs	RacyCell<T> now stores an atomic owner latch and UnsafeCell<T> payload; checked get/read/write claim ownership on first access and panic on cross-thread calls. New get_unsync/read_unsync/write_unsync bypass checks and rely on external synchronization. Unit tests cover same-thread access, panic-on-cross-thread, and unsync correctness with spawn happens-before.
Argv initialization via unsync src/bun_core/util.rs	ARGV view initialization and reads in argv_view_init/argv_view and set_argv switch to unsync accessors.
Output stream configuration via unsync src/bun_core/output.rs	STDOUT_STREAM/STDERR_STREAM initialization in configure_thread/configure_thread_no_js/set_init and SCOPED_FILE_WRITER initialization/retrieval switch to unsync accessors with refined write-once safety documentation.
ThreadPool refcounting via unsync src/bundler/ThreadPool.rs	THREAD_POOL cell accesses in acquire() fast path, mutex-protected initialization, and shutdown() teardown switch to unsync, relying on REF_COUNT/MUTEX protocol for publication.
FileSystem singleton via unsync src/resolver/lib.rs	FileSystem process-global in instance(), get(), and init_with_force() switches to unsync accessors, with INSTANCE_LOADED Release/Acquire providing publication guarantee to resolver worker threads.
Node fallbacks modules via unsync src/resolver/node_fallbacks.rs	MODULES and MAP initialization in init_modules() and map() switch to unsync, with Once providing happens-before for module table population and lookup.
Bin folders array via unsync src/resolver/resolver.rs	BIN_FOLDERS initialization and read/append operations in bin_dirs() and dir_info_uncached() switch to unsync, with BIN_FOLDERS_LOADED atomic guarding initialization across both code paths.
Install queue and shared env via unsync src/install/PackageInstall.rs, src/install/repository.rs	HARDLINK_QUEUE and SHARED_ENV initialization and reads switch to unsync, with ThreadPool::schedule and task-queue enqueue/dequeue providing publication edges.
Crash handler and miscellaneous cell accesses src/crash_handler/lib.rs, src/collections/bit_set.rs, src/io/source.rs	SELF_DEBUG_INFO, SIGALTSTACK, EMPTY_MASKS_DATA, and stdin DATA switch to unsync accessors with safety documentation updated for debug-only, kernel-write, and address-only-compare patterns respectively.
Hardware timer calibration via unsync src/perf/hw_timer.rs	CALIBRATION read in now_ns() and write in calibrate() switch to unsync, with CALIBRATE_ONCE.call_once() providing exclusive write and happens-before guarantee.
JSON parser empty data via unsync src/parsers/json.rs	Empty string/object/array data pointer reads switch to unsync with explicit safety assertions that addresses are never written through.
CLI path and temp-dir buffers via unsync src/runtime/cli/create_command.rs, src/runtime/cli/upgrade_command.rs	BUN_PATH_BUF in --open and GitHandler::run and TEMPDIR_FD in temp-dir tests switch to unsync, with cross-thread sequencing via git thread join and test harness.
Jest test runner access via unsync src/runtime/test_runner/jest.rs	RUNNER reads in runner() and runner_ptr() switch to unsync, documenting that VM thread callbacks may access the test runner from non-test threads.
New immutable wrapper types for FFI constants src/jsc/FFI.rs, src/runtime/ffi/ffi_body.rs	ImmutableJSValue wrapper replaces RacyCell for ValueUndefined/ValueTrue JSC stubs with direct EncodedJSValue initialization; ForeignOffsets wrapper replaces RacyCell for BUN_FFI_OFFSETS C++ interop with unsafe impl Sync.
Windows shim RacyCell reimplementation src/install/windows-shim/main.rs	Shim's local RacyCell copy updated from Cell<T> to UnsafeCell<T> with unsafe read/write methods, matching main codebase refactoring for single-threaded usage.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: replacing RacyCell's unconditional Sync impl with an owner-thread runtime check, which is the primary objective of this PR.
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.
Description check	✅ Passed	The PR description is comprehensive and well-structured, covering the what, why, how, and all necessary technical details.

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

---

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

[robobun]

^{Updated 5:10 AM PT - May 21st, 2026}

❌ @Jarred-Sumner, your commit 1c25170 has 1 failures in Build #56613 (All Failures):

• test/js/bun/http/serve-body-leak.test.ts - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 31178

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

bun-31178 --bun

[coderabbitai]

Actionable comments posted: 4

🤖 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 `@src/bun_core/util.rs`:
- Around line 2436-2448: The match in claim_slow incorrectly panics on
compare_exchange returning Err(me) for reentrant same-thread access; update the
handling of compare_exchange on self.owner to treat Err(owner) where owner == me
as a successful claim (i.e., return/continue) and only panic when owner != me.
Locate claim_slow and the call to self.owner.compare_exchange and change the
Err(owner) branch to check owner == me (allowing the reentrant path) otherwise
keep the existing panic with the same message.

In `@src/crash_handler/lib.rs`:
- Around line 336-340: The change replaces the previous checked accessor for
SELF_DEBUG_INFO with an unsafe get_unsync(), which reintroduces cross-thread
races for lazy init and mutation (notably SelfInfo::address_map) because
dump_stack_trace() and other non-crash paths can still reach this helper; revert
to using the original checked accessor (the safe/get_checked-style API used
previously for SELF_DEBUG_INFO) instead of get_unsync(), remove the unsafe block
around accessing the slot, and keep the deterministic failure semantics so
concurrent access and lazy init remain protected.

In `@src/resolver/resolver.rs`:
- Around line 6081-6084: The BIN_FOLDERS initialization is racy because callers
(e.g., dir_info_uncached() invoked from dir_info_for_resolution()) may not hold
RESOLVER_MUTEX; replace the manual check/write of BIN_FOLDERS when
!BIN_FOLDERS_LOADED with a proper single‑time guard: either use a dedicated
BIN_FOLDERS_LOCK (acquire before checking/writing BIN_FOLDERS and store
BIN_FOLDERS_LOADED under the lock) or use a std::sync::Once to run the one‑time
initializer that writes
(*BIN_FOLDERS.get_unsync()).write(BinFolderArray::default()) and sets the loaded
flag; apply the same change to the other occurrence around lines 6127–6133 so
both sites perform atomic one‑time initialization of BIN_FOLDERS instead of the
current racy pattern.
- Around line 4039-4043: bin_dirs() currently reads BIN_FOLDERS without
synchronization which races with append() writes; fix by acquiring
BIN_FOLDERS_LOCK inside bin_dirs() and produce an immutable snapshot while
holding the lock (e.g., clone the BoundedArray contents into a Vec or other
owned collection) and return that snapshot instead of returning a direct slice
into the unsynchronized BoundedArray; reference BIN_FOLDERS, BIN_FOLDERS_LOCK,
BIN_FOLDERS_LOADED, append(), and the bin_dirs() function when making the
change.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 80150ff2-f81c-4498-869c-bff83b083b23

📥 Commits

Reviewing files that changed from the base of the PR and between 0b20408 and 5d8fb88.

📒 Files selected for processing (19)

• src/bun_core/output.rs
• src/bun_core/util.rs
• src/bundler/ThreadPool.rs
• src/collections/bit_set.rs
• src/crash_handler/lib.rs
• src/install/PackageInstall.rs
• src/install/repository.rs
• src/install/windows-shim/main.rs
• src/io/source.rs
• src/jsc/FFI.rs
• src/parsers/json.rs
• src/perf/hw_timer.rs
• src/resolver/lib.rs
• src/resolver/node_fallbacks.rs
• src/resolver/resolver.rs
• src/runtime/cli/create_command.rs
• src/runtime/cli/upgrade_command.rs
• src/runtime/ffi/ffi_body.rs
• src/runtime/test_runner/jest.rs

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

bin_dirs() still reads BIN_FOLDERS without synchronization.

BIN_FOLDERS_LOADED only publishes the one-time default initialization. Later append() calls mutate the same BoundedArray under BIN_FOLDERS_LOCK, but this method returns a slice without taking that lock or reading from an immutable published snapshot. That leaves a real read/write race on the array contents and length.

🤖 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 `@src/resolver/resolver.rs` around lines 4039 - 4043, bin_dirs() currently
reads BIN_FOLDERS without synchronization which races with append() writes; fix
by acquiring BIN_FOLDERS_LOCK inside bin_dirs() and produce an immutable
snapshot while holding the lock (e.g., clone the BoundedArray contents into a
Vec or other owned collection) and return that snapshot instead of returning a
direct slice into the unsynchronized BoundedArray; reference BIN_FOLDERS,
BIN_FOLDERS_LOCK, BIN_FOLDERS_LOADED, append(), and the bin_dirs() function when
making the change.

[Jarred-Sumner]

This is the pre-existing race called out in the PR description under "Multi-thread findings" (#2): bin_dirs() reads const_slice() without taking BIN_FOLDERS_LOCK while append() mutates under the lock on another resolver thread. This PR is a mechanical audit/conversion of the RacyCell accessors and intentionally does not change the synchronization of any site — fixing this properly (lock + owned snapshot, or an append-only/RCU structure) changes behavior and allocation patterns and belongs in a follow-up. The race is not introduced by this change; get_unsync preserves the exact prior semantics.

[coderabbitai]

@Jarred-Sumner Got it—thanks for the clarification. I’ll treat this as an acknowledged, pre-existing race and not a change request for this PR. The switch to get_unsync in bin_dirs() at Lines 4039–4043 preserves the prior semantics as intended.

If you’d like, I can open a follow-up GitHub issue to track a proper fix (e.g., lock+owned snapshot or an append-only/RCU snapshot) and link back to this PR and thread. Want me to file that now?

---

✏️ Learnings added

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 31178
File: src/resolver/resolver.rs:4039-4043
Timestamp: 2026-05-21T10:02:46.380Z
Learning: Repo oven-sh/bun: In src/resolver/resolver.rs, bin_dirs()’s read of BIN_FOLDERS via const_slice() without BIN_FOLDERS_LOCK is a known, pre-existing race. For PR `#31178` (RacyCell refactor), maintainers prefer not to change synchronization; get_unsync is used to preserve prior semantics, and any fix (snapshot/RCU or similar) should be deferred to a follow-up issue/PR.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: robobun
Repo: oven-sh/bun PR: 30522
File: src/runtime/cli/publish_command.rs:914-916
Timestamp: 2026-05-15T11:58:16.766Z
Learning: In this repo’s Rust code, do not flag a “typed accessor missing” or similar violation when reading `npm_config_*` environment variables. For `npm_config_*` keys (e.g. `NPM_CONFIG_REGISTRY`, `NPM_CONFIG_PROVENANCE`), the accepted pattern is to read them via `bun_core::getenv_z` using both upper-case and lower-case forms combined with `.or_else(...)`. This dual `getenv_z` approach is intentional and consistent even when `src/bun_core/env_var.rs` does not provide typed accessors for specific `npm_config_*` variables (e.g., in `PackageManagerOptions::load` and `publish_command.rs`).

---

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 31176
File: src/io/uv_handle.rs:92-100
Timestamp: 2026-05-21T09:34:31.660Z
Learning: In the oven-sh/bun repo, review allocator-mismatch/safety warnings carefully around raw `Box` round-trips. `bun_core::heap::{into_raw, take, destroy, alloc}` are `#[inline(always)]` thin aliases with identical machine code to `Box::into_raw`/`Box::from_raw` and do not provide additional allocator-mismatch protection or ASAN-risk reduction.

Therefore, when a value is wrapped in a *typed owner* that owns both halves of the raw round-trip (e.g., similar to patterns like `UvHandle<H,T>`, `WorkPool::schedule_owned`, etc.), it is the correct idiomatic form to use `Box::leak` and then `Box::from_raw` (open-coded) inside that wrapper. In this case, do not flag `Box::leak`/`Box::from_raw` usage as an allocator-mismatch risk and do not suggest replacing it with `heap::into_raw`/`take`.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Guard the first BIN_FOLDERS initialization with BIN_FOLDERS_LOCK or a Once.

The safety comment here assumes callers hold RESOLVER_MUTEX, but Line 3571 calls dir_info_uncached() from dir_info_for_resolution() without taking that mutex. Two threads can therefore reach this !BIN_FOLDERS_LOADED branch concurrently and both write(BinFolderArray::default()) before either stores true.

Also applies to: 6127-6133

🤖 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 `@src/resolver/resolver.rs` around lines 6081 - 6084, The BIN_FOLDERS
initialization is racy because callers (e.g., dir_info_uncached() invoked from
dir_info_for_resolution()) may not hold RESOLVER_MUTEX; replace the manual
check/write of BIN_FOLDERS when !BIN_FOLDERS_LOADED with a proper single‑time
guard: either use a dedicated BIN_FOLDERS_LOCK (acquire before checking/writing
BIN_FOLDERS and store BIN_FOLDERS_LOADED under the lock) or use a
std::sync::Once to run the one‑time initializer that writes
(*BIN_FOLDERS.get_unsync()).write(BinFolderArray::default()) and sets the loaded
flag; apply the same change to the other occurrence around lines 6127–6133 so
both sites perform atomic one‑time initialization of BIN_FOLDERS instead of the
current racy pattern.

[Jarred-Sumner]

Same as the bin_dirs() thread: this is the pre-existing synchronization scheme (RESOLVER_MUTEX for first init per the existing SAFETY comments, BIN_FOLDERS_LOCK for appends, an Acquire/Release flag for readers), reported in the PR description as finding #2. This PR only converts the accessor from the implicitly-unsynchronized get() to the explicitly-unsafe get_unsync() without changing which locks are held where; restructuring the init into a Once is a behavioral follow-up.

[coderabbitai]

@Jarred-Sumner Got it — thanks for the clarification. I’ll treat this as pre-existing and out of scope for this PR; no change requested here.

If it helps, I can open a follow-up GitHub issue to track “harden BIN_FOLDERS first-init (Once or lock-guarded check/write), keep BIN_FOLDERS_LOCK for appends; readers via Acquire/Release,” and link it back to #31178 and this discussion. Want me to file that?

---

✏️ Learnings added

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 31178
File: src/resolver/resolver.rs:6081-6084
Timestamp: 2026-05-21T10:02:39.847Z
Learning: Repo oven-sh/bun: For BIN_FOLDERS in src/resolver/resolver.rs, the pre-existing synchronization scheme is: RESOLVER_MUTEX for the first initialization, BIN_FOLDERS_LOCK protecting subsequent appends, and BIN_FOLDERS_LOADED as an Acquire/Release flag for readers. When a PR only refactors access (e.g., RacyCell get() → get_unsync()), do not push a locking/Once restructure in the same PR; suggest it as a follow-up instead.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: robobun
Repo: oven-sh/bun PR: 30522
File: src/runtime/cli/publish_command.rs:914-916
Timestamp: 2026-05-15T11:58:16.766Z
Learning: In this repo’s Rust code, do not flag a “typed accessor missing” or similar violation when reading `npm_config_*` environment variables. For `npm_config_*` keys (e.g. `NPM_CONFIG_REGISTRY`, `NPM_CONFIG_PROVENANCE`), the accepted pattern is to read them via `bun_core::getenv_z` using both upper-case and lower-case forms combined with `.or_else(...)`. This dual `getenv_z` approach is intentional and consistent even when `src/bun_core/env_var.rs` does not provide typed accessors for specific `npm_config_*` variables (e.g., in `PackageManagerOptions::load` and `publish_command.rs`).

---

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 31176
File: src/io/uv_handle.rs:92-100
Timestamp: 2026-05-21T09:34:31.660Z
Learning: In the oven-sh/bun repo, review allocator-mismatch/safety warnings carefully around raw `Box` round-trips. `bun_core::heap::{into_raw, take, destroy, alloc}` are `#[inline(always)]` thin aliases with identical machine code to `Box::into_raw`/`Box::from_raw` and do not provide additional allocator-mismatch protection or ASAN-risk reduction.

Therefore, when a value is wrapped in a *typed owner* that owns both halves of the raw round-trip (e.g., similar to patterns like `UvHandle<H,T>`, `WorkPool::schedule_owned`, etc.), it is the correct idiomatic form to use `Box::leak` and then `Box::from_raw` (open-coded) inside that wrapper. In this case, do not flag `Box::leak`/`Box::from_raw` usage as an allocator-mismatch risk and do not suggest replacing it with `heap::into_raw`/`take`.

[claude]

I didn't find any bugs, but this redesigns a core unsafe impl Sync primitive and audits ~40 call sites across 19 files with per-site checked-vs-unsync safety judgments (plus an intentional UB→panic behavior change and two new FFI layout wrappers), so it warrants human eyes on the audit table and the *_unsync classifications.

Extended reasoning...

Overview

This PR replaces RacyCell<T>'s unconditional unsafe impl Sync with an owner-thread runtime latch: the first thread to call a checked accessor (get/read/write) claims the cell, and any later cross-thread access panics in release. New unsafe fn *_unsync accessors bypass the check for externally-synchronized state. Every RacyCell static in the tree (~40) is audited and classified as either thread-confined (checked) or cross-thread (unsync, with a documented happens-before edge). Two layout-sensitive FFI statics (ValueUndefined/ValueTrue in jsc/FFI.rs, Bun__FFI__offsets in ffi_body.rs) get dedicated #[repr(transparent)] wrappers because RacyCell is no longer repr(transparent). Touches 19 files across bundler, install, resolver, crash handler, FFI, test runner, CLI, and core output/util.

Security risks

No direct security surface (no auth, crypto, input parsing). The risk class is memory-safety/soundness: each *_unsync site asserts a happens-before edge in a SAFETY comment, and a misclassification preserves a pre-existing data race rather than introducing a new one. The two new unsafe impl Sync wrappers (ImmutableJSValue, ForeignOffsets) are narrowly scoped and well-justified (immutable POD / C++-written-once extern), but the FFI layout change deserves a second pair of eyes against the TinyCC/C++ consumers.

Level of scrutiny

High. This is a deliberate, well-documented hardening change, but: (1) it's an intentional behavior change — code that previously raced silently will now panic deterministically, which could surface in production paths the author didn't smoke-test; (2) the audit table embeds ~40 per-site judgments about thread affinity and synchronization edges, several of which (crash handler, Jest::RUNNER, BUFFERED_STDIN) the author explicitly flags as preserving known pre-existing races; (3) the new unit tests don't run in CI (bun_core test target doesn't link standalone) and Miri doesn't cover this crate; (4) the unsafe count goes up (1048→1080), which is the right direction for this change but means more surface to review.

Other factors

CodeRabbit raised four points; two were addressed (idempotent claim_slow arm, SELF_DEBUG_INFO rationale) and two (BIN_FOLDERS races) were correctly deferred as pre-existing and out of scope. The PR description is exceptionally thorough with a complete audit table. CI shows musl build-cpp failures on commit 00a1c6a — likely unrelated infra, but worth confirming the latest commit (1c25170) is green before merge.