Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: memory leak on listeners for elements #2244

Merged
merged 5 commits into from Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 25 additions & 2 deletions packages/core/src/nodes.rs
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
});
// }
}))
}

Expand Down