blob: make Store::data_mut unsafe; make File.last_modified atomic

RO
2mo ago
+330-111

Problem

  • Store::data_mut(this: &RefPtr<Store>) -> &mut Data (src/jsc/webcore_types.rs) hands out a mutable borrow through a shared, clonable handle. Store is Send + Sync, so cloned handles on two threads can each mint &mut Data to the same heap allocation through a safe API. That is immediate undefined behavior (issue #30800).
  • The race is real today: do_read_file clones the RefPtr<Store> into each worker-pool ReadFile task, and those tasks wrote the plain u64 File::last_modified while the JS thread read it.

Fix

  • Store::data_mut is now an unsafe fn whose precondition is borrow exclusivity. Every call site discharges it in an unsafe {} block with a SAFETY note; read-only sites use shared &store.data borrows instead.
  • File::last_modified becomes AtomicU64 (Relaxed). That closes the observable cross-thread data race, and shared borrows on the worker pool stay sound.
  • The sibling back-doors that discharged the same caller-context claim are unsafe fn too: blob_store_mut/set_blob_content_type (Body.rs), BlobExt::shared_view_raw/set_is_ascii_flag, and the free resolve_file_stat (Blob.rs). FileReader::on_start clones the File instead of mutating the shared Store. CopyFileWindows::prepare_pathlike takes &PathOrFileDescriptor (it only reads).
  • Verified: test/internal/source-lints/store-data-mut-unsafe.test.ts (fails 0/2 on main's source) and a 128-task concurrency test in test/js/web/fetch/blob-write.test.ts; blob.test.ts and body.test.ts green under bun bd (ASAN), 595 tests.

Background

  • A Store is the refcounted backing allocation of a Blob (bytes, a file, or an S3 reference). Handles are bun_ptr::RefPtr<Store>; clones share one allocation.
  • Rust forbids two live &mut to the same memory, even briefly, on one thread or two. An API that lets safe code create them is unsound regardless of whether a crash is observed.
  • unsafe fn moves the proof obligation to the caller: each site must state why no aliasing borrow can be live.
Notes

Earlier revisions of this PR targeted the pre-#40478 shape of this code, where the handle was a bespoke StoreRef with its own unsafe impl Sync; that revision dropped Sync, added a compile-time trip-wire, and went through full bot review (all threads resolved). #40478 replaced StoreRef with RefPtr<Store> and moved Send/Sync onto Store, which worker-pool clones genuinely require, so the Sync-drop and trip-wire no longer apply; the current revision re-expresses the surviving substance (unsafe data_mut, atomic last_modified, the sibling unsafe fns, the FileReader clone) against the new architecture. The source-lint guard moved to test/internal/source-lints/ accordingly.

Main-side issues found while maintaining this PR, both reported: the asan-lane Fd::close() EBADF double-close crash (#38630), and a debug-only Bun.file(path).slice(a, b) stream over-read (fixed on main since; blob.test.ts is fully green on this branch).

The remaining narrow overlap: resolve_file_stat's JS-thread &mut File can race worker-pool &File reads of the non-atomic seekable/mode fields. Every writer stores the same fstat-derived values (idempotent); closing it fully means interior-mutable File fields, a follow-up beyond #30800.

[review] gate passed · iteration 47 · 9 files touched

fails on main (without fix) 
ASAN without fix: 2 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/internal/source-lints/store-data-mut-unsafe.test.ts test/js/web/fetch/blob-write.test.ts
bun test v1.4.1 (861e9ae04)

test/internal/source-lints/store-data-mut-unsafe.test.ts:
20 | 
21 | test("Store::data_mut is an unsafe fn", () => {
22 |   // The precondition-bearing signature: every call site must assert, in an
23 |   // `unsafe` block, that no aliasing `&`/`&mut` to the pointee is live.
24 |   const hasUnsafeDataMut = /pub unsafe fn data_mut\s*\(\s*this\s*:\s*&RefPtr<Store>\s*\)/.test(source);
25 |   expect(hasUnsafeDataMut).toBe(true);
                                ^
error: expect(received).toBe(expected)

Expected: true
Received: false

      at <anonymous> (/workspace/bun/test/internal/source-lints/store-data-mut-unsafe.test.ts:25:28)
(fail) Store::data_mut is an unsafe fn [6.16ms]
27 | 
28 | test("the pre-#30800 safe data_mut spelling is absent", () => {
29 |   // Anchored to the start of a line (`^\s*pub fn`) so `// `-prefixed prose
30 |   // can never match.
31 |   const hasSafeDataMut = /^\s*pub fn data_mut\s*\(/m.tes
... (truncated)

release without fix: 2 FAILED
bun test v1.4.1-canary.1 (18f71bcab)

test/internal/source-lints/store-data-mut-unsafe.test.ts:
20 | 
21 | test("Store::data_mut is an unsafe fn", () => {
22 |   // The precondition-bearing signature: every call site must assert, in an
23 |   // `unsafe` block, that no aliasing `&`/`&mut` to the pointee is live.
24 |   const hasUnsafeDataMut = /pub unsafe fn data_mut\s*\(\s*this\s*:\s*&RefPtr<Store>\s*\)/.test(source);
25 |   expect(hasUnsafeDataMut).toBe(true);
                                ^
error: expect(received).toBe(expected)

Expected: true
Received: false

      at <anonymous> (/workspace/bun/test/internal/source-lints/store-data-mut-unsafe.test.ts:25:28)
(fail) Store::data_mut is an unsafe fn [0.20ms]
27 | 
28 | test("the pre-#30800 safe data_mut spelling is absent", () => {
29 |   // Anchored to the start of a line (`^\s*pub fn`) so `// `-prefixed prose
30 |   // can never match.
31 |   const hasSafeDataMut = /^\s*pub fn data_mut\s*\(/m.test(source);
32 |   expect(hasSafeDataMut).toBe(false);
                              ^
error: expect(received).toBe(expected)

Expected: false
Received: true

      at <anonymous> (/workspace/bun/test/internal/source-li
... (truncated)
passes on PR (with fix) 
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/internal/source-lints/store-data-mut-unsafe.test.ts test/js/web/fetch/blob-write.test.ts
bun test v1.4.1 (861e9ae04)

test/internal/source-lints/store-data-mut-unsafe.test.ts:
(pass) Store::data_mut is an unsafe fn [3.51ms]
(pass) the pre-#30800 safe data_mut spelling is absent [2.24ms]

test/js/web/fetch/blob-write.test.ts:
(pass) blob.write() throws for data-backed blob [5.33ms]
(pass) Bun.write() throws for a data-backed blob destination [5.26ms]
(pass) Bun.file(path).write() does not throw [32.68ms]
(pass) blob.unlink() throws for data-backed blob [4.85ms]
(pass) blob.delete() throws for data-backed blob [3.67ms]
(pass) Bun.file(path).unlink() does not throw [25.21ms]
(pass) Bun.file(path).delete() does not throw [15.27ms]
(pass) blob.writer() throws for data-backed blob [3.05ms]
(pass) Bun.file(path).writer() does not throw [309.29ms]
(pass) blob.stat() returns undefined for data-backed blob [4.35ms]
(pass) Bun.file(path).stat() returns stats [11.60ms]
(pass) Bun.file(path).write() rejects a non-string options.type [8.46ms]
(
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     8aedffc298
  features     baseline

23 deps, 129 codegen, 1172 objects in 624ms

ninja: Entering directory `/workspace/bun/build/release'
[1/143] gen generated_host_exports.rs
generated_host_exports.rs: 116 exports (host=5, lazy=10, generic=101, rust=0); 243 extern-C blocks audited
[2/143] gen cpp.rs (cppbind)
[3/143] gen JS modules (bundle-modules)
Preprocess modules (7595ms)
Bundle modules (32ms)
Postprocesss modules (240ms)
Bundle Functions (488ms)
Generate Code (52ms)

[8.41s] Bundled "src/js" for production
  2595 kb
  197 internal modules
  13 native modules
  50 internal functions across 16 files
[3/143] cargo bun_runtime → libbun_runtime.a (--target x86_64-unknown-linux-gnu)

  nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19)

   Compiling bun_core v0.0.0 (/workspace/bun/src/bun_core)
   Compiling bun_jsc v0.0.0 (/workspace/bun/src/jsc)
   Compiling bun_alloc v0.0.0 (/workspace/bun/src/bu
... (truncated)
diff hotspot 
src/jsc/webcore_types.rs                           |  66 +++++--
 src/runtime/webcore/Blob.rs                        | 198 ++++++++++++++-------
 src/runtime/webcore/Body.rs                        |  53 ++++--
 src/runtime/webcore/FileReader.rs                  |  15 +-
 src/runtime/webcore/S3File.rs                      |   4 +-
 src/runtime/webcore/blob/copy_file.rs              |  12 +-
 src/runtime/webcore/blob/read_file.rs              |  19 +-
 .../source-lints/store-data-mut-unsafe.test.ts     |  33 ++++
 test/js/web/fetch/blob-write.test.ts               |  41 +++++
 9 files changed, 330 insertions(+), 111 deletions(-)

gate history · 5 passed · 0 rejected · iteration 47

evidence per changed file 
file                                                      reads  edits  tests
src/jsc/webcore_types.rs                                     25     23     30
src/runtime/webcore/Blob.rs                                  60     59     27
src/runtime/webcore/Body.rs                                  23     23     26
src/runtime/webcore/FileReader.rs                            16     12     27
src/runtime/webcore/S3File.rs                                 1      0     27
src/runtime/webcore/blob/copy_file.rs                         7      6     26
src/runtime/webcore/blob/read_file.rs                        15      8     26
test/internal/source-lints/store-data-mut-unsafe.test.ts      0      1      0
test/js/web/fetch/blob-write.test.ts                          5      6     26

root cause · written by the author bot

The root cause was that StoreRef exposed a safe data_mut(&self) method that minted a mutable reference from a shared reference while also being declared Sync, so two threads sharing a &StoreRef could obtain aliasing &mut references to the same store data, which is undefined behavior reachable entirely from safe code. The fix makes data_mut an unsafe fn with documented exclusivity preconditions and removes the Sync impl, so every call site must uphold the aliasing contract explicitly and cross-thread shared access is ruled out by the type system. Call sites were reworked to use shared borrow…