diff --git a/Cargo.lock b/Cargo.lock index ff9781257b..1e16960588 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14223,6 +14223,7 @@ dependencies = [ "id-arena", "itertools 0.14.0", "rayon", + "rustc-demangle", "tracing", "tracing-subscriber", "walrus", diff --git a/Cargo.toml b/Cargo.toml index 0b71f69ed4..ef458e27f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -222,6 +222,7 @@ slotmap = { version = "1.0.7", features = ["serde"] } futures = "0.3.32" futures-channel = "0.3.32" futures-util = { version = "0.3.32", default-features = false } +rustc-demangle = "0.1" rustc-hash = "2.1.1" wasm-bindgen = "0.2.121" wasm-bindgen-futures = "0.4.71" diff --git a/packages/core/src/diff/component.rs b/packages/core/src/diff/component.rs index 4359f91ef6..bf67d0e14f 100644 --- a/packages/core/src/diff/component.rs +++ b/packages/core/src/diff/component.rs @@ -23,6 +23,13 @@ impl VirtualDom { ) { let scope = &mut self.scopes[scope_id.0]; if SuspenseBoundaryProps::downcast_from_props(&mut *scope.props).is_some() { + // A suspense boundary may rerun (e.g. resolve) while it is itself nested under a + // boundary that is still rendered as suspended. Its rendered state was created + // without a writer, so diffing it against the real dom would emit mutations that + // reference unmounted placeholder element ids. Defer the mutations like any other + // scope under a suspended boundary; the ancestor's own resolve pass will create + // the final state with a writer. + let to = to.filter(|_| self.runtime.scope_should_render(scope_id)); SuspenseBoundaryProps::diff(scope_id, self, to) } else { let new_nodes = self.run_scope(scope_id); diff --git a/packages/core/src/runtime.rs b/packages/core/src/runtime.rs index 2c48d1cb0a..8571d1e0f6 100644 --- a/packages/core/src/runtime.rs +++ b/packages/core/src/runtime.rs @@ -350,10 +350,24 @@ fn MyComponent() -> Element {{ return true; } - // If this is not a suspended scope, and we are under a frozen context, then we should + // If any boundary above this scope is currently rendered as suspended, this scope's + // rendered state only exists in the background (it was created without a writer), so + // it must not emit mutations. Checking only the nearest boundary is not enough: a + // resolved boundary (or this scope itself, if it is a boundary) may still be nested + // inside a suspended one, so walk the whole parent chain. let scopes = self.scope_states.borrow(); - let scope = &scopes[scope_id.0].as_ref().unwrap(); - !matches!(scope.suspense_location(), SuspenseLocation::UnderSuspense(suspense) if suspense.is_suspended()) + let mut current = Some(scope_id); + while let Some(id) = current { + let Some(Some(scope)) = scopes.get(id.0) else { + break; + }; + if matches!(scope.suspense_location(), SuspenseLocation::UnderSuspense(suspense) if suspense.is_suspended()) + { + return false; + } + current = scope.parent_id(); + } + true } /// Call a listener inside the VirtualDom with data from outside the VirtualDom. **The ElementId passed in must be the id of an element with a listener, not a static node or a text node.** diff --git a/packages/core/src/suspense/component.rs b/packages/core/src/suspense/component.rs index d4662da6e8..10c4086b98 100644 --- a/packages/core/src/suspense/component.rs +++ b/packages/core/src/suspense/component.rs @@ -273,29 +273,39 @@ impl SuspenseBoundaryProps { to: Option<&mut M>, ) -> usize { let mut scope_id = ScopeId(dom.get_mounted_dyn_node(mount, idx)); - // If the ScopeId is a placeholder, we need to load up a new scope for this vcomponent. If it's already mounted, then we can just use that - if scope_id.is_placeholder() { - { - let suspense_context = SuspenseContext::new(); - - let suspense_boundary_location = - crate::scope_context::SuspenseLocation::SuspenseBoundary( - suspense_context.clone(), - ); - dom.runtime - .clone() - .with_suspense_location(suspense_boundary_location, || { - let scope_state = dom - .new_scope(component.props.duplicate(), component.name) - .state(); - suspense_context.mount(scope_state.id); - scope_id = scope_state.id; - }); - } - - // Store the scope id for the next render - dom.set_mounted_dyn_node(mount, idx, scope_id.0); + // If the scope already exists, this is a re-creation pass (e.g. an outer suspense + // boundary resolving and rebuilding its subtree). Recreate the boundary's current + // rendered state (the fallback while suspended, or the children once resolved) just + // like any other already-mounted component. Re-running the children from props here + // would mint a second children tree with fresh child scopes and orphan the live + // ones: props.children may hold an unmounted clone left by a memoized props update, + // and the orphaned scopes would later diff against mounts that were never written. + if !scope_id.is_placeholder() { + let new_node = dom.scopes[scope_id.0] + .last_rendered_node + .clone() + .expect("Component to be mounted"); + return dom.create_scope(to, scope_id, new_node, parent); } + { + let suspense_context = SuspenseContext::new(); + + let suspense_boundary_location = + crate::scope_context::SuspenseLocation::SuspenseBoundary(suspense_context.clone()); + dom.runtime + .clone() + .with_suspense_location(suspense_boundary_location, || { + let scope_state = dom + .new_scope(component.props.duplicate(), component.name) + .state(); + suspense_context.mount(scope_state.id); + scope_id = scope_state.id; + }); + } + + // Store the scope id for the next render + dom.set_mounted_dyn_node(mount, idx, scope_id.0); + dom.runtime.clone().with_scope_on_stack(scope_id, || { let scope_state = &mut dom.scopes[scope_id.0]; let props = Self::downcast_from_props(&mut *scope_state.props).unwrap(); diff --git a/packages/core/tests/suspense.rs b/packages/core/tests/suspense.rs index e80bcc8f86..d17c067c92 100644 --- a/packages/core/tests/suspense.rs +++ b/packages/core/tests/suspense.rs @@ -454,6 +454,237 @@ fn toggle_suspense() { }); } +/// A nested suspense boundary that resolves while an ancestor suspense boundary is still +/// suspended must not write mutations. Its entire rendered state (including its own fallback) +/// was created in the background without a writer, so its mounts hold placeholder element ids +/// (`usize::MAX` / `ElementId(0)`). Writing a resolve diff against that state emits mutations +/// like `ReplaceWith { id: 4294967295 }` which crash the renderer. +/// +/// The scenario is a pure-CSR app whose root content suspends (implicit root boundary) while +/// an inner `SuspenseBoundary` suspends and resolves on its own faster future. +#[test] +fn nested_boundary_resolves_under_suspended_root() { + fn app() -> Element { + rsx! { + SlowOuter {} + SuspenseBoundary { + fallback: |_| rsx! { {"inner loading".to_string()} }, + FastInner {} + } + } + } + + #[component] + fn SlowOuter() -> Element { + let mut resolved = use_signal(|| false); + let task = use_hook(|| { + spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(400)).await; + resolved.set(true); + }) + }); + if !resolved() { + suspend(task)?; + } + rsx! { div { "outer done" } } + } + + #[component] + fn FastInner() -> Element { + let mut resolved = use_signal(|| false); + let task = use_hook(|| { + spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + resolved.set(true); + }) + }); + if !resolved() { + suspend(task)?; + } + rsx! { div { "inner done" } } + } + + fn assert_no_placeholder_ids(mutations: &dioxus_core::Mutations) { + let debug = format!("{:?}", mutations.edits); + let max = usize::MAX.to_string(); + assert!( + !debug.contains(&max), + "mutations reference an unmounted placeholder element id: {debug}" + ); + assert!( + !debug.contains("ReplaceWith { id: ElementId(0)"), + "mutations replace the root element: {debug}" + ); + } + + tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap() + .block_on(async { + let mut dom = VirtualDom::new(app); + let mutations = dom.rebuild_to_vec(); + assert_no_placeholder_ids(&mutations); + + // Drive the client event loop until all suspense is resolved, checking every + // batch of mutations for references to unmounted placeholder ids + let work = async { + loop { + dom.wait_for_work().await; + let mutations = dom.render_immediate_to_vec(); + assert_no_placeholder_ids(&mutations); + if !dom.suspended_tasks_remaining() { + break; + } + } + dom + }; + let mut dom = tokio::select! { + dom = work => dom, + _ = tokio::time::sleep(std::time::Duration::from_secs(5)) => { + panic!("suspense never resolved") + } + }; + // Flush any remaining work + let mutations = dom.render_immediate_to_vec(); + assert_no_placeholder_ids(&mutations); + + let out = dioxus_ssr::render(&dom); + assert_eq!(out, "
outer done
inner done
"); + }); +} + +/// A suspense boundary living inside the suspended background of an outer (here: the +/// implicit root) boundary whose parent re-renders during the outer suspension. The +/// memoized props update replaces `props.children` with an unmounted clone; when the +/// outer boundary resolves, its re-creation pass descends into the existing inner +/// boundary via `SuspenseBoundaryProps::create`, which must reuse the boundary's +/// current rendered state instead of re-running the children from props. Re-running +/// them mints a second children tree with fresh scopes and orphans the previous child +/// scopes: still alive and subscribed, but with writer-less renders whose mounts hold +/// placeholder element ids. Once nothing is suspended anymore, any signal write makes +/// the orphan re-render with a real writer, and removing its stale nodes emits +/// `ReplaceWith { id: usize::MAX }` which crashes the renderer. +#[test] +fn boundary_recreation_keeps_child_scopes() { + use std::time::Duration; + static SHARED: GlobalSignal = Signal::global(|| 0); + + fn app() -> Element { + use_hook(|| { + spawn(async move { + // Re-render Section (and memoize the inner boundary's props) while + // the root is still suspended + tokio::time::sleep(Duration::from_millis(10)).await; + *SHARED.write() = 1; + // Write again after everything has resolved + tokio::time::sleep(Duration::from_millis(200)).await; + *SHARED.write() = 2; + }) + }); + rsx! { + RootSuspender {} + Section {} + } + } + + // Suspends against the implicit root boundary, like a lazy route shim + #[component] + fn RootSuspender() -> Element { + let mut resolved = use_signal(|| false); + let task = use_hook(|| { + spawn(async move { + tokio::time::sleep(Duration::from_millis(50)).await; + resolved.set(true); + }) + }); + if !resolved() { + suspend(task)?; + } + rsx! { header { "root done" } } + } + + #[component] + fn Section() -> Element { + let shared = SHARED(); + rsx! { + section { + SuspenseBoundary { + fallback: |_| rsx! { div { "inner loading" } }, + // The dynamic text forces a fresh children VNode every render, + // so SuspenseBoundaryProps::memoize replaces props.children + div { "gen {shared}" } + InnerChild {} + } + } + } + } + + #[component] + fn InnerChild() -> Element { + let shared = SHARED(); + let mut resolved = use_signal(|| false); + let task = use_hook(|| { + spawn(async move { + tokio::time::sleep(Duration::from_millis(100)).await; + resolved.set(true); + }) + }); + if !resolved() { + suspend(task)?; + } + if shared % 2 == 1 { + // dynamic text root: removal goes through remove_dynamic_node(Text) + rsx! { "inner {shared}" } + } else { + rsx! { span { "inner {shared}" } } + } + } + + fn assert_no_placeholder_ids(mutations: &dioxus_core::Mutations) { + let debug = format!("{:?}", mutations.edits); + let max = usize::MAX.to_string(); + assert!( + !debug.contains(&max), + "mutations reference an unmounted placeholder element id: {debug}" + ); + assert!( + !debug.contains("ReplaceWith { id: ElementId(0)"), + "mutations replace the root element: {debug}" + ); + } + + tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap() + .block_on(async { + let mut dom = VirtualDom::new(app); + let mutations = dom.rebuild_to_vec(); + assert_no_placeholder_ids(&mutations); + + // Drive the client event loop through the resolve and past the final + // signal write, checking every batch of mutations + let deadline = tokio::time::Instant::now() + std::time::Duration::from_millis(600); + loop { + tokio::select! { + _ = dom.wait_for_work() => { + let mutations = dom.render_immediate_to_vec(); + assert_no_placeholder_ids(&mutations); + } + _ = tokio::time::sleep_until(deadline) => break, + } + } + assert!(!dom.suspended_tasks_remaining(), "suspense never resolved"); + + let out = dioxus_ssr::render(&dom); + assert_eq!( + out, + "
root done
gen 2
inner 2
" + ); + }); +} + #[test] fn nested_suspense_resolves_client() { use Mutation::*; diff --git a/packages/wasm-split/wasm-split-cli/Cargo.toml b/packages/wasm-split/wasm-split-cli/Cargo.toml index b03aca8702..cf0d187ec2 100644 --- a/packages/wasm-split/wasm-split-cli/Cargo.toml +++ b/packages/wasm-split/wasm-split-cli/Cargo.toml @@ -13,6 +13,7 @@ rust-version = "1.85.0" [dependencies] anyhow = { workspace = true } itertools = { workspace = true } +rustc-demangle = { workspace = true } walrus = { workspace = true, features = ["parallel"]} wasmparser = { workspace = true } id-arena = { workspace = true } diff --git a/packages/wasm-split/wasm-split-cli/src/lib.rs b/packages/wasm-split/wasm-split-cli/src/lib.rs index 8d94be5cbb..1b20ee9945 100644 --- a/packages/wasm-split/wasm-split-cli/src/lib.rs +++ b/packages/wasm-split/wasm-split-cli/src/lib.rs @@ -902,6 +902,32 @@ impl<'a> Splitter<'a> { } fn unused_main_symbols(&self) -> HashSet { + // The linker tail-merges constants, so data symbol ranges can nest and + // overlap. Pruning a symbol from the main module zeroes its whole byte + // range, which would also wipe any main-reachable symbol sharing those + // bytes. Build an interval index of the data ranges main keeps so those + // overlapping symbols stay in the main module too. + let mut main_data_ranges: Vec> = self + .main_graph + .iter() + .filter_map(|n| match n { + Node::DataSymbol(id) => self.data_symbols.get(id).map(|s| s.range.clone()), + _ => None, + }) + .collect(); + main_data_ranges.sort_by_key(|r| r.start); + let starts: Vec = main_data_ranges.iter().map(|r| r.start).collect(); + let mut prefix_max_end = Vec::with_capacity(main_data_ranges.len()); + let mut max_end = 0; + for r in &main_data_ranges { + max_end = max_end.max(r.end); + prefix_max_end.push(max_end); + } + let overlaps_main = |range: &Range| { + let idx = starts.partition_point(|&s| s < range.end); + idx > 0 && prefix_max_end[idx - 1] > range.start + }; + self.split_points .iter() .flat_map(|split| split.reachable_graph.iter()) @@ -914,7 +940,10 @@ impl<'a> Splitter<'a> { // And ensure we aren't also exporting it match sym { Node::Function(u) => self.source_module.exports.get_exported_func(*u).is_none(), - _ => true, + Node::DataSymbol(id) => match self.data_symbols.get(id) { + Some(symbol) => !overlaps_main(&symbol.range), + None => false, + }, } }) .cloned() @@ -940,20 +969,25 @@ impl<'a> Splitter<'a> { .flat_map(|f| Some((f.name.clone()?, f.id()))) .collect(); - let mut old_to_new = HashMap::new(); - let mut new_call_graph: HashMap> = HashMap::new(); - - for (new_name, new_func) in new_names.iter() { - if let Some(old_func) = old_names.get(new_name) { - old_to_new.insert(*old_func, new_func); - } else { - new_call_graph.insert(Node::Function(*new_func), HashSet::new()); + // The two name sections don't line up directly: wasm-bindgen demangles the + // name section (dx runs it with demangling enabled), while the original + // relocatable module carries mangled names. Match the raw name first and + // fall back to the demangled form. rustc-demangle's `Display` output is + // exactly what wasm-bindgen writes back into the module. + let mut old_to_new: HashMap = HashMap::new(); + for (old_name, old_func) in old_names.iter() { + let new_func = new_names + .get(old_name) + .or_else(|| new_names.get(&rustc_demangle::demangle(old_name).to_string())); + if let Some(new_func) = new_func { + old_to_new.insert(*old_func, *new_func); } } + let matched_new: HashSet = old_to_new.values().copied().collect(); let get_old = |old: &Node| -> Option { match old { - Node::Function(id) => old_to_new.get(id).map(|new_id| Node::Function(**new_id)), + Node::Function(id) => old_to_new.get(id).map(|new_id| Node::Function(*new_id)), Node::DataSymbol(id) => Some(Node::DataSymbol(*id)), } }; @@ -1013,7 +1047,10 @@ impl<'a> Splitter<'a> { Node::Function(id) => { let func = original.module.funcs.get(id); let name = func.name.as_ref().unwrap(); - if let Some(entry) = new_names.get(name) { + let entry = new_names + .get(name) + .or_else(|| new_names.get(&rustc_demangle::demangle(name).to_string())); + if let Some(entry) = entry { recovered_children.insert(Node::Function(*entry)); } } @@ -1025,14 +1062,19 @@ impl<'a> Splitter<'a> { } } - // We're going to attach the recovered children to the main function + // Attach the recovered children to the main function. This must land in the + // real call graph (`self.call_graph`). Anything reachable only through a + // function we couldn't line up has to stay in the main module. Otherwise its + // data or code is carved into a split chunk while main-module code still uses + // it, and the chunk's active data segments overwrite memory the main module + // reads from startup. let main_fn = self.source_module.funcs.by_name("main").context("Failed to find `main` function - was this built with LTO, --emit-relocs, and debug symbols?")?; - let main_fn_entry = new_call_graph.entry(Node::Function(main_fn)).or_default(); + let main_fn_entry = self.call_graph.entry(Node::Function(main_fn)).or_default(); main_fn_entry.extend(recovered_children); // Also attach any truly new symbols to the main function. Usually these are the shim functions - for (name, new) in new_names.iter() { - if !old_names.contains_key(name) { + for new in new_names.values() { + if !matched_new.contains(new) { main_fn_entry.insert(Node::Function(*new)); } } @@ -1189,6 +1231,7 @@ struct ModuleWithRelocations<'a> { module: Module, symbols: Vec>, names_to_funcs: HashMap, + index_to_funcs: Vec, call_graph: HashMap>, parents: HashMap>, relocation_map: HashMap>, @@ -1198,7 +1241,7 @@ struct ModuleWithRelocations<'a> { impl<'a> ModuleWithRelocations<'a> { fn new(bytes: &'a [u8]) -> Result { - let module = Module::from_buffer(bytes)?; + let (module, index_to_funcs, _) = parse_module_with_ids(bytes)?; let raw_data = parse_bytes_to_data_segment(bytes)?; let names_to_funcs = module .funcs @@ -1212,6 +1255,7 @@ impl<'a> ModuleWithRelocations<'a> { data_section_range: raw_data.data_range, symbols: raw_data.symbols, names_to_funcs, + index_to_funcs, call_graph: Default::default(), relocation_map: Default::default(), parents: Default::default(), @@ -1324,7 +1368,14 @@ impl<'a> ModuleWithRelocations<'a> { fn get_symbol_dep_node(&self, index: usize) -> Result> { let res = match self.symbols[index] { SymbolInfo::Data { .. } => Some(Node::DataSymbol(index)), - SymbolInfo::Func { name, .. } => Some(Node::Function({ + SymbolInfo::Func { index, name, .. } => Some(Node::Function({ + // Resolve by function index first: it is authoritative and survives + // the linker folding identical functions (which leaves symbol names + // in the symbol table that no longer appear in the name section). + if let Some(func_id) = self.index_to_funcs.get(index as usize) { + return Ok(Some(Node::Function(*func_id))); + } + let name = name.context( "Function symbol has no name - did you forget to enable debug symbols", )?; @@ -1547,3 +1598,37 @@ fn parse_bytes_to_data_segment(bytes: &[u8]) -> Result> { data_symbols, }) } + +#[cfg(test)] +mod tests { + /// `build_call_graph` bridges the original module's mangled name section to the + /// post-wasm-bindgen demangled one by demangling the original names with + /// `rustc_demangle::demangle`. wasm-bindgen produces the demangled names it + /// writes back, so our lookup key must be byte-for-byte identical to that output + /// (hash suffix included). If this ever diverges, the splitter degrades to an + /// empty call graph and starts carving main-module data into split chunks. + #[test] + fn demangled_names_match_wasm_bindgen_form() { + let cases = [ + ( + "_ZN12wasm_bindgen26__wbindgen_object_drop_ref17h958b79f87d38af82E", + "wasm_bindgen::__wbindgen_object_drop_ref::h958b79f87d38af82", + ), + ( + "_ZN4core3ptr13drop_in_place17h1234567890abcdefE", + "core::ptr::drop_in_place::h1234567890abcdef", + ), + ( + "_ZN7web_sys8features11gen_console7console5log_126__wbg_log_6614a4effdb4e98317hde81bf8087b7f274E", + "web_sys::features::gen_console::console::log_1::__wbg_log_6614a4effdb4e983::hde81bf8087b7f274", + ), + ]; + for (mangled, expected) in cases { + assert_eq!(rustc_demangle::demangle(mangled).to_string(), expected); + } + + // Non-mangled names (exports like `main`) must pass through unchanged so the + // direct name match keeps working. + assert_eq!(rustc_demangle::demangle("main").to_string(), "main"); + } +}