resolver: dedupe DirnameStore interns across bust+reread and Route::parse

[robobun]

Repro

const { dirnameStoreAppendCount } = require("bun:internal-for-testing");
// pages/ with 20 .tsx files plus pages/sub/ with 1 .tsx file
const router = new Bun.FileSystemRouter({ dir: pagesDir, style: "nextjs", fileExtensions: [".tsx"] });
router.reload();
let prev = dirnameStoreAppendCount();
for (let i = 0; i < 50; i++) router.reload();
console.log(dirnameStoreAppendCount() - prev); // 3450 before, 0 after

Run long enough and it panics:

panic: unreachable: AllocError
  <bun_router::Route>::parse  src/router/lib.rs
    FileSystem::instance().dirname_store().append(_abs).expect("unreachable")

Cause

BSSStringList::append does not dedupe by content, so the same bytes can be appended repeatedly into the never-freed DirnameStore. Two paths do that on every reload() (and on every --hot / --watch file change):

• Resolver::dir_info_cached_miss (and its sibling dir_info_for_resolution): after bust_dir_cache removes a directory from both caches, the next read_dir_info miss re-interns the directory's _safe_path (append_parts/append_slice) and, because the orphaned DirEntry slot is unreachable, DirEntry.dir (append_slice). 2 appends for the root dir and 4 for each subdir per reload.
• Route::parse: re-interns each route's public_path, abs_path, basename, and lowercased match_name. 3 to 4 appends per route per reload.
• RealFS::read_directory_with_iterator: same DirEntry.dir shape as the resolver path, reached via load_as_file after a bust.

For the repro above that is 69 appends per reload, independent of anything but route/dir counts. The store's slice_buf (4096 slots) plus overflow list cap out around 8.4M appends, after which append returns AllocError and the .expect("unreachable") panics; every overflow append before that leaks a heap buffer.

Fix

Add content-deduplicating wrappers on DirnameStore:

• intern_slice(value): exists() pointer-range fast path (slice already lives in the store's inline buffer), else probe a process-wide Mutex<HashMap<&'static [u8], ()>> keyed by content; hit returns the prior intern, miss appends once and records it. The lookup, append, and insert happen under the same lock so concurrent resolver workers cannot race past each other.
• intern_parts(parts) / intern_lower_case(value): stack-scratch concat/lowercase then delegate to intern_slice.

Route the bust-cycle-sensitive call sites through them:

• resolver.rs: _safe_path in dir_info_cached_miss, and DirEntry.dir in both dir_info_cached_miss and dir_info_for_resolution.
• lib.rs: DirEntry.dir in read_directory_with_iterator.
• router/lib.rs: all six DirnameStore appends in Route::parse (including the Windows cfg arm).

After the first reload, every subsequent reload() over an unchanged tree performs zero new DirnameStore appends.

Also expose DirnameStore::append_count via bun:internal-for-testing; reads slice_buf_used/overflow_list.count as raw-place projections under the store's inner mutex (no whole-struct &BSSStringList before locking).

Why this fix

The invariant that the process-lifetime DirnameStore should not grow when no new paths are seen belongs on DirnameStore itself, not in each caller. The dedup map is process-wide to match the backing store's lifetime; the callers this PR touches already hold RESOLVER_MUTEX or run on the JS thread, so there is no new contention.

Verification

• New test reload() does not re-intern directory paths into DirnameStore on every bust+reread in test/js/bun/util/filesystem_router.test.ts: 21 routes over 2 dirs, 50 reloads, asserts dirnameStoreAppendCount() delta is exactly 0 and match() still works.
  • Without the call-site changes (probe only): delta = 3450, test fails.
  • With the fix: delta = 0, test passes.
• Full filesystem_router.test.ts (30/30), resolve.test.ts (43/43), import-meta.test.js + require.test.ts (42/42) all green under bun bd.
• bun run rust:check-all clean on all targets including the Windows arm in Route::parse.

Relationship to other PRs

Supersedes #34276 (which added a router-local intern_route_path thread-local; this PR puts the dedup on DirnameStore so the resolver's own re-interns are covered in the same place). #29919 addresses the DirEntry/EntryStore reuse side of the same bust cycle and is complementary.

[robobun]

^{Updated 7:15 PM PT - Jul 15th, 2026}

❌ @robobun, your commit 87d758b has 2 failures in Build #73486 (All Failures):

• test/cli/run/no-orphans.test.ts - code 1 on 🍎 14 x64
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64

---

🧪   To try this PR locally:

bunx bun-pr 34284

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

bun-34284 --bun

[robobun]

Status: reproduced (delta=3450 over 50 reloads without the fix, 0 with it). test/js/bun/util/filesystem_router.test.ts 30/30 and the resolver suite pass under bun bd; bun run rust:check-all clean on all targets. Supersedes #34276 by moving the dedup onto DirnameStore so the resolver's own dir_info_cached_miss re-interns are covered in the same place.

Review follow-ups addressed in 87d758b: DIRNAME_INTERN is now a process-wide LazyLock<Mutex<HashMap<..>>> matching the store's lifetime, and append_count uses raw-place reads under the inner mutex (no whole-struct &BSSStringList before locking).

CI build 73486: filesystem_router.test.ts passed on all lanes. Other failures are unrelated to this diff (pre-existing test-net-connect-memleak.js; flaky bake/deinitialization, 30205 napi leak, repl-close, no-orphans perl-daemon timeout on darwin, etc.).

Holding for @Jarred-Sumner's reply on the overflow design question above before pushing anything further.

[coderabbitai]

Walkthrough

Changes

DirnameStore interning

Layer / File(s)	Summary
DirnameStore interning core src/resolver/lib.rs	Adds deduplicated interning helpers and applies them to cached directory-name slices.
Resolver and route integration src/resolver/resolver.rs, src/router/lib.rs	Resolver caches and route parsing now reuse interned directory, route, basename, and normalized path slices.
Append-count test hook and reload regression src/codegen/generate-js2native.ts, src/js/internal-for-testing.ts, src/runtime/api/filesystem_router.rs, test/js/bun/util/filesystem_router.test.ts	Exposes the DirnameStore append counter and verifies repeated router reloads add no directory-store entries while preserving route matching.

Possibly related PRs

• oven-sh/bun#34276: Implements the same DirnameStore interning and reload regression coverage.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly summarizes the main change: deduping DirnameStore interning for resolver reloads and Route::parse.
Description check	✅ Passed	The description covers the problem, fix, and verification, but it doesn't use the template's exact section headings.

---

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

[github-actions]

This PR may be a duplicate of:

1. router: dedupe DirnameStore interns so FileSystemRouter.reload() does not exhaust the store #34276 - Also deduplicates DirnameStore interns to prevent memory exhaustion during FileSystemRouter.reload() cycles, using the same thread-local HashMap approach

🤖 Generated with Claude Code

[coderabbitai]

Actionable comments posted: 1

🤖 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/resolver/lib.rs`:
- Around line 140-199: Make the DIRNAME_INTERN index process-wide rather than
thread-local, using a shared synchronized map accessible by all DirnameStore
users. Update intern_slice so lookup, append_slice, and insertion occur under
the same lock, preventing concurrent workers from appending duplicate paths to
the shared backing store; preserve the existing exists fast path and
intern_parts/intern_lower_case behavior.

🪄 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: 547d76e8-ca1f-4f18-b1b6-e1a4afd11660

📥 Commits

Reviewing files that changed from the base of the PR and between 5d350cc and a61ac31.

📒 Files selected for processing (7)

• src/codegen/generate-js2native.ts
• src/js/internal-for-testing.ts
• src/resolver/lib.rs
• src/resolver/resolver.rs
• src/router/lib.rs
• src/runtime/api/filesystem_router.rs
• test/js/bun/util/filesystem_router.test.ts

[Jarred-Sumner]

No, this is a bad solution. We shouldn't have a situation where DirnameStore can overflow. The memory should be ref counted when it would overflow.

[robobun]

The bug here is specifically that the same N directory/route paths are re-appended to the store on every reload() (and every --hot file change). Content dedup makes that bounded at N total appends regardless of how many times you reload, which is the same footprint as if no bust ever happened. It doesn't add a new growth dimension.

Happy to close this and pursue ref-counted overflow instead, but want to make sure I understand the scope you have in mind:

1. Make the overflow block list unbounded (growable Vec<Box<Block>> instead of [Option<Box<Block>>; 4096] in OverflowGroup). That removes the AllocError cap so it can never panic, but every bust+reread still leaks one copy per path per cycle; RSS is still unbounded over a dev session. Small change to bun_alloc.

2. Ref-count the overflow entries so they can be reclaimed when the DirInfo/DirEntry/Route that holds them is busted. That means threading a counted handle through every consumer that today holds &'static [u8] from the store (DirInfo.abs_path, DirEntry.dir, Entry.abs_path, the Route string fields, PackageJSON/TSConfigJSON path fields, etc.) and dropping the count in each of their reset/bust paths. Roughly the ~80 dirname_store/DirnameStore call sites plus the structs that store the result.

3. Both: keep the dedup (so same-path reload is O(1)) and make overflow ref-counted (so distinct-path growth is reclaimable).

Which of these are you after? If it's (2) or (3) I'll close this and #34276 and open a fresh PR scoped to the allocator + handle type.

[claude]

No bugs found on 87d758b — the earlier append_count aliasing nit and the thread-local-vs-process-global dedupe concern are both addressed. Deferring to a human because this adds a new process-wide Mutex<HashMap> on the resolver's dir-info cache-miss path and reroutes several hot DirnameStore call sites across resolver.rs / router/lib.rs; a maintainer should sign off on the locking layering and the design choice to put dedup on DirnameStore (this supersedes #34276).

What was reviewed:

• intern_slice lock ordering: DIRNAME_INTERN → BSSStringList.mutex is one-way (nothing under the inner mutex reaches back into intern_*), so no deadlock.
• append_count now uses raw-place reads under the inner mutex, matching the append/append_mutable pattern.
• exists() fast-path lifetime widen is sound (pointer-range check into the process-lifetime backing buffer).
• Call-site swaps in Route::parse (incl. Windows cfg arm), dir_info_cached_miss, dir_info_for_resolution, and read_directory_with_iterator preserve return type and 'static contract.

Extended reasoning...

Overview

Adds content-deduplicating intern_slice / intern_parts / intern_lower_case wrappers on DirnameStore backed by a process-wide LazyLock<Mutex<HashMap<&'static [u8], ()>>>, plus an append_count() accessor exposed via bun:internal-for-testing. Reroutes ~9 call sites in src/resolver/resolver.rs, src/resolver/lib.rs (read_directory_with_iterator), and src/router/lib.rs (Route::parse, both platform arms) from append* to intern*. Adds a regression test asserting 50 reloads over 21 routes perform zero new appends.

Security risks

None identified. No user-controlled input reaches new parsing; the intern map is keyed by resolver-derived filesystem paths already trusted at these call sites.

Level of scrutiny

High. The resolver's dir_info_cached_miss / dir_info_for_resolution and read_directory_with_iterator are on the module-resolution hot path for the runtime, bundler, and --hot/--watch. The change introduces a new process-wide mutex acquired on every cache miss (held across the inner BSSStringList append), and the surrounding code has a documented Stacked-Borrows aliasing discipline (*mut Self + raw-place projections) that new code must respect. This is not a mechanical change.

Other factors

• Two prior review threads (mine on append_count aliasing UB, CodeRabbit on thread-local vs. process-global dedupe) were both addressed in 87d758b and verified against the current diff.
• Lock ordering checked: DIRNAME_INTERN is only ever taken by the new intern_* wrappers, which then call append_slice → inner BSSStringList.mutex; nothing under that inner mutex calls back into intern_*, so no inversion.
• Nested locking means all intern_* callers now serialize on DIRNAME_INTERN in addition to the existing inner mutex. The PR description argues callers already hold RESOLVER_MUTEX or run on the JS thread, but bundler workers reach read_directory_with_iterator too — a maintainer should confirm the contention profile is acceptable.
• This supersedes #34276 with a different design (dedup on DirnameStore vs. router-local); that architectural choice deserves human sign-off.
• Tests: 30/30 filesystem_router, 43/43 resolve, rust:check-all clean per PR description; new test asserts delta === 0 and route matching still works.