From 934de21dd673b1b79904a3249998427f11428426 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sat, 18 Dec 2021 23:03:59 -0500 Subject: [PATCH] wip: miri stress tets --- Cargo.toml | 4 +- packages/core/examples/handlerror.rs | 9 +- packages/core/src/component.rs | 4 +- packages/core/src/diff.rs | 21 +-- packages/core/src/lazynodes.rs | 19 ++- packages/core/src/nodes.rs | 2 +- packages/core/src/scope.rs | 81 ++++++++++-- packages/core/src/scopearena.rs | 183 ++++++++++++++------------- packages/core/src/virtual_dom.rs | 151 +++++++++++----------- packages/core/tests/earlyabort.rs | 80 ++++++++++++ packages/core/tests/miri_stress.rs | 126 ++++++++++++++++++ packages/desktop/src/lib.rs | 2 + packages/router/Cargo.toml | 21 ++- packages/router/src/lib.rs | 143 ++------------------- packages/router/src/platform/web.rs | 126 ++++++++++++++++++ packages/router/src/utils.rs | 4 +- 16 files changed, 641 insertions(+), 335 deletions(-) create mode 100644 packages/core/tests/earlyabort.rs create mode 100644 packages/core/tests/miri_stress.rs diff --git a/Cargo.toml b/Cargo.toml index 349a17495..44750d463 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,8 +33,8 @@ html = ["dioxus-html"] router = ["dioxus-router"] liveview = ["dioxus-liveview"] ssr = ["dioxus-ssr"] -web = ["dioxus-web"] -desktop = ["dioxus-desktop"] +web = ["dioxus-web", "dioxus-router/web"] +desktop = ["dioxus-desktop", "dioxus-router/desktop"] mobile = ["dioxus-mobile"] diff --git a/packages/core/examples/handlerror.rs b/packages/core/examples/handlerror.rs index c3283c32b..0de55333e 100644 --- a/packages/core/examples/handlerror.rs +++ b/packages/core/examples/handlerror.rs @@ -7,4 +7,11 @@ use dioxus_core::prelude::*; use thiserror::Error; -fn main() {} +fn main() { + + /* + If a component returns None, do we leave it the same way it was before? + We spent all this time wiping the component + + */ +} diff --git a/packages/core/src/component.rs b/packages/core/src/component.rs index 75a3d4e17..71de3cf99 100644 --- a/packages/core/src/component.rs +++ b/packages/core/src/component.rs @@ -40,7 +40,7 @@ impl<'a, const A: bool> FragmentBuilder<'a, A> { /// ## Example /// /// ```rust, ignore -/// fn App(cx: Context, props: &()) -> Element { +/// fn App(cx: Scope<()>) -> Element { /// cx.render(rsx!{ /// CustomCard { /// h1 {}2 @@ -54,7 +54,7 @@ impl<'a, const A: bool> FragmentBuilder<'a, A> { /// children: Element /// } /// -/// fn CustomCard(cx: Context, props: &CardProps) -> Element { +/// fn CustomCard(cx: Scope) -> Element { /// cx.render(rsx!{ /// div { /// h1 {"Title card"} diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index b1d4818a2..7a5e00810 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -471,16 +471,16 @@ impl<'bump> DiffState<'bump> { ); // Run the scope for one iteration to initialize it - if self.scopes.run_scope(new_idx) { - // Take the node that was just generated from running the component - let nextnode = self.scopes.fin_head(new_idx); - self.stack.create_component(new_idx, nextnode); + self.scopes.run_scope(new_idx); - // todo: subtrees - // if new_component.is_subtree_root.get() { - // self.stack.push_subtree(); - // } - } + // Take the node that was just generated from running the component + let nextnode = self.scopes.fin_head(new_idx); + self.stack.create_component(new_idx, nextnode); + + // todo: subtrees + // if new_component.is_subtree_root.get() { + // self.stack.push_subtree(); + // } // Finally, insert this scope as a seen node. self.mutations.dirty_scopes.insert(new_idx); @@ -735,7 +735,8 @@ impl<'bump> DiffState<'bump> { // React doesn't automatically memoize, but we do. let props_are_the_same = old.comparator.unwrap(); - if (self.force_diff || !props_are_the_same(new)) && self.scopes.run_scope(scope_addr) { + if self.force_diff || !props_are_the_same(new) { + self.scopes.run_scope(scope_addr); self.diff_node( self.scopes.wip_head(scope_addr), self.scopes.fin_head(scope_addr), diff --git a/packages/core/src/lazynodes.rs b/packages/core/src/lazynodes.rs index fb474dc69..2d10622e3 100644 --- a/packages/core/src/lazynodes.rs +++ b/packages/core/src/lazynodes.rs @@ -35,7 +35,6 @@ enum StackNodeStorage<'a, 'b> { } impl<'a, 'b> LazyNodes<'a, 'b> { - pub fn new_some(_val: F) -> Option where F: FnOnce(NodeFactory<'a>) -> VNode<'a> + 'b, @@ -43,6 +42,24 @@ impl<'a, 'b> LazyNodes<'a, 'b> { Some(Self::new(_val)) } + /// force this call onto the stack + pub fn new_boxed(_val: F) -> Option + where + F: FnOnce(NodeFactory<'a>) -> VNode<'a> + 'b, + { + // there's no way to call FnOnce without a box, so we need to store it in a slot and use static dispatch + let mut slot = Some(_val); + + let val = move |fac: Option>| { + let inner = slot.take().unwrap(); + fac.map(inner) + }; + + Some(Self { + inner: StackNodeStorage::Heap(Box::new(val)), + }) + } + pub fn new(_val: F) -> Self where F: FnOnce(NodeFactory<'a>) -> VNode<'a> + 'b, diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index fd0fd3e3c..a6f288a52 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -83,7 +83,7 @@ pub enum VNode<'src> { /// # Example /// /// ```rust, ignore - /// fn Example(cx: Context, props: &()) -> Element { + /// fn Example(cx: Scope<()>) -> Element { /// todo!() /// } /// diff --git a/packages/core/src/scope.rs b/packages/core/src/scope.rs index 6289811e0..40bcd8aca 100644 --- a/packages/core/src/scope.rs +++ b/packages/core/src/scope.rs @@ -1,7 +1,6 @@ use crate::innerlude::*; use futures_channel::mpsc::UnboundedSender; -use smallvec::SmallVec; use std::{ any::{Any, TypeId}, cell::{Cell, RefCell}, @@ -27,7 +26,7 @@ use bumpalo::{boxed::Box as BumpBox, Bump}; /// name: String /// } /// -/// fn Example(cx: Context, props: &ExampleProps) -> Element { +/// fn Example(cx: Scope) -> Element { /// cx.render(rsx!{ div {"Hello, {cx.props.name}"} }) /// } /// ``` @@ -62,7 +61,7 @@ impl<'a, P> std::ops::Deref for Scope<'a, P> { #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] pub struct ScopeId(pub usize); -/// Every component in Dioxus is represented by a `Scope`. +/// Every component in Dioxus is represented by a `ScopeState`. /// /// Scopes contain the state for hooks, the component's props, and other lifecycle information. /// @@ -93,7 +92,9 @@ pub struct ScopeState { pub(crate) items: RefCell>, pub(crate) hook_arena: Bump, - pub(crate) hook_vals: RefCell>, + + pub(crate) hook_vals: RefCell>, + pub(crate) hook_idx: Cell, pub(crate) shared_contexts: RefCell>>, @@ -141,7 +142,7 @@ impl ScopeState { /// # Example /// /// ```rust, ignore - /// fn App(cx: Context, props: &()) -> Element { + /// fn App(cx: Scope<()>) -> Element { /// todo!(); /// rsx!(cx, div { "Subtree {id}"}) /// }; @@ -250,9 +251,8 @@ impl ScopeState { /// Get the Root Node of this scope pub fn root_node(&self) -> &VNode { - todo!("Portals have changed how we address nodes. Still fixing this, sorry."); - // let node = *self.wip_frame().nodes.borrow().get(0).unwrap(); - // unsafe { std::mem::transmute(&*node) } + let node = unsafe { &*self.wip_frame().node.get() }; + unsafe { std::mem::transmute(node) } } /// This method enables the ability to expose state to children further down the VirtualDOM Tree. @@ -329,6 +329,8 @@ impl ScopeState { let pinned_fut: Pin> = boxed_fut.into(); // erase the 'src lifetime for self-referential storage + // todo: provide a miri test around this + // concerned about segfaulting let self_ref_fut = unsafe { std::mem::transmute(pinned_fut) }; // Push the future into the tasks @@ -358,9 +360,8 @@ impl ScopeState { }; match rsx { Some(s) => Some(s.call(fac)), - None => todo!(), + None => todo!("oh no no nodes"), } - // rsx.map(|f| f.call(fac)) } /// Store a value between renders @@ -463,24 +464,78 @@ impl ScopeState { pub(crate) fn bump(&self) -> &Bump { &self.wip_frame().bump } + + pub(crate) fn drop_hooks(&mut self) { + self.hook_vals.get_mut().drain(..).for_each(|state| { + let as_mut = unsafe { &mut *state }; + let boxed = unsafe { bumpalo::boxed::Box::from_raw(as_mut) }; + drop(boxed); + }); + } + + pub(crate) fn reset(&mut self) { + // we're just reusing scopes so we need to clear it out + self.drop_hooks(); + + self.hook_idx.set(0); + + self.hook_arena.reset(); + self.shared_contexts.get_mut().clear(); + self.parent_scope = None; + self.generation.set(0); + self.is_subtree_root.set(false); + self.subtree.set(0); + + self.frames[0].reset(); + self.frames[1].reset(); + + let SelfReferentialItems { + borrowed_props, + listeners, + tasks, + } = self.items.get_mut(); + + borrowed_props.clear(); + listeners.clear(); + tasks.clear(); + } +} + +impl Drop for ScopeState { + fn drop(&mut self) { + self.drop_hooks(); + } } pub(crate) struct BumpFrame { pub bump: Bump, - pub nodes: Cell<*const VNode<'static>>, + pub node: Cell<*const VNode<'static>>, } impl BumpFrame { pub(crate) fn new(capacity: usize) -> Self { let bump = Bump::with_capacity(capacity); let node = &*bump.alloc(VText { - text: "asd", + text: "placeholdertext", dom_id: Default::default(), is_static: false, }); let node = bump.alloc(VNode::Text(unsafe { std::mem::transmute(node) })); let nodes = Cell::new(node as *const _); - Self { bump, nodes } + Self { bump, node: nodes } + } + + pub(crate) fn reset(&mut self) { + self.bump.reset(); + let node = self.bump.alloc(VText { + text: "placeholdertext", + dom_id: Default::default(), + is_static: false, + }); + let node = self + .bump + .alloc(VNode::Text(unsafe { std::mem::transmute(node) })); + self.node.set(node as *const _); } } diff --git a/packages/core/src/scopearena.rs b/packages/core/src/scopearena.rs index 9afb80866..a08455895 100644 --- a/packages/core/src/scopearena.rs +++ b/packages/core/src/scopearena.rs @@ -21,14 +21,15 @@ pub(crate) struct Heuristic { // // has an internal heuristics engine to pre-allocate arenas to the right size pub(crate) struct ScopeArena { - bump: Bump, pub pending_futures: RefCell>, scope_counter: Cell, + pub(crate) sender: UnboundedSender, + bump: Bump, + pub scopes: RefCell>, pub heuristics: RefCell>, free_scopes: RefCell>, nodes: RefCell>>, - pub(crate) sender: UnboundedSender, } impl ScopeArena { @@ -51,6 +52,7 @@ impl ScopeArena { let node = bump.alloc(VNode::Element(el)); let mut nodes = Slab::new(); let root_id = nodes.insert(unsafe { std::mem::transmute(node as *const _) }); + debug_assert_eq!(root_id, 0); Self { @@ -88,63 +90,73 @@ impl ScopeArena { let new_scope_id = ScopeId(self.scope_counter.get()); self.scope_counter.set(self.scope_counter.get() + 1); - if let Some(old_scope) = self.free_scopes.borrow_mut().pop() { - let scope = unsafe { &mut *old_scope }; + /* + This scopearena aggressively reuse old scopes when possible. + We try to minimize the new allocations for props/arenas. - scope.caller.set(caller); - scope.parent_scope = parent_scope; - scope.height = height; - scope.subtree = Cell::new(subtree); - scope.our_arena_idx = new_scope_id; - scope.container = container; + However, this will probably lead to some sort of fragmentation. + I'm not exactly sure how to improve this today. + */ + match self.free_scopes.borrow_mut().pop() { + // No free scope, make a new scope + None => { + let (node_capacity, hook_capacity) = self + .heuristics + .borrow() + .get(&fc_ptr) + .map(|h| (h.node_arena_size, h.hook_arena_size)) + .unwrap_or_default(); - let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); - debug_assert!(any_item.is_none()); - } else { - let (node_capacity, hook_capacity) = { - let heuristics = self.heuristics.borrow(); - if let Some(heuristic) = heuristics.get(&fc_ptr) { - (heuristic.node_arena_size, heuristic.hook_arena_size) - } else { - (0, 0) - } - }; + let frames = [BumpFrame::new(node_capacity), BumpFrame::new(node_capacity)]; - let frames = [BumpFrame::new(node_capacity), BumpFrame::new(node_capacity)]; + let scope = self.bump.alloc(ScopeState { + sender: self.sender.clone(), + container, + our_arena_idx: new_scope_id, + parent_scope, + height, + frames, + subtree: Cell::new(subtree), + is_subtree_root: Cell::new(false), - let scope = self.bump.alloc(ScopeState { - sender: self.sender.clone(), - container, - our_arena_idx: new_scope_id, - parent_scope, - height, - frames, - subtree: Cell::new(subtree), - is_subtree_root: Cell::new(false), + caller: Cell::new(caller), + generation: 0.into(), - caller: Cell::new(caller), - generation: 0.into(), + shared_contexts: Default::default(), - shared_contexts: Default::default(), + items: RefCell::new(SelfReferentialItems { + listeners: Default::default(), + borrowed_props: Default::default(), + tasks: Default::default(), + }), - items: RefCell::new(SelfReferentialItems { - listeners: Default::default(), - borrowed_props: Default::default(), - tasks: Default::default(), - }), + hook_arena: Bump::new(), + hook_vals: RefCell::new(Vec::with_capacity(hook_capacity)), + hook_idx: Default::default(), + }); - hook_arena: Bump::new(), - hook_vals: RefCell::new(smallvec::SmallVec::with_capacity(hook_capacity)), - hook_idx: Default::default(), - }); + let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); + debug_assert!(any_item.is_none()); + } - let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); - debug_assert!(any_item.is_none()); + // Reuse a free scope + Some(old_scope) => { + let scope = unsafe { &mut *old_scope }; + scope.caller.set(caller); + scope.parent_scope = parent_scope; + scope.height = height; + scope.subtree = Cell::new(subtree); + scope.our_arena_idx = new_scope_id; + scope.container = container; + let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); + debug_assert!(any_item.is_none()); + } } new_scope_id } + // Removes a scope and its descendents from the arena pub fn try_remove(&self, id: ScopeId) -> Option<()> { self.ensure_drop_safety(id); @@ -152,34 +164,7 @@ impl ScopeArena { // - ensure_drop_safety ensures that no references to this scope are in use // - this raw pointer is removed from the map let scope = unsafe { &mut *self.scopes.borrow_mut().remove(&id).unwrap() }; - - // we're just reusing scopes so we need to clear it out - scope.hook_vals.get_mut().drain(..).for_each(|state| { - let as_mut = unsafe { &mut *state }; - let boxed = unsafe { bumpalo::boxed::Box::from_raw(as_mut) }; - drop(boxed); - }); - scope.hook_idx.set(0); - scope.hook_arena.reset(); - - scope.shared_contexts.get_mut().clear(); - scope.parent_scope = None; - scope.generation.set(0); - scope.is_subtree_root.set(false); - scope.subtree.set(0); - - scope.frames[0].bump.reset(); - scope.frames[1].bump.reset(); - - let SelfReferentialItems { - borrowed_props, - listeners, - tasks, - } = scope.items.get_mut(); - - borrowed_props.clear(); - listeners.clear(); - tasks.clear(); + scope.reset(); self.free_scopes.borrow_mut().push(scope); @@ -245,7 +230,9 @@ impl ScopeArena { } } - pub(crate) fn run_scope(&self, id: ScopeId) -> bool { + // pub(crate) fn run_scope(&self, id: ScopeId) -> bool { + + pub(crate) fn run_scope(&self, id: ScopeId) { // Cycle to the next frame and then reset it // This breaks any latent references, invalidating every pointer referencing into it. // Remove all the outdated listeners @@ -278,6 +265,11 @@ impl ScopeArena { let render: &dyn Fn(&ScopeState) -> Element = unsafe { &*scope.caller.get() }; + /* + If the component returns None, then we fill in a placeholder node. This will wipe what was there. + + An alternate approach is to leave the Real Dom the same, but that can lead to safety issues and a lot more checks. + */ if let Some(node) = render(scope) { if !scope.items.borrow().tasks.is_empty() { self.pending_futures.borrow_mut().insert(id); @@ -285,19 +277,20 @@ impl ScopeArena { let frame = scope.wip_frame(); let node = frame.bump.alloc(node); - frame.nodes.set(unsafe { std::mem::transmute(node) }); - - // make the "wip frame" contents the "finished frame" - // any future dipping into completed nodes after "render" will go through "fin head" - scope.cycle_frame(); - true + frame.node.set(unsafe { std::mem::transmute(node) }); } else { - // the component bailed out early - // this leaves the wip frame and all descendents in a state where - // their WIP frames are invalid - // todo: descendents should not cause the app to crash - false + let frame = scope.wip_frame(); + let node = frame + .bump + .alloc(VNode::Placeholder(frame.bump.alloc(VPlaceholder { + dom_id: Default::default(), + }))); + frame.node.set(unsafe { std::mem::transmute(node) }); } + + // make the "wip frame" contents the "finished frame" + // any future dipping into completed nodes after "render" will go through "fin head" + scope.cycle_frame(); } pub fn call_listener_with_bubbling(&self, event: UserEvent, element: ElementId) { @@ -328,7 +321,7 @@ impl ScopeArena { pub fn wip_head(&self, id: ScopeId) -> &VNode { let scope = self.get_scope(id).unwrap(); let frame = scope.wip_frame(); - let node = unsafe { &*frame.nodes.get() }; + let node = unsafe { &*frame.node.get() }; unsafe { std::mem::transmute::<&VNode, &VNode>(node) } } @@ -336,7 +329,7 @@ impl ScopeArena { pub fn fin_head(&self, id: ScopeId) -> &VNode { let scope = self.get_scope(id).unwrap(); let frame = scope.fin_frame(); - let node = unsafe { &*frame.nodes.get() }; + let node = unsafe { &*frame.node.get() }; unsafe { std::mem::transmute::<&VNode, &VNode>(node) } } @@ -344,3 +337,19 @@ impl ScopeArena { self.fin_head(id) } } + +// when dropping the virtualdom, we need to make sure and drop everything important +impl Drop for ScopeArena { + fn drop(&mut self) { + for (_, scopeptr) in self.scopes.get_mut().drain() { + let scope = unsafe { bumpalo::boxed::Box::from_raw(scopeptr) }; + drop(scope); + } + + // these are probably complete invalid unfortunately ? + for scopeptr in self.free_scopes.get_mut().drain(..) { + let scope = unsafe { bumpalo::boxed::Box::from_raw(scopeptr) }; + drop(scope); + } + } +} diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 23d7570cc..13551d337 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -1,4 +1,4 @@ -//! # VirtualDOM Implementation for Rust +//! # VirtualDom Implementation for Rust //! //! This module provides the primary mechanics to create a hook-based, concurrent VDOM for Rust. @@ -23,7 +23,7 @@ use std::{any::Any, collections::VecDeque, pin::Pin, sync::Arc, task::Poll}; /// title: String /// } /// -/// fn App(cx: Context, props: &AppProps) -> Element { +/// fn App(cx: Scope) -> Element { /// cx.render(rsx!( /// div {"hello, {cx.props.title}"} /// )) @@ -33,7 +33,7 @@ use std::{any::Any, collections::VecDeque, pin::Pin, sync::Arc, task::Poll}; /// Components may be composed to make complex apps. /// /// ```rust, ignore -/// fn App(cx: Context, props: &AppProps) -> Element { +/// fn App(cx: Scope) -> Element { /// cx.render(rsx!( /// NavBar { routes: ROUTES } /// Title { "{cx.props.title}" } @@ -81,7 +81,7 @@ use std::{any::Any, collections::VecDeque, pin::Pin, sync::Arc, task::Poll}; /// Putting everything together, you can build an event loop around Dioxus by using the methods outlined above. /// /// ```rust, ignore -/// fn App(cx: Context, props: &()) -> Element { +/// fn App(cx: Scope<()>) -> Element { /// cx.render(rsx!{ /// div { "Hello World" } /// }) @@ -103,25 +103,24 @@ use std::{any::Any, collections::VecDeque, pin::Pin, sync::Arc, task::Poll}; /// } /// ``` pub struct VirtualDom { - base_scope: ScopeId, - - // it's stored here so the props are dropped when the VirtualDom is dropped - _root_caller: Box Fn(&'r ScopeState) -> Element<'r> + 'static>, - scopes: Box, - receiver: UnboundedReceiver, - - sender: UnboundedSender, - + // should always be ScopeId(0) + base_scope: ScopeId, pending_messages: VecDeque, - dirty_scopes: IndexSet, + + channel: ( + UnboundedSender, + UnboundedReceiver, + ), + + caller: *mut dyn for<'r> Fn(&'r ScopeState) -> Element<'r>, } // Methods to create the VirtualDom impl VirtualDom { - /// Create a new VirtualDOM with a component that does not have special props. + /// Create a new VirtualDom with a component that does not have special props. /// /// # Description /// @@ -140,12 +139,12 @@ impl VirtualDom { /// let dom = VirtualDom::new(Example); /// ``` /// - /// Note: the VirtualDOM is not progressed, you must either "run_with_deadline" or use "rebuild" to progress it. + /// Note: the VirtualDom is not progressed, you must either "run_with_deadline" or use "rebuild" to progress it. pub fn new(root: Component<()>) -> Self { Self::new_with_props(root, ()) } - /// Create a new VirtualDOM with the given properties for the root component. + /// Create a new VirtualDom with the given properties for the root component. /// /// # Description /// @@ -169,15 +168,18 @@ impl VirtualDom { /// let dom = VirtualDom::new(Example); /// ``` /// - /// Note: the VirtualDOM is not progressed on creation. You must either "run_with_deadline" or use "rebuild" to progress it. + /// Note: the VirtualDom is not progressed on creation. You must either "run_with_deadline" or use "rebuild" to progress it. /// /// ```rust, ignore /// let mut dom = VirtualDom::new_with_props(Example, SomeProps { name: "jane" }); /// let mutations = dom.rebuild(); /// ``` pub fn new_with_props(root: Component

, root_props: P) -> Self { - let (sender, receiver) = futures_channel::mpsc::unbounded::(); - Self::new_with_props_and_scheduler(root, root_props, sender, receiver) + Self::new_with_props_and_scheduler( + root, + root_props, + futures_channel::mpsc::unbounded::(), + ) } /// Launch the VirtualDom, but provide your own channel for receiving and sending messages into the scheduler @@ -192,26 +194,26 @@ impl VirtualDom { pub fn new_with_props_and_scheduler( root: Component

, root_props: P, - sender: UnboundedSender, - receiver: UnboundedReceiver, + channel: ( + UnboundedSender, + UnboundedReceiver, + ), ) -> Self { - // move these two things onto the heap so we have stable ptrs - let scopes = Box::new(ScopeArena::new(sender.clone())); + // move these two things onto the heap so we have stable ptrs even if the VirtualDom itself is moved + let scopes = Box::new(ScopeArena::new(channel.0.clone())); let root_props = Box::new(root_props); - // create the root caller which will properly drop its props when the VirtualDom is dropped - let mut _root_caller: Box Fn(&'r ScopeState) -> Element<'r> + 'static> = - Box::new(move |scope: &ScopeState| -> Element { + // effectively leak the caller so we can drop it when the VirtualDom is dropped + let caller: *mut dyn for<'r> Fn(&'r ScopeState) -> Element<'r> = + Box::into_raw(Box::new(move |scope: &ScopeState| -> Element { // Safety: The props at this pointer can never be moved. // Also, this closure will never be ran when the VirtualDom is destroyed. // This is where the root lifetime of the VirtualDom originates. - let props: *const P = root_props.as_ref(); - let props = unsafe { &*props }; - root(Scope { scope, props }) - }); + let props = unsafe { std::mem::transmute::<&P, &P>(root_props.as_ref()) }; + let scp = Scope { scope, props }; + root(scp) + })); - // safety: the raw pointer is aliased or used after this point. - let caller: *mut dyn Fn(&ScopeState) -> Element = _root_caller.as_mut(); let base_scope = scopes.new_with_key(root as _, caller, None, ElementId(0), 0, 0); let mut dirty_scopes = IndexSet::new(); @@ -223,11 +225,10 @@ impl VirtualDom { Self { scopes, base_scope, - receiver, - _root_caller, + caller, pending_messages, dirty_scopes, - sender, + channel, } } @@ -260,7 +261,7 @@ impl VirtualDom { /// let sender = dom.get_scheduler_channel(); /// ``` pub fn get_scheduler_channel(&self) -> futures_channel::mpsc::UnboundedSender { - self.sender.clone() + self.channel.0.clone() } /// Add a new message to the scheduler queue directly. @@ -275,7 +276,7 @@ impl VirtualDom { /// dom.insert_scheduler_message(SchedulerMsg::Immediate(ScopeId(0))); /// ``` pub fn insert_scheduler_message(&self, msg: SchedulerMsg) { - self.sender.unbounded_send(msg).unwrap() + self.channel.0.unbounded_send(msg).unwrap() } /// Check if the [`VirtualDom`] has any pending updates or work to be done. @@ -314,18 +315,18 @@ impl VirtualDom { if self.pending_messages.is_empty() { if self.scopes.pending_futures.borrow().is_empty() { self.pending_messages - .push_front(self.receiver.next().await.unwrap()); + .push_front(self.channel.1.next().await.unwrap()); } else { use futures_util::future::{select, Either}; - match select(PollTasks(&mut self.scopes), self.receiver.next()).await { + match select(PollTasks(&mut self.scopes), self.channel.1.next()).await { Either::Left((_, _)) => {} Either::Right((msg, _)) => self.pending_messages.push_front(msg.unwrap()), } } } - while let Ok(Some(msg)) = self.receiver.try_next() { + while let Ok(Some(msg)) = self.channel.1.try_next() { self.pending_messages.push_front(msg); } @@ -378,7 +379,7 @@ impl VirtualDom { /// # Example /// /// ```rust, ignore - /// fn App(cx: Context, props: &()) -> Element { + /// fn App(cx: Scope<()>) -> Element { /// cx.render(rsx!( div {"hello"} )) /// } /// @@ -416,15 +417,14 @@ impl VirtualDom { if !ran_scopes.contains(&scopeid) { ran_scopes.insert(scopeid); - if self.scopes.run_scope(scopeid) { - let (old, new) = - (self.scopes.wip_head(scopeid), self.scopes.fin_head(scopeid)); - diff_state.stack.push(DiffInstruction::Diff { new, old }); - diff_state.stack.scope_stack.push(scopeid); + self.scopes.run_scope(scopeid); - let scope = scopes.get_scope(scopeid).unwrap(); - diff_state.stack.element_stack.push(scope.container); - } + let (old, new) = (self.scopes.wip_head(scopeid), self.scopes.fin_head(scopeid)); + diff_state.stack.push(DiffInstruction::Diff { new, old }); + diff_state.stack.scope_stack.push(scopeid); + + let scope = scopes.get_scope(scopeid).unwrap(); + diff_state.stack.element_stack.push(scope.container); } } @@ -466,30 +466,29 @@ impl VirtualDom { /// apply_edits(edits); /// ``` pub fn rebuild(&mut self) -> Mutations { + let scope_id = self.base_scope; let mut diff_state = DiffState::new(&self.scopes); - let scope_id = self.base_scope; - if self.scopes.run_scope(scope_id) { - diff_state - .stack - .create_node(self.scopes.fin_head(scope_id), MountType::Append); - - diff_state.stack.element_stack.push(ElementId(0)); - diff_state.stack.scope_stack.push(scope_id); - - diff_state.work(|| false); - } + self.scopes.run_scope(scope_id); + diff_state + .stack + .create_node(self.scopes.fin_head(scope_id), MountType::Append); + diff_state.stack.element_stack.push(ElementId(0)); + diff_state.stack.scope_stack.push(scope_id); + diff_state.work(|| false); diff_state.mutations } - /// Compute a manual diff of the VirtualDOM between states. + /// Compute a manual diff of the VirtualDom between states. /// /// This can be useful when state inside the DOM is remotely changed from the outside, but not propagated as an event. /// /// In this case, every component will be diffed, even if their props are memoized. This method is intended to be used /// to force an update of the DOM when the state of the app is changed outside of the app. /// + /// To force a reflow of the entire VirtualDom, use `ScopeId(0)` as the scope_id. + /// /// # Example /// ```rust, ignore /// #[derive(PartialEq, Props)] @@ -513,10 +512,9 @@ impl VirtualDom { /// ``` pub fn hard_diff<'a>(&'a mut self, scope_id: ScopeId) -> Option> { let mut diff_machine = DiffState::new(&self.scopes); - if self.scopes.run_scope(scope_id) { - diff_machine.force_diff = true; - diff_machine.diff_scope(scope_id); - } + self.scopes.run_scope(scope_id); + diff_machine.force_diff = true; + diff_machine.diff_scope(scope_id); Some(diff_machine.mutations) } @@ -525,7 +523,7 @@ impl VirtualDom { /// Useful when needing to render nodes from outside the VirtualDom, such as in a test. /// /// ```rust - /// fn Base(cx: Context, props: &()) -> Element { + /// fn Base(cx: Scope<()>) -> Element { /// rsx!(cx, div {}) /// } /// @@ -545,7 +543,7 @@ impl VirtualDom { /// Useful when needing to render nodes from outside the VirtualDom, such as in a test. /// /// ```rust - /// fn Base(cx: Context, props: &()) -> Element { + /// fn Base(cx: Scope<()>) -> Element { /// rsx!(cx, div {}) /// } /// @@ -567,7 +565,7 @@ impl VirtualDom { /// /// /// ```rust - /// fn Base(cx: Context, props: &()) -> Element { + /// fn Base(cx: Scope<()>) -> Element { /// rsx!(cx, div {}) /// } /// @@ -589,7 +587,7 @@ impl VirtualDom { /// /// /// ```rust - /// fn Base(cx: Context, props: &()) -> Element { + /// fn Base(cx: Scope<()>) -> Element { /// rsx!(cx, div {}) /// } /// @@ -619,6 +617,17 @@ impl VirtualDom { } } +impl Drop for VirtualDom { + fn drop(&mut self) { + // ensure the root component's caller is dropped + let caller = unsafe { Box::from_raw(self.caller) }; + drop(caller); + + // destroy the root component (which will destroy all of the other scopes) + // load the root node and descend + } +} + #[derive(Debug)] pub enum SchedulerMsg { // events from the host @@ -644,7 +653,7 @@ pub enum SchedulerMsg { /// /// # Example /// ```rust -/// fn App(cx: Context, props: &()) -> Element { +/// fn App(cx: Scope<()>) -> Element { /// rsx!(cx, div { /// onclick: move |_| println!("Clicked!") /// }) diff --git a/packages/core/tests/earlyabort.rs b/packages/core/tests/earlyabort.rs new file mode 100644 index 000000000..713fe7ccb --- /dev/null +++ b/packages/core/tests/earlyabort.rs @@ -0,0 +1,80 @@ +#![allow(unused, non_upper_case_globals, non_snake_case)] + +//! Prove that the dom works normally through virtualdom methods. +//! +//! This methods all use "rebuild" which completely bypasses the scheduler. +//! Hard rebuilds don't consume any events from the event queue. + +use dioxus::{prelude::*, DomEdit, ScopeId}; +use dioxus_core as dioxus; +use dioxus_core_macro::*; +use dioxus_html as dioxus_elements; + +mod test_logging; +use DomEdit::*; + +const IS_LOGGING_ENABLED: bool = false; + +fn new_dom(app: Component

, props: P) -> VirtualDom { + test_logging::set_up_logging(IS_LOGGING_ENABLED); + VirtualDom::new_with_props(app, props) +} + +/// This test ensures that if a component aborts early, it is replaced with a placeholder. +/// In debug, this should also toss a warning. +#[test] +fn test_early_abort() { + const app: Component<()> = |cx| { + let val = cx.use_hook(|_| 0, |f| f); + + *val += 1; + + if *val == 2 { + return None; + } + + rsx!(cx, div { "Hello, world!" }) + }; + + let mut dom = new_dom(app, ()); + + let edits = dom.rebuild(); + assert_eq!( + edits.edits, + [ + CreateElement { + tag: "div", + root: 1, + }, + CreateTextNode { + text: "Hello, world!", + root: 2, + }, + AppendChildren { many: 1 }, + AppendChildren { many: 1 }, + ] + ); + + let edits = dom.hard_diff(ScopeId(0)).unwrap(); + assert_eq!( + edits.edits, + [CreatePlaceholder { root: 3 }, ReplaceWith { root: 1, m: 1 },], + ); + + let edits = dom.hard_diff(ScopeId(0)).unwrap(); + assert_eq!( + edits.edits, + [ + CreateElement { + tag: "div", + root: 2, + }, + CreateTextNode { + text: "Hello, world!", + root: 4, + }, + AppendChildren { many: 1 }, + ReplaceWith { root: 3, m: 1 }, + ] + ); +} diff --git a/packages/core/tests/miri_stress.rs b/packages/core/tests/miri_stress.rs new file mode 100644 index 000000000..c51daa536 --- /dev/null +++ b/packages/core/tests/miri_stress.rs @@ -0,0 +1,126 @@ +/* +Stress Miri as much as possible. + +Prove that we don't leak memory and that our methods are safe. + +Specifically: +- [ ] VirtualDom drops memory safely +- [ ] Borrowed components don't expose invalid pointers +- [ ] Async isn't busted +*/ + +use dioxus::{prelude::*, DomEdit, ScopeId}; +use dioxus_core as dioxus; +use dioxus_core_macro::*; +use dioxus_html as dioxus_elements; + +mod test_logging; +use DomEdit::*; + +const IS_LOGGING_ENABLED: bool = false; + +fn new_dom(app: Component

, props: P) -> VirtualDom { + test_logging::set_up_logging(IS_LOGGING_ENABLED); + VirtualDom::new_with_props(app, props) +} + +/// This test ensures that if a component aborts early, it is replaced with a placeholder. +/// In debug, this should also toss a warning. +#[test] +fn test_memory_leak() { + fn app(cx: Scope<()>) -> Element { + let val = cx.use_hook(|_| 0, |f| f); + + *val += 1; + + if *val == 2 || *val == 4 { + return None; + } + + let name = cx.use_hook(|_| String::from("asd"), |f| f); + + cx.render(rsx!( + div { "Hello, world!" } + child() + child() + child() + child() + child() + child() + borrowed_child(na: name) + borrowed_child(na: name) + borrowed_child(na: name) + borrowed_child(na: name) + borrowed_child(na: name) + )) + } + + fn child(cx: Scope<()>) -> Element { + rsx!(cx, div { + "goodbye world" + }) + } + + #[derive(Props)] + struct BorrowedProps<'a> { + na: &'a str, + } + + fn borrowed_child<'a>(cx: Scope<'a, BorrowedProps<'a>>) -> Element { + rsx!(cx, div { + "goodbye {cx.props.na}" + child() + child() + }) + } + + let mut dom = new_dom(app, ()); + + dom.rebuild(); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); +} + +#[test] +fn memo_works_properly() { + fn app(cx: Scope<()>) -> Element { + let val = cx.use_hook(|_| 0, |f| f); + + *val += 1; + + if *val == 2 || *val == 4 { + return None; + } + + let name = cx.use_hook(|_| String::from("asd"), |f| f); + + cx.render(rsx!( + div { "Hello, world!" } + child(na: "asd".to_string()) + )) + } + + #[derive(PartialEq, Props)] + struct ChildProps { + na: String, + } + + fn child(cx: Scope) -> Element { + rsx!(cx, div { + "goodbye world" + }) + } + + let mut dom = new_dom(app, ()); + + dom.rebuild(); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); +} diff --git a/packages/desktop/src/lib.rs b/packages/desktop/src/lib.rs index 7de3bdd8c..2e962f800 100644 --- a/packages/desktop/src/lib.rs +++ b/packages/desktop/src/lib.rs @@ -64,6 +64,8 @@ pub fn run( let modifiers = ModifiersState::default(); let event_loop = EventLoop::new(); + log::debug!("Starting event loop"); + event_loop.run(move |window_event, event_loop, control_flow| { *control_flow = ControlFlow::Wait; diff --git a/packages/router/Cargo.toml b/packages/router/Cargo.toml index e12468436..41822b97f 100644 --- a/packages/router/Cargo.toml +++ b/packages/router/Cargo.toml @@ -15,6 +15,11 @@ dioxus-core = { path = "../core", version = "^0.1.3", default-features = false } dioxus-html = { path = "../html", version = "^0.1.0", default-features = false } dioxus-core-macro = { path = "../core-macro", version = "^0.1.2" } +serde = "1.0.130" +url = "2.2.2" +url_serde = "0.2.0" +serde_urlencoded = "0.7" + web-sys = { version = "0.3", features = [ "Attr", "Document", @@ -26,21 +31,13 @@ web-sys = { version = "0.3", features = [ "UrlSearchParams", "Window", ], optional = true } - -serde = "1.0.130" - -serde_urlencoded = "0.7" - -wasm-bindgen = "0.2" -js-sys = "0.3" -gloo = "0.4" -route-recognizer = "0.3.1" -url = "2.2.2" -url_serde = "0.2.0" +wasm-bindgen = { version = "0.2", optional = true } +js-sys = { version = "0.3", optional = true } +gloo = { version = "0.4", optional = true } [features] -default = ["web", "derive"] +default = ["derive"] web = ["web-sys"] desktop = [] mobile = [] diff --git a/packages/router/src/lib.rs b/packages/router/src/lib.rs index cadfb8218..0ce88d59b 100644 --- a/packages/router/src/lib.rs +++ b/packages/router/src/lib.rs @@ -6,137 +6,13 @@ use dioxus_core as dioxus; use dioxus_core::prelude::*; use dioxus_core_macro::{rsx, Props}; use dioxus_html as dioxus_elements; -use wasm_bindgen::{JsCast, JsValue}; -use web_sys::{window, Event}; +// use wasm_bindgen::{JsCast, JsValue}; use crate::utils::strip_slash_suffix; pub trait Routable: 'static + Send + Clone + PartialEq {} impl Routable for T where T: 'static + Send + Clone + PartialEq {} -pub struct RouterService { - historic_routes: Vec, - history_service: RefCell, - base_ur: RefCell>, -} - -impl RouterService { - fn push_route(&self, r: R) { - todo!() - // self.historic_routes.borrow_mut().push(r); - } - - fn get_current_route(&self) -> &str { - todo!() - } - - fn update_route_impl(&self, url: String, push: bool) { - let history = web_sys::window().unwrap().history().expect("no history"); - let base = self.base_ur.borrow(); - let path = match base.as_ref() { - Some(base) => { - let path = format!("{}{}", base, url); - if path.is_empty() { - "/".to_string() - } else { - path - } - } - None => url, - }; - - if push { - history - .push_state_with_url(&JsValue::NULL, "", Some(&path)) - .expect("push history"); - } else { - history - .replace_state_with_url(&JsValue::NULL, "", Some(&path)) - .expect("replace history"); - } - let event = Event::new("popstate").unwrap(); - - web_sys::window() - .unwrap() - .dispatch_event(&event) - .expect("dispatch"); - } -} - -/// This hould only be used once per app -/// -/// You can manually parse the route if you want, but the derived `parse` method on `Routable` will also work just fine -pub fn use_router(cx: &ScopeState, mut parse: impl FnMut(&str) -> R + 'static) -> &R { - // for the web, attach to the history api - cx.use_hook( - |f| { - // - use gloo::events::EventListener; - - let base = window() - .unwrap() - .document() - .unwrap() - .query_selector("base[href]") - .ok() - .flatten() - .and_then(|base| { - let base = JsCast::unchecked_into::(base).href(); - let url = web_sys::Url::new(&base).unwrap(); - - if url.pathname() != "/" { - Some(strip_slash_suffix(&base).to_string()) - } else { - None - } - }); - - let location = window().unwrap().location(); - let pathname = location.pathname().unwrap(); - let initial_route = parse(&pathname); - - let service: RouterService = RouterService { - historic_routes: vec![initial_route], - history_service: RefCell::new( - web_sys::window().unwrap().history().expect("no history"), - ), - base_ur: RefCell::new(base), - }; - - // let base = base_url(); - // let url = route.to_path(); - // pending_routes: RefCell::new(vec![]), - // service.history_service.push_state(data, title); - - // cx.provide_state(service); - - let regenerate = cx.schedule_update(); - - // // when "back" is called by the user, we want to to re-render the component - let listener = EventListener::new(&web_sys::window().unwrap(), "popstate", move |_| { - // - regenerate(); - }); - - service - }, - |state| { - let base = state.base_ur.borrow(); - if let Some(base) = base.as_ref() { - // - let path = format!("{}{}", base, state.get_current_route()); - } - let history = state.history_service.borrow(); - - state.historic_routes.last().unwrap() - }, - ) -} - -pub fn use_router_service(cx: &ScopeState) -> Option<&Rc>> { - cx.use_hook(|_| cx.consume_state::>(), |f| f.as_ref()) -} - #[derive(Props)] pub struct LinkProps<'a, R: Routable> { to: R, @@ -161,12 +37,13 @@ pub struct LinkProps<'a, R: Routable> { } pub fn Link<'a, R: Routable>(cx: Scope<'a, LinkProps<'a, R>>) -> Element { - let service = use_router_service::(&cx)?; - cx.render(rsx! { - a { - href: format_args!("{}", (cx.props.href)(&cx.props.to)), - onclick: move |_| service.push_route(cx.props.to.clone()), - // todo!() {&cx.props.children}, - } - }) + let service = todo!(); + // let service: todo!() = use_router_service::(&cx)?; + // cx.render(rsx! { + // a { + // href: format_args!("{}", (cx.props.href)(&cx.props.to)), + // onclick: move |_| service.push_route(cx.props.to.clone()), + // // todo!() {&cx.props.children}, + // } + // }) } diff --git a/packages/router/src/platform/web.rs b/packages/router/src/platform/web.rs index e69de29bb..3630fec85 100644 --- a/packages/router/src/platform/web.rs +++ b/packages/router/src/platform/web.rs @@ -0,0 +1,126 @@ +use web_sys::{window, Event}; +pub struct RouterService { + historic_routes: Vec, + history_service: RefCell, + base_ur: RefCell>, +} + +impl RouterService { + fn push_route(&self, r: R) { + todo!() + // self.historic_routes.borrow_mut().push(r); + } + + fn get_current_route(&self) -> &str { + todo!() + } + + fn update_route_impl(&self, url: String, push: bool) { + let history = web_sys::window().unwrap().history().expect("no history"); + let base = self.base_ur.borrow(); + let path = match base.as_ref() { + Some(base) => { + let path = format!("{}{}", base, url); + if path.is_empty() { + "/".to_string() + } else { + path + } + } + None => url, + }; + + if push { + history + .push_state_with_url(&JsValue::NULL, "", Some(&path)) + .expect("push history"); + } else { + history + .replace_state_with_url(&JsValue::NULL, "", Some(&path)) + .expect("replace history"); + } + let event = Event::new("popstate").unwrap(); + + web_sys::window() + .unwrap() + .dispatch_event(&event) + .expect("dispatch"); + } +} + +pub fn use_router_service(cx: &ScopeState) -> Option<&Rc>> { + cx.use_hook(|_| cx.consume_state::>(), |f| f.as_ref()) +} + +/// This hould only be used once per app +/// +/// You can manually parse the route if you want, but the derived `parse` method on `Routable` will also work just fine +pub fn use_router(cx: &ScopeState, mut parse: impl FnMut(&str) -> R + 'static) -> &R { + // for the web, attach to the history api + cx.use_hook( + #[cfg(not(feature = "web"))] + |_| {}, + #[cfg(feature = "web")] + |f| { + // + use gloo::events::EventListener; + + let base = window() + .unwrap() + .document() + .unwrap() + .query_selector("base[href]") + .ok() + .flatten() + .and_then(|base| { + let base = JsCast::unchecked_into::(base).href(); + let url = web_sys::Url::new(&base).unwrap(); + + if url.pathname() != "/" { + Some(strip_slash_suffix(&base).to_string()) + } else { + None + } + }); + + let location = window().unwrap().location(); + let pathname = location.pathname().unwrap(); + let initial_route = parse(&pathname); + + let service: RouterService = RouterService { + historic_routes: vec![initial_route], + history_service: RefCell::new( + web_sys::window().unwrap().history().expect("no history"), + ), + base_ur: RefCell::new(base), + }; + + // let base = base_url(); + // let url = route.to_path(); + // pending_routes: RefCell::new(vec![]), + // service.history_service.push_state(data, title); + + // cx.provide_state(service); + + let regenerate = cx.schedule_update(); + + // // when "back" is called by the user, we want to to re-render the component + let listener = EventListener::new(&web_sys::window().unwrap(), "popstate", move |_| { + // + regenerate(); + }); + + service + }, + |state| { + let base = state.base_ur.borrow(); + if let Some(base) = base.as_ref() { + // + let path = format!("{}{}", base, state.get_current_route()); + } + let history = state.history_service.borrow(); + + state.historic_routes.last().unwrap() + }, + ) +} diff --git a/packages/router/src/utils.rs b/packages/router/src/utils.rs index a8e9a2c35..48afe0d80 100644 --- a/packages/router/src/utils.rs +++ b/packages/router/src/utils.rs @@ -1,5 +1,5 @@ -use wasm_bindgen::JsCast; -use web_sys::window; +// use wasm_bindgen::JsCast; +// use web_sys::window; pub(crate) fn strip_slash_suffix(path: &str) -> &str { path.strip_suffix('/').unwrap_or(path)