diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index a622920a50b..6749a47268e 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -6826,19 +6826,21 @@ pub mod bv2_impl { .text; let loader = this.graph.input_files.items_loader()[source_index as usize]; if this.should_add_watcher(source_path) { - // const generic `CLONE_FILE_PATH = isWindows` - // matches `cfg!(windows)` at compile time. - let _ = this - .bun_watcher_mut() - .unwrap() - .add_file::<{ cfg!(windows) }>( - parse_result.watcher_data.fd, - source_path, - bun_wyhash::hash(source_path) as u32, - bun_watcher::Loader(loader as u8), - parse_result.watcher_data.dir_fd, - None, - ); + // CLONE_FILE_PATH=true: `source_path` is not guaranteed to + // be process-lifetime storage — `dupe_alloc` allocates the + // disjoint `text\0pretty\0` case (e.g. every `ssr:`-prefixed + // SSR-graph path) from the per-bundle arena, which is freed + // when the bundle completes. The watchlist outlives the + // bundle, so it must own its copy of the path (the watcher + // dedups by hash, so this is one copy per watched file). + let _ = this.bun_watcher_mut().unwrap().add_file::( + parse_result.watcher_data.fd, + source_path, + bun_wyhash::hash(source_path) as u32, + bun_watcher::Loader(loader as u8), + parse_result.watcher_data.dir_fd, + None, + ); } } } diff --git a/src/runtime/bake/DevServer.rs b/src/runtime/bake/DevServer.rs index 930ca31e707..bb4bdd9ea1d 100644 --- a/src/runtime/bake/DevServer.rs +++ b/src/runtime/bake/DevServer.rs @@ -4503,13 +4503,21 @@ pub(super) fn finalize_bundle( dev.incremental_result.html_routes_soft_affected.clear(); ctx.gts.clear(); - for index in &dev.incremental_result.client_components_affected { + // `trace_dependencies` appends to `client_components_affected` when it + // visits another client component boundary, so iterate by index against + // the live Vec (never a captured slice). Entries appended mid-loop + // already had their `gts` bit set when pushed, so re-visiting them is a + // no-op and the loop is bounded. + let mut i = 0; + while i < dev.incremental_result.client_components_affected.len() { + let index = dev.incremental_result.client_components_affected[i]; dev.server_graph.trace_dependencies( - *index, + index, ctx.gts, incremental_graph::TraceDependencyGoal::NoStop, - *index, + index, )?; + i += 1; } for request in &dev.incremental_result.framework_routes_affected { diff --git a/src/runtime/bake/FrameworkRouter.rs b/src/runtime/bake/FrameworkRouter.rs index 4f7fb1f503a..879b8f1c878 100644 --- a/src/runtime/bake/FrameworkRouter.rs +++ b/src/runtime/bake/FrameworkRouter.rs @@ -1385,7 +1385,10 @@ impl TinyLog { } pub fn print(&self, rel_path: &[u8]) { - let cursor_at = self.cursor_at as usize; + // `cursor_at` may still be the `empty()` sentinel (`u32::MAX`) when the + // producer recorded a message without a cursor position. Clamp to the + // path length so the slicing below cannot go out of bounds. + let cursor_at = (self.cursor_at as usize).min(rel_path.len()); let cursor_len = self.cursor_len as usize; let after = &rel_path[cursor_at..]; Output::err_generic( @@ -1639,8 +1642,25 @@ impl FrameworkRouter { .parse(rel_path, ext, &mut log, t.allow_layouts, arena_state); let parsed = match parse_result { Err(_) => { - log.cursor_at += - u32::try_from(abs_root_len - root_len).expect("int cast"); + // Rebase the cursor from `rel_path` coordinates + // (where `parse` recorded it) onto + // `full_rel_path` (what `print` receives). + // `rel_path` starts one byte before the + // `full_rel_path` suffix it mirrors — the + // leading '/' — so the offset is one less than + // the root-length delta. Error paths that never + // called `log.fail()` (e.g. OOM) leave the + // `u32::MAX` sentinel; keep it pinned there so + // `print` clamps to the end of the path. + if log.cursor_at != u32::MAX { + log.cursor_at = log + .cursor_at + .saturating_add( + u32::try_from(abs_root_len - root_len) + .expect("int cast"), + ) + .saturating_sub(1); + } ctx.on_router_syntax_error(full_rel_path, log)?; arena_state.reset_retain_with_limit(8 * 1024 * 1024); continue 'outer; @@ -1673,7 +1693,14 @@ impl FrameworkRouter { } if param_count > 64 { - log.write(format_args!("Pattern cannot have more than 64 param")); + // Use `fail()` so the cursor is set, highlighting + // the whole path: unlike `fail()` callers inside + // `parse`, there is no narrower span to point at. + let _ = log.fail( + format_args!("Pattern cannot have more than 64 param"), + 0, + full_rel_path.len(), + ); ctx.on_router_syntax_error(full_rel_path, log)?; arena_state.reset_retain_with_limit(8 * 1024 * 1024); continue 'outer; diff --git a/test/bake/dev/bundle.test.ts b/test/bake/dev/bundle.test.ts index 1a4e904167e..24c22a7d1b9 100644 --- a/test/bake/dev/bundle.test.ts +++ b/test/bake/dev/bundle.test.ts @@ -814,3 +814,65 @@ devTest("barrel optimization: two import statements from the same barrel (#28886 await c.expectMessage("got: ALPHA BETA"); }, }); + +// Regression: editing a file shared by several client component boundaries +// crashed the dev server. `finalizeBundle` iterates +// `incremental_result.client_components_affected` while `trace_dependencies` +// re-appends every visited boundary to that same list (the trace bits were +// just cleared), so with 3+ boundaries the Vec reallocates mid-iteration and +// the stale iterator reads freed memory — caught as a use-after-free under +// ASAN, and as `index out of bounds: the len is N but the index is N` in +// release once the freed block is reused by `framework_routes_affected`. +devTest("hot update touching multiple client component boundaries does not crash", { + framework: { + ...minimalFramework, + serverComponents: { + ...minimalFramework.serverComponents!, + separateSSRGraph: true, + }, + }, + files: { + "routes/index.ts": ` + import * as A from '../components/A'; + import * as B from '../components/B'; + import * as C from '../components/C'; + import * as D from '../components/D'; + export default function (req, meta) { + // Client component exports are reference proxies on the server; only + // their existence is observable here. + return new Response('page: ' + [typeof A.a, typeof B.b, typeof C.c, typeof D.d].join(',')); + } + `, + "components/A.ts": ` + "use client"; + import { shared } from './shared'; + export const a = 'A' + shared; + `, + "components/B.ts": ` + "use client"; + import { shared } from './shared'; + export const b = 'B' + shared; + `, + "components/C.ts": ` + "use client"; + import { shared } from './shared'; + export const c = 'C' + shared; + `, + "components/D.ts": ` + "use client"; + import { shared } from './shared'; + export const d = 'D' + shared; + `, + "components/shared.ts": `export const shared = '1';`, + }, + async test(dev) { + await dev.fetch("/").equals("page: object,object,object,object"); + // Invalidate every client component boundary in a single hot update. + await dev.write("components/shared.ts", `export const shared = '2';`); + await dev.fetch("/").equals("page: object,object,object,object"); + // A second round exercises the watcher entries recorded during the + // previous (SSR-duplicated) bundle. + await dev.write("components/shared.ts", `export const shared = '3';`); + await dev.fetch("/").equals("page: object,object,object,object"); + }, +}); diff --git a/test/bake/framework-router.test.ts b/test/bake/framework-router.test.ts index 62a07c4b79f..82a0af9b74f 100644 --- a/test/bake/framework-router.test.ts +++ b/test/bake/framework-router.test.ts @@ -1,6 +1,6 @@ import { frameworkRouterInternals } from "bun:internal-for-testing"; import { describe, expect, test } from "bun:test"; -import { tempDirWithFiles } from "harness"; +import { bunEnv, bunExe, isWindows, tempDirWithFiles } from "harness"; import path from "path"; const { parseRoutePattern, FrameworkRouter } = frameworkRouterInternals; @@ -133,3 +133,78 @@ test("discovers from filesystem paths", () => { ], }); }); + +// Regression: printing a route syntax error crashed the dev server instead of +// reporting it. The 65+ parameter check recorded its message without a cursor +// position, leaving `TinyLog.cursor_at` at the `u32::MAX` sentinel, and +// `TinyLog.print` sliced `rel_path[cursor_at..]` with it. +// The 65-directory fixture exceeds Windows' MAX_PATH; the code under test is +// platform-independent. +test.skipIf(isWindows)("dev server reports route syntax errors without crashing", async () => { + const manyParams = Array.from({ length: 65 }, (_, i) => `[p${i}]`).join("/"); + const dir = tempDirWithFiles("fsr-syntax-error", { + "server.ts": ` + export function render(req, meta) { + return meta.pageModule.default(req, meta); + } + `, + "main.ts": ` + export default { + port: 0, + development: true, + app: { + framework: { + fileSystemRouterTypes: [ + { root: "routes", style: "nextjs-pages", serverEntryPoint: "./server.ts" }, + ], + }, + }, + }; + `, + "routes/index.tsx": `export default () => new Response("ok");`, + "routes/[bad.tsx": `export default () => new Response("bad");`, + [`routes/${manyParams}/index.tsx`]: `export default () => new Response("deep");`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.ts"], + cwd: dir, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + // Drain stderr concurrently with the stdout scan below so neither pipe + // can fill up and block the child. + const stderrPromise = proc.stderr.text(); + + // The route scan runs during server startup, so "Started development + // server" only prints after both syntax errors were reported. Without the + // fix the process dies while printing the 65-param error. + let stdout = ""; + let url: string | undefined; + const decoder = new TextDecoder(); + for await (const chunk of proc.stdout) { + stdout += decoder.decode(chunk, { stream: true }); + url = stdout.match(/Started development server: (http:\/\/\S+)/)?.[1]; + if (url) break; + } + expect(url).toBeDefined(); + + // The server must still respond after reporting the errors. + const res = await fetch(url!); + expect(await res.text()).toBe("ok"); + + proc.kill(); + const [stderr] = await Promise.all([stderrPromise, proc.exited]); + // The caret/underline must line up with the offending "[" — column + // 8 for the `error: "` prefix plus 7 for `routes/`. + expect(stderr).toContain( + 'error: "routes/[bad.tsx" is not a valid route\n' + + " ".repeat(8 + 7) + + "-------\n" + + " ".repeat(8 + 7) + + 'Missing "]" to match this route parameter', + ); + expect(stderr).toContain("Pattern cannot have more than 64 param"); +});