Merge pull request #8 from Ivy-Interactive/tendril/00246-ReplaceRawPointersInBuildContextWithSafeAlternative

[00246] Replace raw pointers in BuildContext with safe alternative

Author: rorychatt

rusty/src/core/runtime.rs

@@ -86,12 +86,15 @@ impl Runtime {
             let mut ctx = BuildContext::with_view_id(store, Some(rebuild_tx), view_id);
             ctx.reset();
 
-            // Build the view
-            let view_node = self.view_tree.get(&view_id).expect("view_id not in tree");
-            // SAFETY: We need to borrow view immutably while store is borrowed mutably.
-            // The view reference does not alias with the store reference.
-            let view_ptr = &*view_node.view as *const dyn View;
-            let element = unsafe { &*view_ptr }.build(&mut ctx);
+            // Clone the Arc<dyn View> so we can borrow view immutably while
+            // store is borrowed mutably — no raw pointer needed.
+            let view_clone = self
+                .view_tree
+                .get(&view_id)
+                .expect("view_id not in tree")
+                .view
+                .clone();
+            let element = view_clone.build(&mut ctx);
 
             let mut element = element;
             element.assign_ids(&mut ctx);

rusty/src/core/view_tree.rs

@@ -1,12 +1,13 @@
 use std::collections::HashMap;
+use std::sync::Arc;
 
 use crate::shared::ViewId;
 use crate::views::view::View;
 
 /// A node in the view tree, representing a single view with its parent/child relationships.
 pub struct ViewNode {
     pub view_id: ViewId,
-    pub view: Box<dyn View>,
+    pub view: Arc<dyn View>,
     pub parent: Option<ViewId>,
     pub children: Vec<ViewId>,
 }
@@ -28,7 +29,7 @@ impl ViewTree {
         let root_id = uuid::Uuid::new_v4();
         let root_node = ViewNode {
             view_id: root_id,
-            view: root_view,
+            view: Arc::from(root_view),
             parent: None,
             children: Vec::new(),
         };
@@ -47,7 +48,7 @@ impl ViewTree {
         let child_id = uuid::Uuid::new_v4();
         let child_node = ViewNode {
             view_id: child_id,
-            view,
+            view: Arc::from(view),
             parent: Some(parent_id),
             children: Vec::new(),
         };
@@ -67,7 +68,7 @@ impl ViewTree {
     ) -> ViewId {
         let child_node = ViewNode {
             view_id: child_id,
-            view,
+            view: Arc::from(view),
             parent: Some(parent_id),
             children: Vec::new(),
         };

rusty/src/hooks/hook_store.rs

@@ -1,5 +1,6 @@
 use std::any::Any;
 use std::collections::HashMap;
+use std::sync::Arc;
 
 use super::deps::DynEq;
 use crate::views::view::EffectCleanup;
@@ -33,8 +34,9 @@ pub struct HookStore {
     pub effects: HashMap<usize, EffectEntry>,
     /// Cached memo values keyed by hook index.
     pub memos: HashMap<usize, MemoEntry>,
-    /// Context values keyed by TypeId name (for use_context).
-    pub contexts: HashMap<std::any::TypeId, Box<dyn Any + Send + Sync>>,
+    /// Context values keyed by TypeId (for use_context). Uses `Arc` so ancestor
+    /// context snapshots can be cheaply cloned without raw pointers.
+    pub contexts: HashMap<std::any::TypeId, Arc<dyn Any + Send + Sync>>,
 }
 
 impl HookStore {

rusty/src/hooks/use_context.rs

@@ -1,5 +1,6 @@
 use crate::views::view::BuildContext;
 use std::any::TypeId;
+use std::sync::Arc;
 
 /// Register a context value of type `T` for descendant views to access.
 ///
@@ -8,7 +9,7 @@ use std::any::TypeId;
 /// any descendant that calls `use_context::<T>()`.
 pub fn create_context<T: Send + Sync + Clone + 'static>(ctx: &mut BuildContext, value: T) {
     let type_id = TypeId::of::<T>();
-    ctx.store.contexts.insert(type_id, Box::new(value));
+    ctx.store.contexts.insert(type_id, Arc::new(value));
 }
 
 /// Retrieve a context value of type `T` from the current view or an ancestor's context.
@@ -80,4 +81,62 @@ mod tests {
         let mut ctx = BuildContext::new(&mut store, None);
         let _: Theme = use_context(&mut ctx);
     }
+
+    #[test]
+    fn test_ancestor_context_snapshot_isolation() {
+        use crate::views::view::{Element, View};
+        use crate::widgets::text::TextBlock;
+        use std::sync::Mutex;
+
+        static CHILD_RESULT: Mutex<Option<String>> = Mutex::new(None);
+
+        struct ChildView;
+        impl View for ChildView {
+            fn build(&self, ctx: &mut BuildContext) -> Element {
+                let theme: Theme = use_context(ctx);
+                *CHILD_RESULT.lock().unwrap() = Some(theme.primary_color.clone());
+                Element::Widget(Box::new(TextBlock::new(&theme.primary_color)))
+            }
+        }
+
+        let mut parent_store = HookStore::new();
+        let parent_view_id = uuid::Uuid::new_v4();
+        let mut ctx = BuildContext::with_view_id(&mut parent_store, None, parent_view_id);
+
+        // Parent creates context with "blue"
+        create_context(
+            &mut ctx,
+            Theme {
+                primary_color: "blue".to_string(),
+            },
+        );
+
+        // Child reads the context — gets a snapshot
+        let (_element, _child_id, _child_store) = ctx.child_view(ChildView, None);
+        let child_saw = CHILD_RESULT.lock().unwrap().take().unwrap();
+        assert_eq!(child_saw, "blue");
+
+        // Parent modifies its context to "red" after child was built
+        create_context(
+            &mut ctx,
+            Theme {
+                primary_color: "red".to_string(),
+            },
+        );
+
+        // Verify parent's store changed
+        let parent_theme = ctx
+            .store
+            .contexts
+            .get(&TypeId::of::<Theme>())
+            .unwrap()
+            .downcast_ref::<Theme>()
+            .unwrap();
+        assert_eq!(parent_theme.primary_color, "red");
+
+        // Build another child — it should see "red" since snapshot is taken at child_view() time
+        let (_element2, _child_id2, _child_store2) = ctx.child_view(ChildView, None);
+        let child2_saw = CHILD_RESULT.lock().unwrap().take().unwrap();
+        assert_eq!(child2_saw, "red");
+    }
 }

rusty/src/views/view.rs

@@ -1,8 +1,10 @@
 use serde::{Deserialize, Serialize};
-use std::any::Any;
+use std::any::{Any, TypeId};
 use std::collections::hash_map::DefaultHasher;
+use std::collections::HashMap;
 use std::fmt::Debug;
 use std::hash::{Hash, Hasher};
+use std::sync::Arc;
 
 use crate::core::event_registry::{EventCallback, EventRegistry};
 use crate::hooks::hook_store::HookStore;
@@ -112,6 +114,9 @@ impl Element {
     }
 }
 
+/// A snapshot of a view's context map, cheaply clonable via `Arc`.
+pub type ContextSnapshot = HashMap<TypeId, Arc<dyn Any + Send + Sync>>;
+
 /// A stateful component that produces an element tree.
 pub trait View: Send + Sync + 'static {
     /// Build the element tree for this view.
@@ -134,9 +139,9 @@ pub struct BuildContext<'a> {
     widget_id_counter: usize,
     /// Child views registered during this build via `child_view()`.
     pub(crate) child_views: Vec<ChildViewEntry>,
-    /// Access to ancestor HookStores for context lookup (read-only view).
-    /// Maps ViewId -> reference to ancestor contexts.
-    pub(crate) ancestor_contexts: Vec<(ViewId, *const HookStore)>,
+    /// Cloned snapshots of ancestor context maps for safe context lookup.
+    /// Each entry is a (ViewId, context map) pair, cheaply cloned via Arc.
+    pub(crate) ancestor_contexts: Vec<(ViewId, ContextSnapshot)>,
 }
 
 /// Cleanup function returned by an effect callback.
@@ -292,11 +297,12 @@ impl<'a> BuildContext<'a> {
         let mut child_ctx =
             BuildContext::with_view_id(&mut owned_store, self.rebuild_tx.clone(), child_view_id);
         child_ctx.reset();
-        // Set ancestor contexts: current view's store + all ancestors above
+        // Set ancestor contexts: current view's store + all ancestors above.
+        // Clone via Arc::clone per entry (cheap reference count bump).
         child_ctx.ancestor_contexts = self.ancestor_contexts.clone();
         child_ctx
             .ancestor_contexts
-            .push((self.current_view_id, self.store as *const HookStore));
+            .push((self.current_view_id, self.store.contexts.clone()));
 
         let mut element = view.build(&mut child_ctx);
         element.assign_ids(&mut child_ctx);
@@ -329,12 +335,9 @@ impl<'a> BuildContext<'a> {
         if let Some(val) = self.store.contexts.get(&type_id) {
             return Some(val.as_ref());
         }
-        // Walk ancestors from nearest to farthest
-        for (_view_id, store_ptr) in self.ancestor_contexts.iter().rev() {
-            // SAFETY: ancestor_contexts are set up during build traversal and the
-            // stores remain valid for the duration of the build
-            let store = unsafe { &**store_ptr };
-            if let Some(val) = store.contexts.get(&type_id) {
+        // Walk ancestors from nearest to farthest (safe — no raw pointers)
+        for (_view_id, contexts) in self.ancestor_contexts.iter().rev() {
+            if let Some(val) = contexts.get(&type_id) {
                 return Some(val.as_ref());
             }
         }