From b99f081c0839defc9d9c61cb428f5e6b1464aab6 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 25 Oct 2023 16:37:04 -0500 Subject: [PATCH] drop any attribute after rendering --- packages/core/src/arena.rs | 5 +---- packages/core/src/bump_frame.rs | 28 +++++++++++++++++++++++++--- packages/core/src/scope_arena.rs | 4 ++-- packages/core/src/scopes.rs | 12 +++++++++--- 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/packages/core/src/arena.rs b/packages/core/src/arena.rs index b7f768755..4f441bb81 100644 --- a/packages/core/src/arena.rs +++ b/packages/core/src/arena.rs @@ -164,16 +164,13 @@ impl VirtualDom { }); // Now that all the references are gone, we can safely drop our own references in our listeners. - let mut listeners = scope.attributes_to_drop.borrow_mut(); + let mut listeners = scope.attributes_to_drop_before_render.borrow_mut(); listeners.drain(..).for_each(|listener| { let listener = unsafe { &*listener }; match &listener.value { AttributeValue::Listener(l) => { _ = l.take(); } - AttributeValue::Any(a) => { - _ = a.take(); - } _ => (), } }); diff --git a/packages/core/src/bump_frame.rs b/packages/core/src/bump_frame.rs index 0fe7b3867..d4b3d2e07 100644 --- a/packages/core/src/bump_frame.rs +++ b/packages/core/src/bump_frame.rs @@ -1,10 +1,13 @@ use crate::nodes::RenderReturn; +use crate::{Attribute, AttributeValue}; use bumpalo::Bump; +use std::cell::RefCell; use std::cell::{Cell, UnsafeCell}; pub(crate) struct BumpFrame { pub bump: UnsafeCell, pub node: Cell<*const RenderReturn<'static>>, + pub(crate) attributes_to_drop_before_reset: RefCell>>, } impl BumpFrame { @@ -13,6 +16,7 @@ impl BumpFrame { Self { bump: UnsafeCell::new(bump), node: Cell::new(std::ptr::null()), + attributes_to_drop_before_reset: Default::default(), } } @@ -31,8 +35,26 @@ impl BumpFrame { unsafe { &*self.bump.get() } } - #[allow(clippy::mut_from_ref)] - pub(crate) unsafe fn bump_mut(&self) -> &mut Bump { - unsafe { &mut *self.bump.get() } + pub(crate) fn add_attribute_to_drop(&self, attribute: *const Attribute<'static>) { + self.attributes_to_drop_before_reset + .borrow_mut() + .push(attribute); + } + + pub(crate) unsafe fn reset(&self) { + let mut attributes = self.attributes_to_drop_before_reset.borrow_mut(); + attributes.drain(..).for_each(|attribute| { + let attribute = unsafe { &*attribute }; + match &attribute.value { + AttributeValue::Any(l) => { + _ = l.take(); + } + _ => (), + } + }); + unsafe { + let bump = &mut *self.bump.get(); + bump.reset(); + } } } diff --git a/packages/core/src/scope_arena.rs b/packages/core/src/scope_arena.rs index 1ed5b816c..397f5328e 100644 --- a/packages/core/src/scope_arena.rs +++ b/packages/core/src/scope_arena.rs @@ -35,7 +35,7 @@ impl VirtualDom { hook_idx: Default::default(), borrowed_props: Default::default(), - attributes_to_drop: Default::default(), + attributes_to_drop_before_render: Default::default(), })); let context = @@ -54,7 +54,7 @@ impl VirtualDom { let new_nodes = unsafe { let scope = &self.scopes[scope_id.0]; - scope.previous_frame().bump_mut().reset(); + scope.previous_frame().reset(); scope.context().suspended.set(false); diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 996529a0d..3fd2aac30 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -94,7 +94,7 @@ pub struct ScopeState { pub(crate) hook_idx: Cell, pub(crate) borrowed_props: RefCell>>, - pub(crate) attributes_to_drop: RefCell>>, + pub(crate) attributes_to_drop_before_render: RefCell>>, pub(crate) props: Option>>, } @@ -348,13 +348,19 @@ impl<'src> ScopeState { pub fn render(&'src self, rsx: LazyNodes<'src, '_>) -> Element<'src> { let element = rsx.call(self); - let mut listeners = self.attributes_to_drop.borrow_mut(); + let mut listeners = self.attributes_to_drop_before_render.borrow_mut(); for attr in element.dynamic_attrs { match attr.value { - AttributeValue::Any(_) | AttributeValue::Listener(_) => { + // We need to drop listeners before the next render because they may borrow data from the borrowed props which will be dropped + AttributeValue::Listener(_) => { let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) }; listeners.push(unbounded); } + // We need to drop any values manually to make sure that their drop implementation is called before the next render + AttributeValue::Any(_) => { + let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) }; + self.previous_frame().add_attribute_to_drop(unbounded); + } _ => (), }