From 44fe2defc2d17e07233961cd2760056363792e1d Mon Sep 17 00:00:00 2001
From: Jonathan Kelley <jkelleyrtp@gmail.com>
Date: Thu, 4 Apr 2024 14:25:17 -0700
Subject: [PATCH] Fix: memory leak on listeners for elements (#2244)

* Fix: memory leak on listeners for elements
* Recycle in manuallydrop

---------

Co-authored-by: Evan Almloff <evanalmloff@gmail.com>
---
 Cargo.lock                           |  1 +
 packages/core/src/nodes.rs           | 27 +++++++++++++++++++++++++--
 packages/generational-box/src/lib.rs |  1 +
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 4096c5791..b774c3f5f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2453,6 +2453,7 @@ dependencies = [
  "serde_json",
  "serde_repr",
  "tokio",
+ "tracing",
  "wasm-bindgen",
  "web-sys",
 ]
diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs
index fd6af9d2a..f71e41fc5 100644
--- a/packages/core/src/nodes.rs
+++ b/packages/core/src/nodes.rs
@@ -142,6 +142,29 @@ impl Clone for VNode {
     }
 }
 
+impl Drop for VNode {
+    fn drop(&mut self) {
+        // FIXME:
+        // TODO:
+        //
+        // We have to add this drop *here* becase we can't add a drop impl to AttributeValue and
+        // keep semver compatibility. Adding a drop impl means you can't destructure the value, which
+        // we need to do for enums.
+        //
+        // if dropping this will drop the last vnode (rc count is 1), then we need to drop the listeners
+        // in this template
+        if Rc::strong_count(&self.vnode) == 1 {
+            for attrs in self.vnode.dynamic_attrs.iter() {
+                for attr in attrs.iter() {
+                    if let AttributeValue::Listener(listener) = &attr.value {
+                        listener.callback.manually_drop();
+                    }
+                }
+            }
+        }
+    }
+}
+
 impl PartialEq for VNode {
     fn eq(&self, other: &Self) -> bool {
         Rc::ptr_eq(&self.vnode, &other.vnode)
@@ -714,14 +737,14 @@ impl AttributeValue {
     ///
     /// The callback must be confined to the lifetime of the ScopeState
     pub fn listener<T: 'static>(mut callback: impl FnMut(Event<T>) + 'static) -> AttributeValue {
+        // TODO: maybe don't use the copy-variant of EventHandler here?
+        // Maybe, create an Owned variant so we are less likely to run into leaks
         AttributeValue::Listener(EventHandler::new(move |event: Event<dyn Any>| {
             let data = event.data.downcast::<T>().unwrap();
-            // if let Ok(data) = event.data.downcast::<T>() {
             callback(Event {
                 propagates: event.propagates,
                 data,
             });
-            // }
         }))
     }
 
diff --git a/packages/generational-box/src/lib.rs b/packages/generational-box/src/lib.rs
index c94ebb12c..f73de000f 100644
--- a/packages/generational-box/src/lib.rs
+++ b/packages/generational-box/src/lib.rs
@@ -189,6 +189,7 @@ impl<T, S: Storage<T>> GenerationalBox<T, S> {
     /// Drop the value out of the generational box and invalidate the generational box. This will return the value if the value was taken.
     pub fn manually_drop(&self) -> Option<T> {
         if self.validate() {
+            <S as AnyStorage>::recycle(&self.raw);
             Storage::take(&self.raw.0.data)
         } else {
             None