Skip to content

resolver: take Entry.mutex when filling Entry.abs_path in load_as_file and siblings #34411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 4 commits into
base: main
Choose a base branch
from
Open

resolver: take Entry.mutex when filling Entry.abs_path in load_as_file and siblings #34411

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3da13a6
resolver: take Entry.mutex when filling Entry.abs_path in load_as_fil…
robobun Jul 16, 2026
05e17c8
router: re-check abs_path under Entry.mutex before recomputing in Rou…
robobun Jul 16, 2026
6e4546a
hot_reloader: read Entry.abs_path inside the existing Entry.mutex guard
robobun Jul 16, 2026
dbbfcff
resolver: centralize Entry.abs_path lazy fill in abs_path_or_fill()
robobun Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 48 additions & 30 deletions src/resolver/resolver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3896,17 +3896,21 @@ impl<'a> Resolver<'a> {
}

let absolute_out_path: &[u8] = {
if entry_query.entry().abs_path.is_empty() {
// SAFETY: EntryStore-owned slot; resolver mutex held. RHS fully
// evaluated before LHS `&mut Entry` is materialized.
unsafe { &mut *entry_query.entry }.abs_path = Interned::from_static(
let _entry_guard = entry_query.entry().mutex.lock_guard();
let cached = entry_query.entry().abs_path;
if !cached.is_empty() {
cached.as_bytes()
} else {
let interned = Interned::from_static(
self.fs_ref()
.dirname_store
.append_slice(abs_esm_path)
.expect("unreachable"),
);
// SAFETY: EntryStore-owned slot; `Entry.mutex` held.
unsafe { &mut *entry_query.entry }.abs_path = interned;
interned.as_bytes()
}
entry_query.entry().abs_path.as_bytes()
};
let module_type = if let Some(pkg) = resolved_dir_info.package_json() {
pkg.module_type
Expand Down Expand Up @@ -5271,19 +5275,23 @@ impl<'a> Resolver<'a> {
== Fs::file_system::EntryKind::File
{
let out_buf: &[u8] = {
if lookup.entry().abs_path.is_empty() {
let _entry_guard = lookup.entry().mutex.lock_guard();
let cached = lookup.entry().abs_path;
if !cached.is_empty() {
cached.as_bytes()
} else {
let parts = [dir_info.abs_path, &base[..]];
let out_buf_ = self.fs_ref().abs_buf(&parts, bufs!(index));
// SAFETY: EntryStore-owned slot; resolver mutex held. RHS fully
// evaluated before LHS `&mut Entry` is materialized.
unsafe { &mut *lookup.entry }.abs_path = Interned::from_static(
let interned = Interned::from_static(
self.fs_ref()
.dirname_store
.append_slice(out_buf_)
.expect("unreachable"),
);
// SAFETY: EntryStore-owned slot; `Entry.mutex` held.
unsafe { &mut *lookup.entry }.abs_path = interned;
interned.as_bytes()
}
lookup.entry().abs_path.as_bytes()
};

if let Some(debug) = self.debug_logs.as_mut() {
Expand Down Expand Up @@ -5768,19 +5776,26 @@ impl<'a> Resolver<'a> {
}

let abs_path: &'static [u8] = {
if query.entry().abs_path.is_empty() {
// `abs_path` is a two-word slice; serialize on the per-entry
// mutex so the check/write/read is atomic relative to the
// router's `Route::parse` doing the same fill.
let _entry_guard = query.entry().mutex.lock_guard();
let cached = query.entry().abs_path;
if !cached.is_empty() {
cached.as_bytes()
} else {
let abs_path_parts = [query.entry().dir, query.entry().base()];
let joined = self.fs_ref().abs_buf(&abs_path_parts, bufs!(load_as_file));
// SAFETY: EntryStore-owned slot; resolver mutex held. RHS fully
// evaluated before LHS `&mut Entry` is materialized.
unsafe { &mut *query.entry }.abs_path = Interned::from_static(
let interned = Interned::from_static(
self.fs_ref()
.dirname_store
.append_slice(joined)
.expect("unreachable"),
);
// SAFETY: EntryStore-owned slot; `Entry.mutex` held.
unsafe { &mut *query.entry }.abs_path = interned;
interned.as_bytes()
}
query.entry().abs_path.as_bytes()
};

dec_ret!(Some(LoadResult {
Expand Down Expand Up @@ -5874,7 +5889,11 @@ impl<'a> Resolver<'a> {

dec_ret!(Some(LoadResult {
path: {
if query.entry().abs_path.is_empty() {
let _entry_guard = query.entry().mutex.lock_guard();
let cached = query.entry().abs_path;
if !cached.is_empty() {
cached.as_bytes()
} else {
// SAFETY: `dir` is `&'static [u8]` (DirnameStore-interned),
// copied out so no `&Entry` borrow survives into the
// `&mut Entry` write below.
Expand All @@ -5900,11 +5919,10 @@ impl<'a> Resolver<'a> {
.expect("unreachable"),
)
};
// SAFETY: EntryStore-owned slot; resolver mutex held. RHS
// fully evaluated above — sole `&mut Entry` for this write.
// SAFETY: EntryStore-owned slot; `Entry.mutex` held.
unsafe { &mut *query.entry }.abs_path = new_abs;
new_abs.as_bytes()
}
query.entry().abs_path.as_bytes()
},
diff_case: query.diff_case,
dirname_fd: entries!().fd,
Expand Down Expand Up @@ -5981,21 +5999,21 @@ impl<'a> Resolver<'a> {
// now that we've found it, we allocate it.
return Some(LoadResult {
path: {
// SAFETY: EntryStore-owned slot; resolver mutex held. RHS is fully
// evaluated (shared reads) before the LHS `&mut Entry` is
// materialized for the write — no overlapping unique borrow.
unsafe { &mut *query.entry }.abs_path = if query.entry().abs_path.is_empty()
{
Interned::from_static(
let _entry_guard = query.entry().mutex.lock_guard();
let cached = query.entry().abs_path;
if !cached.is_empty() {
cached.as_bytes()
} else {
let interned = Interned::from_static(
self.fs_ref()
.dirname_store
.append_slice(&buffer[..])
.expect("unreachable"),
)
} else {
query.entry().abs_path
};
query.entry().abs_path.as_bytes()
);
// SAFETY: EntryStore-owned slot; `Entry.mutex` held.
unsafe { &mut *query.entry }.abs_path = interned;
interned.as_bytes()
}
},
diff_case: query.diff_case,
dirname_fd: entries.fd,
Expand Down
24 changes: 21 additions & 3 deletions src/router/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,11 +1032,17 @@
// `base_`/`extname` are no longer used.
// SAFETY: caller passes an EntryStore-owned pointer valid for the
// process lifetime; no other live `&mut` to it during this call.
let entry_abs_path = unsafe { &*entry }.abs_path().as_bytes();
// `abs_path` is a two-word slice that other threads (the bundler's
// resolver) fill under `Entry.mutex`; read it under the same lock so
// this thread can never observe a torn (ptr, len).
let entry_abs_path = {
let _entry_guard = unsafe { &*entry }.mutex.lock_guard();

Check failure on line 1039 in src/router/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy 

unsafe block missing a safety comment
unsafe { &*entry }.abs_path()

Check failure on line 1040 in src/router/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy 

unsafe block missing a safety comment
};
Comment thread
robobun marked this conversation as resolved.
Outdated
let mut abs_path_str: &[u8] = if entry_abs_path.is_empty() {
b""
} else {
entry_abs_path
entry_abs_path.as_bytes()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
};

let base = &base_[0..base_.len() - extname.len()];
Expand Down Expand Up @@ -1153,12 +1159,24 @@
)
};

if abs_path_str.is_empty() {
'fill: {
if !abs_path_str.is_empty() {
break 'fill;
}
// The reads of `cache().fd` and the `set_abs_path` write below
// rewrite the cached `Entry`; serialize them on the per-entry
// mutex (the same lock every other `Entry` rewrite path takes).
// SAFETY: see fn-level NOTE — read-only reborrow.
let _entry_guard = unsafe { &*entry }.mutex.lock_guard();
// Re-check under the lock: the bundler's resolver may have
// filled `abs_path` between the initial locked read above and
// this acquire, in which case the open + get_fd_path + intern
// below would recompute an equivalent value.
let cached = unsafe { &*entry }.abs_path();

Check failure on line 1175 in src/router/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy 

unsafe block missing a safety comment
if !cached.is_empty() {
abs_path_str = cached.as_bytes();
break 'fill;
}
// NOTE: reshaped for borrowck — `defer if (needs_close) file.close()`
// becomes a scopeguard owning the Option<File>; `needs_close` is a
// Cell so the drop closure can read it while the body still mutates.
Expand Down
12 changes: 10 additions & 2 deletions test/js/bun/util/filesystem_router.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,7 @@ it("reload() while Bun.build() resolves the same directory", async () => {
await Bun.build({ entrypoints, target: "bun", throw: false });
let matches = 0;
let buildsOk = true;
let pathsOk = true;
for (let round = 0; round < 40; round++) {
const builds = Array.from({ length: 4 }, () =>
Bun.build({ entrypoints, target: "bun", throw: false }),
Expand All @@ -852,10 +853,17 @@ it("reload() while Bun.build() resolves the same directory", async () => {
const m = router.match("/p7");
if (m && m.filePath.endsWith("p7.tsx")) matches++;
}
// Each route's abs-path is filled by both the router (under the
// per-entry mutex) and the bundler's resolver for the same fresh
// Entry after every bust+reread; a torn value surfaces as a
// filePath that isn't the absolute .tsx path.
for (const fp of Object.values(router.routes)) {
pathsOk &&= typeof fp === "string" && fp.startsWith(pagesDir) && fp.endsWith(".tsx");
}
Comment thread
robobun marked this conversation as resolved.
const results = await Promise.all(builds);
buildsOk &&= results.every(r => r.success);
}
console.log("matches", matches, "builds-ok", buildsOk);
console.log("matches", matches, "builds-ok", buildsOk, "paths-ok", pathsOk);
`,
};
for (let i = 1; i <= 40; i++) {
Expand All @@ -877,7 +885,7 @@ it("reload() while Bun.build() resolves the same directory", async () => {
stderr: normalizeBunSnapshot(stderr, String(dir)),
exitCode,
signalCode: proc.signalCode,
}).toEqual({ stdout: "matches 2000 builds-ok true", stderr: "", exitCode: 0, signalCode: null });
}).toEqual({ stdout: "matches 2000 builds-ok true paths-ok true", stderr: "", exitCode: 0, signalCode: null });
}, 60_000);

it("loads routes from a directory already cached by Bun.build()", async () => {
Expand Down
Loading