swaits/dioxus
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #1700 from ealmloff/fix-leak

Browse Source 
Fix leak in the render macro
This commit is contained in:
Jonathan Kelley authored and GitHub committed 2024-01-08 19:51:17 -08:00
2 parents a0ce5493ea b8fac44537
commit dd4dc93578
9 files changed
+115 -85
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

packages/core/src/arena.rs
+9 -1
View File
@@ -1,3 +1,4 @@
use crate::ScopeState;
use std::ptr::NonNull; use std::ptr::NonNull;
use crate::{ use crate::{
@@ -174,7 +175,14 @@ impl VirtualDom {
scope.borrowed_props.borrow_mut().clear(); scope.borrowed_props.borrow_mut().clear();
// Now that all the references are gone, we can safely drop our own references in our listeners. // Now that all the references are gone, we can safely drop our own references in our listeners.
let mut listeners = scope.attributes_to_drop_before_render.borrow_mut(); scope.drop_listeners();
}
}
impl ScopeState {
/// Drop all listeners that were allocated in the bump allocator.
pub(crate) fn drop_listeners(&self) {
let mut listeners = self.attributes_to_drop_before_render.borrow_mut();
listeners.drain(..).for_each(|listener| { listeners.drain(..).for_each(|listener| {
let listener = unsafe { &*listener }; let listener = unsafe { &*listener };
if let AttributeValue::Listener(l) = &listener.value { if let AttributeValue::Listener(l) = &listener.value {
packages/core/src/fragment.rs
+8 -9
View File
@@ -28,15 +28,14 @@ use crate::innerlude::*;
#[allow(non_upper_case_globals, non_snake_case)] #[allow(non_upper_case_globals, non_snake_case)]
pub fn Fragment<'a>(cx: Scope<'a, FragmentProps<'a>>) -> Element { pub fn Fragment<'a>(cx: Scope<'a, FragmentProps<'a>>) -> Element {
let children = cx.props.0.as_ref()?; let children = cx.props.0.as_ref()?;
Some(VNode { Some(cx.vnode(
key: children.key, children.parent.clone(),
parent: children.parent.clone(), children.key,
stable_id: children.stable_id.clone(), children.template.clone(),
template: children.template.clone(), children.root_ids.clone(),
root_ids: children.root_ids.clone(), children.dynamic_nodes,
dynamic_nodes: children.dynamic_nodes, children.dynamic_attrs,
dynamic_attrs: children.dynamic_attrs, ))
})
} }
pub struct FragmentProps<'a>(Element<'a>); pub struct FragmentProps<'a>(Element<'a>);
packages/core/src/lazynodes.rs
+10 -10
View File
@@ -24,7 +24,7 @@ use crate::{innerlude::VNode, ScopeState};
/// ///
/// ```rust, ignore /// ```rust, ignore
/// LazyNodes::new(|f| { /// LazyNodes::new(|f| {
/// static TEMPLATE: dioxus::core::Template = dioxus::core::Template { /// static TEMPLATE: dioxus::core::Template = dioxus::core::Template {
/// name: "main.rs:5:5:20", // Source location of the template for hot reloading /// name: "main.rs:5:5:20", // Source location of the template for hot reloading
/// roots: &[ /// roots: &[
/// dioxus::core::TemplateNode::Element { /// dioxus::core::TemplateNode::Element {
@@ -37,19 +37,19 @@ use crate::{innerlude::VNode, ScopeState};
/// node_paths: &[], /// node_paths: &[],
/// attr_paths: &[], /// attr_paths: &[],
/// }; /// };
/// dioxus::core::VNode { /// f.vnode(
/// parent: None, /// None,
/// key: None, /// None,
/// template: std::cell::Cell::new(TEMPLATE), /// std::cell::Cell::new(TEMPLATE),
/// root_ids: dioxus::core::exports::bumpalo::collections::Vec::with_capacity_in( /// dioxus::core::exports::bumpalo::collections::Vec::with_capacity_in(
/// 1usize, /// 1usize,
/// f.bump(), /// f.bump(),
/// ) /// )
/// .into(), /// .into(),
/// dynamic_nodes: f.bump().alloc([]), /// f.bump().alloc([]),
/// dynamic_attrs: f.bump().alloc([]), /// f.bump().alloc([]),
/// }) /// )
/// } /// })
/// ``` /// ```
/// ///
/// Find more information about how to construct [`VNode`] at <https://dioxuslabs.com/learn/0.4/contributing/walkthrough_readme#the-rsx-macro> /// Find more information about how to construct [`VNode`] at <https://dioxuslabs.com/learn/0.4/contributing/walkthrough_readme#the-rsx-macro>
packages/core/src/nodes.rs
+19 -19
View File
@@ -40,6 +40,7 @@ impl<'a> Default for RenderReturn<'a> {
/// ///
/// The dynamic parts of the template are stored separately from the static parts. This allows faster diffing by skipping /// The dynamic parts of the template are stored separately from the static parts. This allows faster diffing by skipping
/// static parts of the template. /// static parts of the template.
#[non_exhaustive]
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct VNode<'a> { pub struct VNode<'a> {
/// The key given to the root of this template. /// The key given to the root of this template.
@@ -70,20 +71,19 @@ pub struct VNode<'a> {
impl<'a> VNode<'a> { impl<'a> VNode<'a> {
/// Create a template with no nodes that will be skipped over during diffing /// Create a template with no nodes that will be skipped over during diffing
pub fn empty(cx: &'a ScopeState) -> Element<'a> { pub fn empty(cx: &'a ScopeState) -> Element<'a> {
Some(VNode { Some(cx.vnode(
key: None, Cell::new(None),
parent: Default::default(), None,
stable_id: Default::default(), Cell::new(Template {
root_ids: RefCell::new(bumpalo::collections::Vec::new_in(cx.bump())),
dynamic_nodes: &[],
dynamic_attrs: &[],
template: Cell::new(Template {
name: "dioxus-empty", name: "dioxus-empty",
roots: &[], roots: &[],
node_paths: &[], node_paths: &[],
attr_paths: &[], attr_paths: &[],
}), }),
}) RefCell::new(bumpalo::collections::Vec::new_in(cx.bump())),
&[],
&[],
))
} }
/// Create a new VNode /// Create a new VNode
@@ -820,16 +820,16 @@ impl<'b> IntoDynNode<'b> for Arguments<'_> {
} }
impl<'a> IntoDynNode<'a> for &'a VNode<'a> { impl<'a> IntoDynNode<'a> for &'a VNode<'a> {
fn into_dyn_node(self, _cx: &'a ScopeState) -> DynamicNode<'a> { fn into_dyn_node(self, cx: &'a ScopeState) -> DynamicNode<'a> {
DynamicNode::Fragment(_cx.bump().alloc([VNode { let vnode = cx.vnode(
parent: self.parent.clone(), self.parent.clone(),
stable_id: self.stable_id.clone(), self.key,
template: self.template.clone(), self.template.clone(),
root_ids: self.root_ids.clone(), self.root_ids.clone(),
key: self.key, self.dynamic_nodes,
dynamic_nodes: self.dynamic_nodes, self.dynamic_attrs,
dynamic_attrs: self.dynamic_attrs, );
}])) DynamicNode::Fragment(cx.bump().alloc([vnode]))
} }
} }
packages/core/src/scopes.rs
+37 -10
View File
@@ -2,13 +2,13 @@ use crate::{
any_props::AnyProps, any_props::AnyProps,
any_props::VProps, any_props::VProps,
bump_frame::BumpFrame, bump_frame::BumpFrame,
innerlude::{DynamicNode, EventHandler, VComponent, VNodeId, VText}, innerlude::{DynamicNode, ElementRef, EventHandler, VComponent, VNodeId, VText},
lazynodes::LazyNodes, lazynodes::LazyNodes,
nodes::{IntoAttributeValue, IntoDynNode, RenderReturn}, nodes::{IntoAttributeValue, IntoDynNode, RenderReturn},
runtime::Runtime, runtime::Runtime,
scope_context::ScopeContext, scope_context::ScopeContext,
AnyValue, Attribute, AttributeType, AttributeValue, Element, Event, MountedAttribute, AnyValue, Attribute, AttributeType, AttributeValue, Element, ElementId, Event,
Properties, TaskId, MountedAttribute, Properties, TaskId, Template, VNode,
}; };
use bumpalo::{boxed::Box as BumpBox, Bump}; use bumpalo::{boxed::Box as BumpBox, Bump};
use std::{ use std::{
@@ -102,6 +102,7 @@ pub struct ScopeState {
impl Drop for ScopeState { impl Drop for ScopeState {
fn drop(&mut self) { fn drop(&mut self) {
self.drop_listeners();
self.runtime.remove_context(self.context_id); self.runtime.remove_context(self.context_id);
} }
} }
@@ -345,19 +346,37 @@ impl<'src> ScopeState {
/// // Actually build the tree and allocate it /// // Actually build the tree and allocate it
/// cx.render(lazy_tree) /// cx.render(lazy_tree)
/// } /// }
///``` /// ```
pub fn render(&'src self, rsx: LazyNodes<'src, '_>) -> Element<'src> { pub fn render(&'src self, rsx: LazyNodes<'src, '_>) -> Element<'src> {
let element = rsx.call(self); // Note: We can't do anything in this function related to safety because the user could always circumvent it by creating a VNode manually and return it without calling this function
Some(rsx.call(self))
}
/// Create a [`crate::VNode`] from the component parts
pub fn vnode(
&'src self,
parent: Cell<Option<ElementRef>>,
key: Option<&'src str>,
template: Cell<Template<'static>>,
root_ids: RefCell<bumpalo::collections::Vec<'src, ElementId>>,
dynamic_nodes: &'src [DynamicNode<'src>],
dynamic_attrs: &'src [MountedAttribute<'src>],
) -> VNode<'src> {
let mut listeners = self.attributes_to_drop_before_render.borrow_mut(); let mut listeners = self.attributes_to_drop_before_render.borrow_mut();
for attr in element.dynamic_attrs { for attr in dynamic_attrs {
attr.ty.for_each(|attr| { let attrs = match attr.ty {
AttributeType::Single(ref attr) => std::slice::from_ref(attr),
AttributeType::Many(attrs) => attrs,
};
for attr in attrs {
match attr.value { match attr.value {
// We need to drop listeners before the next render because they may borrow data from the borrowed props which will be dropped // We need to drop listeners before the next render because they may borrow data from the borrowed props which will be dropped
AttributeValue::Listener(_) => { AttributeValue::Listener(_) => {
let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) }; let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) };
listeners.push(unbounded); listeners.push(unbounded);
} }
// We need to drop any values manually to make sure that their drop implementation is called before the next render // We need to drop any values manually to make sure that their drop implementation is called before the next render
AttributeValue::Any(_) => { AttributeValue::Any(_) => {
let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) }; let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) };
@@ -366,7 +385,7 @@ impl<'src> ScopeState {
_ => (), _ => (),
} }
}) }
} }
let mut props = self.borrowed_props.borrow_mut(); let mut props = self.borrowed_props.borrow_mut();
@@ -374,7 +393,7 @@ impl<'src> ScopeState {
.previous_frame() .previous_frame()
.props_to_drop_before_reset .props_to_drop_before_reset
.borrow_mut(); .borrow_mut();
for node in element.dynamic_nodes { for node in dynamic_nodes {
if let DynamicNode::Component(comp) = node { if let DynamicNode::Component(comp) = node {
let unbounded = unsafe { std::mem::transmute(comp as *const VComponent) }; let unbounded = unsafe { std::mem::transmute(comp as *const VComponent) };
if !comp.static_props { if !comp.static_props {
@@ -384,7 +403,15 @@ impl<'src> ScopeState {
} }
} }
Some(element) VNode {
stable_id: Default::default(),
key,
parent,
template,
root_ids,
dynamic_nodes,
dynamic_attrs,
}
} }
/// Create a dynamic text node using [`Arguments`] and the [`ScopeState`]'s internal [`Bump`] allocator /// Create a dynamic text node using [`Arguments`] and the [`ScopeState`]'s internal [`Bump`] allocator
packages/core/tests/fuzzing.rs
+12 -9
View File
@@ -2,7 +2,7 @@
use dioxus::prelude::Props; use dioxus::prelude::Props;
use dioxus_core::{MountedAttribute, *}; use dioxus_core::{MountedAttribute, *};
use std::{cfg, collections::HashSet}; use std::{cell::Cell, cfg, collections::HashSet};
fn random_ns() -> Option<&'static str> { fn random_ns() -> Option<&'static str> {
let namespace = rand::random::<u8>() % 2; let namespace = rand::random::<u8>() % 2;
@@ -171,15 +171,16 @@ fn create_random_dynamic_node(cx: &ScopeState, depth: usize) -> DynamicNode {
match rand::random::<u8>() % range { match rand::random::<u8>() % range {
0 => DynamicNode::Placeholder(Default::default()), 0 => DynamicNode::Placeholder(Default::default()),
1 => cx.make_node((0..(rand::random::<u8>() % 5)).map(|_| { 1 => cx.make_node((0..(rand::random::<u8>() % 5)).map(|_| {
VNode::new( cx.vnode(
None, None.into(),
Template { Default::default(),
Cell::new(Template {
name: concat!(file!(), ":", line!(), ":", column!(), ":0"), name: concat!(file!(), ":", line!(), ":", column!(), ":0"),
roots: &[TemplateNode::Dynamic { id: 0 }], roots: &[TemplateNode::Dynamic { id: 0 }],
node_paths: &[&[0]], node_paths: &[&[0]],
attr_paths: &[], attr_paths: &[],
}, }),
bumpalo::collections::Vec::new_in(cx.bump()), bumpalo::collections::Vec::new_in(cx.bump()).into(),
cx.bump().alloc([cx.component( cx.bump().alloc([cx.component(
create_random_element, create_random_element,
DepthProps { depth, root: false }, DepthProps { depth, root: false },
@@ -273,10 +274,12 @@ fn create_random_element(cx: Scope<DepthProps>) -> Element {
) )
.into_boxed_str(), .into_boxed_str(),
)); ));
let node = VNode::new( println!("{template:#?}");
let node = cx.vnode(
None.into(),
None, None,
template, Cell::new(template),
bumpalo::collections::Vec::new_in(cx.bump()), bumpalo::collections::Vec::new_in(cx.bump()).into(),
{ {
let dynamic_nodes: Vec<_> = dynamic_node_types let dynamic_nodes: Vec<_> = dynamic_node_types
.iter() .iter()
packages/dioxus-tui/examples/many_small_edit_stress.rs
+1 -1
View File
@@ -83,7 +83,7 @@ fn Grid(cx: Scope<GridProps>) -> Element {
let alpha = y as f32*100.0/size as f32 + counts.read()[x*size + y] as f32; let alpha = y as f32*100.0/size as f32 + counts.read()[x*size + y] as f32;
let key = format!("{}-{}", x, y); let key = format!("{}-{}", x, y);
rsx! { rsx! {
Box{ Box {
x: x, x: x,
y: y, y: y,
alpha: 100.0, alpha: 100.0,
packages/native-core/tests/fuzzing.rs
+14 -23
View File
@@ -1,3 +1,5 @@
use std::cell::Cell;
use dioxus::prelude::Props; use dioxus::prelude::Props;
use dioxus_core::*; use dioxus_core::*;
use dioxus_native_core::prelude::*; use dioxus_native_core::prelude::*;
@@ -178,15 +180,16 @@ fn create_random_dynamic_node(cx: &ScopeState, depth: usize) -> DynamicNode {
match rand::random::<u8>() % range { match rand::random::<u8>() % range {
0 => DynamicNode::Placeholder(Default::default()), 0 => DynamicNode::Placeholder(Default::default()),
1 => cx.make_node((0..(rand::random::<u8>() % 5)).map(|_| { 1 => cx.make_node((0..(rand::random::<u8>() % 5)).map(|_| {
VNode::new( cx.vnode(
None, None.into(),
Template { Default::default(),
Cell::new(Template {
name: concat!(file!(), ":", line!(), ":", column!(), ":0"), name: concat!(file!(), ":", line!(), ":", column!(), ":0"),
roots: &[TemplateNode::Dynamic { id: 0 }], roots: &[TemplateNode::Dynamic { id: 0 }],
node_paths: &[&[0]], node_paths: &[&[0]],
attr_paths: &[], attr_paths: &[],
}, }),
dioxus::core::exports::bumpalo::collections::Vec::new_in(cx.bump()), dioxus::core::exports::bumpalo::collections::Vec::new_in(cx.bump()).into(),
cx.bump().alloc([cx.component( cx.bump().alloc([cx.component(
create_random_element, create_random_element,
DepthProps { depth, root: false }, DepthProps { depth, root: false },
@@ -204,20 +207,6 @@ fn create_random_dynamic_node(cx: &ScopeState, depth: usize) -> DynamicNode {
} }
} }
fn create_random_dynamic_mounted_attr(cx: &ScopeState) -> MountedAttribute {
match rand::random::<u8>() % 2 {
0 => MountedAttribute::from(
&*cx.bump().alloc(
(0..(rand::random::<u8>() % 3) as usize)
.map(|_| create_random_dynamic_attr(cx))
.collect::<Vec<_>>(),
),
),
1 => MountedAttribute::from(create_random_dynamic_attr(cx)),
_ => unreachable!(),
}
}
fn create_random_dynamic_attr(cx: &ScopeState) -> Attribute { fn create_random_dynamic_attr(cx: &ScopeState) -> Attribute {
let value = match rand::random::<u8>() % 6 { let value = match rand::random::<u8>() % 6 {
0 => AttributeValue::Text(Box::leak( 0 => AttributeValue::Text(Box::leak(
@@ -230,6 +219,7 @@ fn create_random_dynamic_attr(cx: &ScopeState) -> Attribute {
5 => AttributeValue::None, 5 => AttributeValue::None,
_ => unreachable!(), _ => unreachable!(),
}; };
Attribute::new( Attribute::new(
Box::leak(format!("attr{}", rand::random::<usize>()).into_boxed_str()), Box::leak(format!("attr{}", rand::random::<usize>()).into_boxed_str()),
value, value,
@@ -266,10 +256,11 @@ fn create_random_element(cx: Scope<DepthProps>) -> Element {
.into_boxed_str(), .into_boxed_str(),
)); ));
println!("{template:#?}"); println!("{template:#?}");
let node = VNode::new( let node = cx.vnode(
None.into(),
None, None,
template, Cell::new(template),
dioxus::core::exports::bumpalo::collections::Vec::new_in(cx.bump()), dioxus::core::exports::bumpalo::collections::Vec::new_in(cx.bump()).into(),
{ {
let dynamic_nodes: Vec<_> = dynamic_node_types let dynamic_nodes: Vec<_> = dynamic_node_types
.iter() .iter()
@@ -287,7 +278,7 @@ fn create_random_element(cx: Scope<DepthProps>) -> Element {
cx.bump() cx.bump()
.alloc( .alloc(
(0..template.attr_paths.len()) (0..template.attr_paths.len())
.map(|_| create_random_dynamic_mounted_attr(cx)) .map(|_| create_random_dynamic_attr(cx).into())
.collect::<Vec<_>>(), .collect::<Vec<_>>(),
) )
.as_slice(), .as_slice(),
packages/rsx/src/lib.rs
+5 -3
View File
@@ -254,10 +254,12 @@ impl<'a> ToTokens for TemplateRenderer<'a> {
node_paths: &[ #(#node_paths),* ], node_paths: &[ #(#node_paths),* ],
attr_paths: &[ #(#attr_paths),* ], attr_paths: &[ #(#attr_paths),* ],
}; };
::dioxus::core::VNode::new(
__cx.vnode(
None.into(),
#key_tokens, #key_tokens,
TEMPLATE, std::cell::Cell::new(TEMPLATE),
dioxus::core::exports::bumpalo::collections::Vec::with_capacity_in(#root_count, __cx.bump()), dioxus::core::exports::bumpalo::collections::Vec::with_capacity_in(#root_count, __cx.bump()).into(),
__cx.bump().alloc([ #( #node_printer ),* ]), __cx.bump().alloc([ #( #node_printer ),* ]),
__cx.bump().alloc([ #( #dyn_attr_printer ),* ]), __cx.bump().alloc([ #( #dyn_attr_printer ),* ]),
) )