From bdd6be309e8fa4c05a03c5cfc164f7564e7c3051 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Thu, 4 Mar 2021 13:49:18 -0500 Subject: [PATCH] Feat: notes on safety, and inline listeners --- notes/SAFETY.md | 26 +++++++ packages/core/src/context.rs | 73 ++++++++---------- packages/core/src/nodebuilder.rs | 12 ++- packages/core/src/scope.rs | 122 ++++++++++++++++--------------- packages/core/src/virtual_dom.rs | 3 +- 5 files changed, 133 insertions(+), 103 deletions(-) create mode 100644 notes/SAFETY.md diff --git a/notes/SAFETY.md b/notes/SAFETY.md new file mode 100644 index 000000000..01358f710 --- /dev/null +++ b/notes/SAFETY.md @@ -0,0 +1,26 @@ +# Safety + +We don't claim to be a "safe" library. We want to be 100% safe, and will create tests and validate that we are, but our priorities for this library are: + +- productivity +- performance +- safety + +We are willing to use sharp tools (ie transmuting self-referential pointer types) in order to achieve higher ergonomics (ie returning self-referential listeners). + +However, we can only use these sharp tools if we verify that it's not possible to write user-facing code that breaks safety guarantees. For internal code... well, whoever contributes needs to understand the architecture and read the comments related to safety. + +We are doing one of the more annoying things to do with Rust: self-referential graph structures. VNodes reference a bump arena which is contained by scope. Conveniently, the bump arenas also belong to scope, and now we have a self-referential struct. + +We */would/* use a solution like ourborous or self_referential, but these libraries generate "imaginary code" that doesn't integrate with RA. It's simpler and easier to review if we set some rules on what is/isn't allowed. + +Here's the two main sources of unsafety: +- 1) vnodes, bump arenas, and scope +- 2) context_api and use_context + +For 1), we can fairly confidently guarantee safety by being careful about lifetime casts. + +For 2), use_context authors (mostly state management) can implement either the Unsafe API or the Safe API. The Safe API is less performant, but will likely do everything you need. The Unsafe API is more performant, + +Because of 2), we provide two state management solutions (D-Reducer and D-Dataflow) that use the Unsafe API, but will still be 100% safe. + diff --git a/packages/core/src/context.rs b/packages/core/src/context.rs index 184318e1d..4b6b46343 100644 --- a/packages/core/src/context.rs +++ b/packages/core/src/context.rs @@ -39,16 +39,14 @@ pub struct Context<'src> { pub(crate) final_nodes: Rc>>>, + pub listeners: &'src RefCell>, + // holder for the src lifetime // todo @jon remove this pub _p: std::marker::PhantomData<&'src ()>, } impl<'a> Context<'a> { - // pub fn props

() -> &'a P { - // todo!() - // } - // impl<'a, PropType> Context<'a, PropType> { /// Access the children elements passed into the component pub fn children(&self) -> Vec { @@ -73,7 +71,19 @@ impl<'a> Context<'a> { ) -> VNode<'a> { todo!() } +} +// NodeCtx is used to build VNodes in the component's memory space. +// This struct adds metadata to the final DomTree about listeners, attributes, and children +#[derive(Debug, Clone)] +pub struct NodeCtx<'a> { + pub bump: &'a Bump, + pub idx: RefCell, + pub scope: ScopeIdx, + pub listeners: &'a RefCell>, +} + +impl<'a> Context<'a> { /// Take a lazy VNode structure and actually build it with the context of the VDom's efficient VNode allocator. /// /// This function consumes the context and absorb the lifetime, so these VNodes *must* be returned. @@ -92,8 +102,9 @@ impl<'a> Context<'a> { pub fn render(self, lazy_nodes: impl FnOnce(&NodeCtx<'a>) -> VNode<'a> + 'a) -> DomTree { let ctx = NodeCtx { bump: self.bump, - idx: 0.into(), scope: self.scope, + idx: 0.into(), + listeners: self.listeners, }; let safe_nodes = lazy_nodes(&ctx); @@ -103,26 +114,8 @@ impl<'a> Context<'a> { } } -// NodeCtx is used to build VNodes in the component's memory space. -// This struct adds metadata to the final DomTree about listeners, attributes, and children -#[derive(Debug, Clone)] -pub struct NodeCtx<'a> { - pub bump: &'a Bump, - pub idx: RefCell, - pub scope: ScopeIdx, -} - -impl NodeCtx<'_> { - #[inline] - pub fn bump(&self) -> &Bump { - self.bump - } -} - +/// This module provides internal state management functionality for Dioxus components pub mod hooks { - //! This module provides internal state management functionality for Dioxus components - //! - use super::*; #[derive(Debug)] @@ -196,23 +189,21 @@ pub mod hooks { } } -mod context_api { - //! Context API - //! - //! The context API provides a mechanism for components to borrow state from other components higher in the tree. - //! By combining the Context API and the Subscription API, we can craft ergonomic global state management systems. - //! - //! This API is inherently dangerous because we could easily cause UB by allowing &T and &mut T to exist at the same time. - //! To prevent this, we expose the RemoteState and RemoteLock types which act as a form of reverse borrowing. - //! This is very similar to RwLock, except that RemoteState is copy-able. Unlike RwLock, derefing RemoteState can - //! cause panics if the pointer is null. In essence, we sacrifice the panic protection for ergonomics, but arrive at - //! a similar end result. - //! - //! Instead of placing the onus on the receiver of the data to use it properly, we wrap the source object in a - //! "shield" where gaining &mut access can only be done if no active StateGuards are open. This would fail and indicate - //! a failure of implementation. - //! - //! +/// Context API +/// +/// The context API provides a mechanism for components to borrow state from other components higher in the tree. +/// By combining the Context API and the Subscription API, we can craft ergonomic global state management systems. +/// +/// This API is inherently dangerous because we could easily cause UB by allowing &T and &mut T to exist at the same time. +/// To prevent this, we expose the RemoteState and RemoteLock types which act as a form of reverse borrowing. +/// This is very similar to RwLock, except that RemoteState is copy-able. Unlike RwLock, derefing RemoteState can +/// cause panics if the pointer is null. In essence, we sacrifice the panic protection for ergonomics, but arrive at +/// a similar end result. +/// +/// Instead of placing the onus on the receiver of the data to use it properly, we wrap the source object in a +/// "shield" where gaining &mut access can only be done if no active StateGuards are open. This would fail and indicate +/// a failure of implementation. +pub mod context_api { use std::ops::Deref; diff --git a/packages/core/src/nodebuilder.rs b/packages/core/src/nodebuilder.rs index dccb1e2d6..7d9cbff33 100644 --- a/packages/core/src/nodebuilder.rs +++ b/packages/core/src/nodebuilder.rs @@ -1,6 +1,6 @@ //! Helpers for building virtual DOM VNodes. -use std::ops::Deref; +use std::{borrow::BorrowMut, ops::Deref}; use crate::{ context::NodeCtx, @@ -344,6 +344,7 @@ where // todo: // increment listner id from nodectx ref // add listener attrs here instead of later? + self.listeners.push(Listener { event, callback: self.ctx.bump.alloc(callback), @@ -353,6 +354,15 @@ where // bump the context id forward *self.ctx.idx.borrow_mut() += 1; + + // Add this listener to the context list + // This casts the listener to a self-referential pointer + // This is okay because the bump arena is stable + self.listeners.last().map(|g| { + let r = unsafe { std::mem::transmute::<&Listener<'a>, &Listener<'static>>(g) }; + self.ctx.listeners.borrow_mut().push(r.callback as *const _); + }); + self } } diff --git a/packages/core/src/scope.rs b/packages/core/src/scope.rs index 55bf18e6a..b6edbb67b 100644 --- a/packages/core/src/scope.rs +++ b/packages/core/src/scope.rs @@ -3,18 +3,12 @@ use crate::context::hooks::Hook; use crate::innerlude::*; use crate::nodes::VNode; use bumpalo::Bump; -// use generational_arena::ScopeIdx; use std::{ - any::TypeId, - borrow::{Borrow, BorrowMut}, - cell::{RefCell, UnsafeCell}, - future::Future, + any::{Any, TypeId}, + cell::RefCell, marker::PhantomData, - ops::{Deref, DerefMut}, - sync::atomic::AtomicUsize, - sync::atomic::Ordering, - todo, + ops::Deref, }; /// Every component in Dioxus is represented by a `Scope`. @@ -24,31 +18,35 @@ use std::{ /// Scopes are allocated in a generational arena. As components are mounted/unmounted, they will replace slots of dead components. /// The actual contents of the hooks, though, will be allocated with the standard allocator. These should not allocate as frequently. pub struct Scope { - // pub(crate) struct Scope { - // TODO @Jon - // These hooks are actually references into the hook arena - // These two could be combined with "OwningRef" to remove unsafe usage - // could also use ourborous - pub hooks: RefCell>, - pub hook_arena: typed_arena::Arena, - // Map to the parent pub parent: Option, - pub frames: ActiveFrame, - - // List of listeners generated when CTX is evaluated - pub listeners: Vec<*const dyn Fn(crate::events::VirtualEvent)>, - // pub listeners: Vec<*const dyn Fn(crate::events::VirtualEvent)>, - // pub listeners: Vec>, - // lying, cheating reference >:( pub props: Box, // our own index pub myidx: ScopeIdx, - // pub props_type: TypeId, + // ========================== + // slightly unsafe stuff + // ========================== + // an internal, highly efficient storage of vnodes + pub frames: ActiveFrame, + + // These hooks are actually references into the hook arena + // These two could be combined with "OwningRef" to remove unsafe usage + // could also use ourborous + pub hooks: RefCell>, + pub hook_arena: typed_arena::Arena, + + // Unsafety: + // - is self-refenrential and therefore needs to point into the bump + // + // Stores references into the listeners attached to the vnodes + pub listeners: RefCell>, + + // Unsafety + // - is a raw ptr because we need to compare pub caller: *const (), } @@ -66,8 +64,7 @@ impl Scope { // Capture the caller let caller = f as *const (); - let listeners = vec![]; - // let listeners: Vec> = vec![]; + let listeners = Default::default(); let old_frame = BumpFrame { bump: Bump::new(), @@ -97,7 +94,21 @@ impl Scope { props, } } +} +pub struct RawComponent { + // used as a memoization strategy + comparator: *const Box) -> bool>, + + // used to actually run the component + // encapsulates props + runner: *const Box DomTree>, + + // the actual FC + raw: *const (), +} + +impl Scope { /// Create a new context and run the component with references from the Virtual Dom /// This function downcasts the function pointer based on the stored props_type /// @@ -106,11 +117,11 @@ impl Scope { let frame = { let frame = self.frames.next(); frame.bump.reset(); - log::debug!("Rednering into frame {:?}", frame as *const _); frame }; let node_slot = std::rc::Rc::new(RefCell::new(None)); + let ctx: Context<'bump> = Context { arena: &self.hook_arena, hooks: &self.hooks, @@ -119,6 +130,7 @@ impl Scope { _p: PhantomData {}, final_nodes: node_slot.clone(), scope: self.myidx, + listeners: &self.listeners, }; unsafe { @@ -139,7 +151,7 @@ impl Scope { let props = self.props.downcast_ref::().unwrap(); // Note that the actual modification of the vnode head element occurs during this call - let _nodes: DomTree = caller(ctx, props); + let _: DomTree = caller(ctx, props); /* SAFETY ALERT @@ -158,32 +170,8 @@ impl Scope { .borrow_mut() .take() .expect("Viewing did not happen"); - - // todo: - // make this so we dont have to iterate through the vnodes to get its listener - let mut listeners = vec![]; - retrieve_listeners(&frame.head_node, &mut listeners); - self.listeners = listeners - .into_iter() - .map(|f| { - let g = f.callback; - g as *const _ - }) - .collect(); - - // consume the listeners from the head_node into a list of boxed ref listeners } } - - /// Accessor to get the root node and its children (safely)\ - /// Scope is self-referntial, so we are forced to use the 'static lifetime to cheat - pub fn new_frame<'bump>(&'bump self) -> &'bump VNode<'bump> { - self.frames.current_head_node() - } - - pub fn old_frame<'bump>(&'bump self) -> &'bump VNode<'bump> { - self.frames.prev_head_node() - } } fn retrieve_listeners(node: &VNode<'static>, listeners: &mut Vec<&Listener>) { @@ -198,6 +186,21 @@ fn retrieve_listeners(node: &VNode<'static>, listeners: &mut Vec<&Listener>) { } } +// ========================== +// Active-frame related code +// ========================== +impl Scope { + /// Accessor to get the root node and its children (safely)\ + /// Scope is self-referntial, so we are forced to use the 'static lifetime to cheat + pub fn new_frame<'bump>(&'bump self) -> &'bump VNode<'bump> { + self.frames.current_head_node() + } + + pub fn old_frame<'bump>(&'bump self) -> &'bump VNode<'bump> { + self.frames.prev_head_node() + } +} + // todo, do better with the active frame stuff // somehow build this vnode with a lifetime tied to self // This root node has "static" lifetime, but it's really not static. @@ -205,7 +208,7 @@ fn retrieve_listeners(node: &VNode<'static>, listeners: &mut Vec<&Listener>) { // Use this node as if it were static is unsafe, and needs to be fixed with ourborous or owning ref // ! do not copy this reference are things WILL break ! pub struct ActiveFrame { - pub idx: AtomicUsize, + pub idx: RefCell, pub frames: [BumpFrame; 2], } @@ -223,7 +226,7 @@ impl ActiveFrame { } fn current_head_node<'b>(&'b self) -> &'b VNode<'b> { - let raw_node = match self.idx.borrow().load(Ordering::Relaxed) & 1 == 0 { + let raw_node = match *self.idx.borrow() & 1 == 0 { true => &self.frames[0], false => &self.frames[1], }; @@ -237,7 +240,7 @@ impl ActiveFrame { } fn prev_head_node<'b>(&'b self) -> &'b VNode<'b> { - let raw_node = match self.idx.borrow().load(Ordering::Relaxed) & 1 != 0 { + let raw_node = match *self.idx.borrow() & 1 != 0 { true => &self.frames[0], false => &self.frames[1], }; @@ -251,10 +254,9 @@ impl ActiveFrame { } fn next(&mut self) -> &mut BumpFrame { - self.idx.fetch_add(1, Ordering::Relaxed); - let cur = self.idx.borrow().load(Ordering::Relaxed); + *self.idx.borrow_mut() += 1; - if cur % 2 == 0 { + if *self.idx.borrow() % 2 == 0 { &mut self.frames[0] } else { &mut self.frames[1] @@ -262,7 +264,7 @@ impl ActiveFrame { } } -// #[cfg(test)] +#[cfg(test)] mod tests { use super::*; use crate::prelude::*; diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index be62bcc0a..20a249d78 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -136,11 +136,12 @@ impl VirtualDom { .get_mut(component_id) .expect("Component should exist if an event was triggered"); - log::debug!("list: {}", component.listeners.len()); + log::debug!("list: {}", component.listeners.borrow().len()); let listener = unsafe { component .listeners + .borrow() .get(listener_id as usize) .expect("Listener should exist if it was triggered") .as_ref()