Skip to content
1 change: 1 addition & 0 deletions scripts/rust-miri.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const MIRI_CRATES = [
"bun_http_types",
"bun_md",
"bun_ptr",
"bun_resolver",
"bun_resolve_builtins",
"bun_shell_parser",
"bun_threading",
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/hot_reloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,7 @@ where
core::sync::atomic::Ordering::Release,
);
}
path_string = ent.abs_path;
path_string = ent.abs_path();
file_hash = Watcher::get_hash(path_string.as_bytes());
for (entry_id, hash) in hashes.iter().enumerate() {
if *hash == file_hash {
Expand Down
3 changes: 3 additions & 0 deletions src/resolver/data_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ mod tests {
}

#[test]
#[cfg_attr(miri, ignore = "calls native SIMD functions")]
fn shortest_data_url_percent_path() {
// Plain ASCII: percent-escaped form is shorter than base64.
let url = DataURL::encode_string_as_shortest_data_url(b"text/plain", b"hello");
Expand All @@ -348,6 +349,7 @@ mod tests {
}

#[test]
#[cfg_attr(miri, ignore = "calls native SIMD functions")]
fn shortest_data_url_base64_fallback_invalid_utf8() {
// Non-UTF-8 input makes the percent-escape path bail; the fallback
// must emit a real base64 payload.
Expand All @@ -359,6 +361,7 @@ mod tests {
}

#[test]
#[cfg_attr(miri, ignore = "calls native SIMD functions")]
fn shortest_data_url_base64_fallback_when_shorter() {
// Every byte needs escaping (3 bytes each) so base64 (4/3 per byte)
// wins and the fallback path is taken.
Expand Down
158 changes: 128 additions & 30 deletions src/resolver/fs.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use core::sync::atomic::{AtomicBool, Ordering};
use core::sync::atomic::{AtomicBool, AtomicPtr, AtomicUsize, Ordering};
use std::borrow::Cow;
use std::io::Write as _;

Expand Down Expand Up @@ -137,7 +137,9 @@ pub struct Entry {
pub mutex: Mutex,
pub need_stat: AtomicBool,

pub abs_path: Interned,
// One `Interned`, written once under `mutex`: the length, then the pointer that publishes it.
abs_path_ptr: AtomicPtr<u8>,
abs_path_len: AtomicUsize,
}

impl Entry {
Expand Down Expand Up @@ -181,15 +183,47 @@ impl Entry {
self.dir
}

/// `Interned` is `Copy`.
fn published_abs_path(&self) -> Option<Interned> {
let ptr = self.abs_path_ptr.load(Ordering::Acquire);
if ptr.is_null() {
return None;
}
// Relaxed: the Acquire load above orders this after the length store.
let len = self.abs_path_len.load(Ordering::Relaxed);
// SAFETY: `abs_path_or_try_fill` stored the length and then the pointer of one
// `Interned`, so the pair is that process-lifetime slice.
Some(unsafe { Interned::assume(core::slice::from_raw_parts(ptr, len)) })
}

/// Lock-free. `Interned::EMPTY` until a fill has been published.
#[inline]
pub fn abs_path(&self) -> Interned {
self.abs_path
self.published_abs_path().unwrap_or(Interned::EMPTY)
}

#[inline]
pub fn set_abs_path(&mut self, p: Interned) {
self.abs_path = p;
/// The only writer of `abs_path`. A failed `fill` publishes nothing. `fill` runs under `mutex`.
pub fn abs_path_or_try_fill<E>(
&self,
fill: impl FnOnce() -> Result<Interned, E>,
) -> Result<Interned, E> {
if let Some(p) = self.published_abs_path() {
return Ok(p);
}
let _guard = self.mutex.lock_guard();
if let Some(p) = self.published_abs_path() {
return Ok(p);
}
let p = fill()?;
// Relaxed: the Release store of the pointer publishes the length too.
self.abs_path_len.store(p.len(), Ordering::Relaxed);
self.abs_path_ptr
.store(p.as_bytes().as_ptr().cast_mut(), Ordering::Release);
Ok(p)
}

pub fn abs_path_or_fill(&self, fill: impl FnOnce() -> Interned) -> Interned {
let Ok(p) = self.abs_path_or_try_fill(|| Ok::<_, core::convert::Infallible>(fill()));
p
}

/// Stat-on-first-use.
Expand Down Expand Up @@ -272,8 +306,7 @@ impl Entry {

// `entry` is a RAW `*mut Entry`. A safe
// `&self → &mut Entry` accessor would let two `get()` calls produce coexisting
// aliased `&mut Entry` (PORTING.md §Forbidden). Callers `unsafe { &mut *entry }`
// at each write site under the per-entry `Entry.mutex`.
// aliased `&mut Entry` (PORTING.md §Forbidden).
pub struct EntryLookup<'a> {
pub(crate) entry: *mut Entry,
// tie the lookup's nominal lifetime to the DirEntry it came from
Expand All @@ -285,23 +318,16 @@ impl<'a> EntryLookup<'a> {
///
/// # Safety (encapsulated)
/// `self.entry` is a slot in the process-lifetime `EntryStore` BSSMap
/// singleton (see `dir_entry::EntryStore`); never freed. `Entry`'s
/// only mutable state (`cache`) is behind `Cell`, so interior
/// writes via `set_cache*()` do not alias this `&Entry`. The
/// singleton (see `dir_entry::EntryStore`); never freed. `cache` is
/// behind `Cell` and `abs_path` is atomic, so the interior writes through
/// `set_cache*()` / `abs_path_or_fill()` do not alias this `&Entry`. The
Comment thread
robobun marked this conversation as resolved.
Resolved
/// `PhantomData<&'a Entry>` ties the borrow to the `DirEntry` it was
/// looked up from.
#[inline(always)]
pub fn entry(&self) -> &'a Entry {
// SAFETY: ARENA — EntryStore-owned slot; see fn doc.
unsafe { &*self.entry }
}

// former `entry_mut() -> &'a mut Entry` accessor removed
// (zero callers). `Entry`'s only mutable state (`cache`) is `Cell`-backed,
// so all mutation goes through `entry().set_cache*()` on a shared borrow;
// no `&mut Entry` escape hatch is needed. Write sites that bypass the
// accessor go through the raw `self.entry` field directly under the
// per-entry `Entry.mutex` (see struct doc above).
}

/// `DirEntry` companion items: the entry map, the global entry store, and the
Expand Down Expand Up @@ -459,14 +485,10 @@ impl DirEntry {
// `name_hash` instead of re-hashing.
if let Some(&existing_ptr) = map.get_hashed(name_hash, name_lc) {
// SAFETY: EntryStore-owned pointer, valid for lifetime of store
let existing = unsafe { &mut *existing_ptr };
// `MutexGuard` stores a `BackRef<Mutex>` (lifetime-erased), so
// holding it does not borrow `existing` — the field writes
// below remain unconstrained. Replaces the manual
// `lock()` + `scopeguard(addr_of!(mutex), |m| (*m).unlock())`
// backref-deref pair.
let existing = unsafe { &*existing_ptr };
let _guard = existing.mutex.lock_guard();
existing.dir = self.dir;
// An in-place refresh keeps the slot's `dir`, so there is nothing to store.
debug_assert!(core::ptr::eq(existing.dir, self.dir));

// No cache rewrite here, even when the kind changed: a
// lock-free `kind()`/`symlink()` reader that already
Expand Down Expand Up @@ -538,7 +560,8 @@ impl DirEntry {
kind: found_kind.unwrap_or(EntryKind::File),
fd: Fd::INVALID,
}));
addr_of_mut!((*p).abs_path).write(Interned::EMPTY);
addr_of_mut!((*p).abs_path_ptr).write(AtomicPtr::new(core::ptr::null_mut()));
addr_of_mut!((*p).abs_path_len).write(AtomicUsize::new(0));
p
}
};
Expand Down Expand Up @@ -678,9 +701,8 @@ impl ModKey {
}
}

// SAFETY: ARENA — `Entry` lives in the `BSSList` singleton; `*mut Entry` raw
// pointers are the only !Send/!Sync field. All access is serialized through
// `RealFS.entries_mutex`.
// SAFETY: `cache` is the only `!Sync` field. It is shared under the per-entry
// `mutex` discipline described on `Entry`.
unsafe impl Sync for Entry {}
// SAFETY: same invariant as the `Sync` impl above.
unsafe impl Send for Entry {}
Expand Down Expand Up @@ -1061,3 +1083,79 @@ pub struct PathContentsPair<'buf> {

#[path = "fs/stat_hash.rs"]
pub mod stat_hash;

#[cfg(test)]
mod tests {
use super::*;

fn entry() -> Entry {
Entry {
cache: core::cell::Cell::new(EntryCache {
symlink: Interned::EMPTY,
fd: Fd::INVALID,
kind: EntryKind::File,
}),
dir: b"/dir/",
base_: strings::StringOrTinyString::init(b"a.ts"),
base_lowercase_: strings::StringOrTinyString::init(b"a.ts"),
mutex: Mutex::new(),
need_stat: AtomicBool::new(false),
abs_path_ptr: AtomicPtr::new(core::ptr::null_mut()),
abs_path_len: AtomicUsize::new(0),
}
}

// Fails if two fills interleave or a reader sees a published pointer without its length.
#[test]
fn abs_path_fills_once_and_publishes_whole() {
const PATHS: [&[u8]; 2] = [b"/dir/a.ts", b"/real/dir/a.tsx"];
// One schedule does not always put the reader between the two stores.
for _ in 0..8 {
let entry = entry();
let (started, fills) = (AtomicUsize::new(0), AtomicUsize::new(0));
std::thread::scope(|s| {
for path in PATHS {
let (entry, started, fills) = (&entry, &started, &fills);
s.spawn(move || {
started.fetch_add(1, Ordering::SeqCst);
let filled = entry.abs_path_or_fill(|| {
// Hold the first fill open until the other filler contends.
while started.load(Ordering::SeqCst) < PATHS.len() {
std::thread::yield_now();
}
fills.fetch_add(1, Ordering::SeqCst);
Interned::from_static(path)
});
assert!(filled == entry.abs_path());
});
}
s.spawn(|| {
loop {
// Only the pointer's publish orders this reader with the fill.
let seen = entry.abs_path().as_bytes();
if PATHS
.iter()
.any(|path| core::ptr::eq(path.as_ptr(), seen.as_ptr()))
{
assert!(PATHS.contains(&seen));
break;
}
std::thread::yield_now();
}
});
});
assert_eq!(fills.load(Ordering::SeqCst), 1);
}
}

#[test]
fn failed_abs_path_fill_publishes_nothing() {
let entry = entry();
assert!(entry.abs_path_or_try_fill(|| Err(())).is_err());
assert!(entry.abs_path().is_empty());
let first = entry.abs_path_or_fill(|| Interned::from_static(b"/dir/a.ts"));
let second = entry.abs_path_or_try_fill(|| Err(()));
assert!(first.as_bytes() == b"/dir/a.ts" && second.is_ok_and(|p| p == first));
assert!(entry.abs_path() == first);
}
}
Loading
Loading