resolver: take Entry.mutex when filling Entry.abs_path in load_as_file and siblings - #34411
Open
robobun wants to merge 4 commits into
Open
resolver: take Entry.mutex when filling Entry.abs_path in load_as_file and siblings#34411robobun wants to merge 4 commits into
robobun wants to merge 4 commits into
Claude / Claude Code Review
completed
Jul 16, 2026 in 18m 48s
Code review found 1 important issue
Found 3 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | test/js/bun/util/filesystem_router.test.ts:860-862 |
pathsOk assertion fails on Windows: router.routes values are forward-slash but pagesDir uses backslashes |
| 🟡 Nit | src/router/lib.rs:1035-1041 |
Sibling torn-read site missed: hot_reloader.rs reads Entry.abs_path after releasing Entry.mutex |
Annotations
Check failure on line 862 in test/js/bun/util/filesystem_router.test.ts
claude / Claude Code Review
pathsOk assertion fails on Windows: router.routes values are forward-slash but pagesDir uses backslashes
On Windows this new `pathsOk` check will always fail: `pagesDir = path.join(import.meta.dir, "pages")` yields a backslash path, but `router.routes` values are normalized to forward slashes (see `platform_to_posix_buf` in `Route::parse`, and the `make()` helper at the top of this file which does `.replaceAll("\\", "/")` for exactly this reason). Normalize `pagesDir` to forward slashes before the `startsWith` check so the assertion passes on Windows CI.
Check warning on line 1041 in src/router/lib.rs
claude / Claude Code Review
Sibling torn-read site missed: hot_reloader.rs reads Entry.abs_path after releasing Entry.mutex
Same-class sibling: `src/jsc/hot_reloader.rs:1205` reads `ent.abs_path` right *after* the `_entry_guard` block closes at line 1204, so the two-word `Interned` load is unsynchronized against the resolver fill sites this PR just locked — the identical torn-`(ptr, len)` hazard you're fixing here for `Route::parse`. Moving `path_string = ent.abs_path;` inside the existing guard block closes it. (A lower-risk sibling also exists at `src/runtime/cli/test/Scanner.rs:420/453`, though `bun test`'s scanne
Loading