diff --git a/packages/sycamore-reactive/Cargo.toml b/packages/sycamore-reactive/Cargo.toml index 520ea1079..79a2a33f9 100644 --- a/packages/sycamore-reactive/Cargo.toml +++ b/packages/sycamore-reactive/Cargo.toml @@ -11,6 +11,7 @@ repository = "https://github.com/sycamore-rs/sycamore" version = "0.8.0-beta.4" [dependencies] +bumpalo = { version = "3.9.1", features = ["boxed"] } indexmap = "1.8.0" serde = { version = "1.0.136", optional = true } slotmap = "1.0.6" diff --git a/packages/sycamore-reactive/src/arena.rs b/packages/sycamore-reactive/src/arena.rs index e1547c684..033185883 100644 --- a/packages/sycamore-reactive/src/arena.rs +++ b/packages/sycamore-reactive/src/arena.rs @@ -2,6 +2,7 @@ use std::cell::UnsafeCell; +use bumpalo::Bump; use smallvec::SmallVec; /// The size of the [`SmallVec`] inline data. @@ -13,14 +14,18 @@ impl ReallyAny for T {} #[derive(Default)] pub(crate) struct ScopeArena<'a> { + pub bump: Bump, + // We need to store the raw pointers because otherwise the values won't be dropped. inner: UnsafeCell>, } impl<'a> ScopeArena<'a> { - /// Allocate a value onto the arena. Returns a reference that lasts as long as the arena itself. - pub fn alloc(&'a self, value: T) -> &'a T { - let boxed = Box::new(value); - let ptr = Box::into_raw(boxed); + /// Allocate a value onto the arena. Returns a mutable reference that lasts as long as the arena + /// itself. + #[allow(clippy::mut_from_ref)] // We return a new reference each time so this is a false-positive. + pub fn alloc(&'a self, value: T) -> &'a mut T { + let boxed = bumpalo::boxed::Box::new_in(value, &self.bump); + let ptr = bumpalo::boxed::Box::into_raw(boxed); unsafe { // SAFETY: The only place where self.inner.get() is mutably borrowed is right here. // It is impossible to have two alloc() calls on the same ScopeArena at the same time so @@ -35,7 +40,7 @@ impl<'a> ScopeArena<'a> { // dropped. // - The drop code for Scope never reads the allocated value and therefore does not create a // stacked-borrows violation. - unsafe { &*ptr } + unsafe { &mut *ptr } } /// Cleanup the resources owned by the [`ScopeArena`]. This is automatically called in [`Drop`]. @@ -45,8 +50,8 @@ impl<'a> ScopeArena<'a> { /// If a [`ScopeArena`] has already been disposed, calling it again does nothing. pub unsafe fn dispose(&self) { for &ptr in &*self.inner.get() { - // SAFETY: the ptr was allocated in Self::alloc using Box::into_raw. - let boxed: Box = Box::from_raw(ptr); + // SAFETY: the ptr was allocated in Self::alloc using bumpalo::boxed::Box::into_raw. + let boxed: bumpalo::boxed::Box = bumpalo::boxed::Box::from_raw(ptr); // Call the drop code for the allocated value. drop(boxed); } diff --git a/packages/sycamore-reactive/src/effect.rs b/packages/sycamore-reactive/src/effect.rs index a59c17fa6..4511e57cf 100644 --- a/packages/sycamore-reactive/src/effect.rs +++ b/packages/sycamore-reactive/src/effect.rs @@ -69,30 +69,27 @@ impl<'a> EffectState<'a> { /// # }); /// ``` pub fn create_effect<'a>(cx: Scope<'a>, f: impl FnMut() + 'a) { - _create_effect(cx, Box::new(f)) + let f = cx.alloc(f); + _create_effect(cx, f) } /// Internal implementation for `create_effect`. Use dynamic dispatch to reduce code-bloat. -fn _create_effect<'a>(cx: Scope<'a>, mut f: Box) { - let effect = Rc::new(RefCell::new(None::>)); +fn _create_effect<'a>(cx: Scope<'a>, f: &'a mut (dyn FnMut() + 'a)) { + let effect = &*cx.alloc(RefCell::new(None::>)); let cb = Rc::new(RefCell::new({ - let effect = Rc::downgrade(&effect); move || { EFFECTS.with(|effects| { // Record initial effect stack length to verify that it is the same after. let initial_effect_stack_len = effects.borrow().len(); - // Upgrade the effect to an Rc now so that it is valid for the rest of the - // callback. - let effect_ref = effect.upgrade().unwrap(); // Take effect out. - let mut effect = effect_ref.take().unwrap(); - effect.clear_dependencies(); + let mut tmp_effect = effect.take().unwrap(); + tmp_effect.clear_dependencies(); // Push the effect onto the effect stack so that it is visible by signals. effects .borrow_mut() - .push(unsafe { std::mem::transmute(&mut effect as *mut EffectState<'a>) }); + .push(unsafe { std::mem::transmute(&mut tmp_effect as *mut EffectState<'a>) }); // Now we can call the user-provided function. f(); // Pop the effect from the effect stack. @@ -100,23 +97,22 @@ fn _create_effect<'a>(cx: Scope<'a>, mut f: Box) { // The raw pointer pushed onto `effects` is dead and can no longer be accessed. // We can now access `effect` directly again. - // For all the signals collected by the EffectState, - // we need to add backlinks from the signal to the effect, so that - // updating the signal will trigger the effect. - for emitter in &effect.dependencies { + // For all the signals collected by the EffectState, we need to add backlinks from + // the signal to the effect, so that updating the signal will trigger the effect. + for emitter in &tmp_effect.dependencies { // The SignalEmitter might have been destroyed between when the signal was // accessed and now. if let Some(emitter) = emitter.0.upgrade() { // SAFETY: When the effect is destroyed or when the emitter is dropped, - // this link will be destroyed to prevent - // dangling references. - emitter - .subscribe(Rc::downgrade(unsafe { std::mem::transmute(&effect.cb) })); + // this link will be destroyed to prevent dangling references. + emitter.subscribe(Rc::downgrade(unsafe { + std::mem::transmute(&tmp_effect.cb) + })); } } // Get the effect state back into the Rc - *effect_ref.borrow_mut() = Some(effect); + *effect.borrow_mut() = Some(tmp_effect); debug_assert_eq!(effects.borrow().len(), initial_effect_stack_len); }); @@ -131,9 +127,6 @@ fn _create_effect<'a>(cx: Scope<'a>, mut f: Box) { // Initial callback call to get everything started. cb.borrow_mut()(); - - // Push Rc to self.effects so that it is not dropped immediately. - cx.raw.inner.borrow_mut().effects.push(effect); } /// Creates an effect on signals used inside the effect closure. @@ -167,9 +160,7 @@ where // use-after-free during the clear dependencies phase. if let Some(disposer) = disposer.take() { // SAFETY: we are not accessing the scope after the effect has been dropped. - unsafe { - disposer.dispose(); - } + unsafe { disposer.dispose() }; } // Create a new nested scope and save the disposer. let new_disposer: Option>> = @@ -178,9 +169,6 @@ where // self.create_child_scope(_). f(unsafe { std::mem::transmute(cx) }); }))); - // SAFETY: transmute the lifetime. This is safe because disposer is only used within the - // effect which is necessarily within the lifetime of self (the Scope). - // disposer = unsafe { std::mem::transmute(new_disposer) }; disposer = new_disposer; }); } diff --git a/packages/sycamore-reactive/src/lib.rs b/packages/sycamore-reactive/src/lib.rs index d8c482b6f..f8a7b0cb2 100644 --- a/packages/sycamore-reactive/src/lib.rs +++ b/packages/sycamore-reactive/src/lib.rs @@ -34,8 +34,6 @@ struct InvariantLifetime<'id>(PhantomData<&'id mut &'id ()>); /// instead of a [`RefCell`] for every field. #[derive(Default)] struct ScopeInner<'a> { - /// Effect functions created on the [`Scope`]. - effects: Vec>>>>, /// Cleanup functions. cleanups: Vec>, /// Child scopes. @@ -96,6 +94,11 @@ impl<'a, 'b: 'a> BoundedScope<'a, 'b> { _phantom: PhantomData, } } + + /// Alias for `self.raw.arena.alloc`. + fn alloc(&self, value: T) -> &'a mut T { + self.raw.arena.alloc(value) + } } /// A type-alias for [`BoundedScope`] where both lifetimes are the same. @@ -112,7 +115,6 @@ impl<'a> ScopeRaw<'a> { // Self::new() is intentionally pub(crate) only to prevent end-users from creating a Scope. Self { inner: RefCell::new(ScopeInner { - effects: Default::default(), cleanups: Default::default(), child_scopes: Default::default(), contexts: None, @@ -371,8 +373,6 @@ impl<'a> ScopeRaw<'a> { // Dispose of cx if it has not already been disposed. cx.dispose() } - // Drop effects. - drop(mem::take(&mut inner.effects)); // Call cleanup functions in an untracked scope. untrack(|| { for cb in mem::take(&mut inner.cleanups) {