bug: Pontential memory leak, event handlers not being properly dropped

[marc2332]

Problem

Removing X elements with events handlers that clone data inside and adding them back should maintain the same memory usage, instead it always increases

Steps To Reproduce

Run https://github.com/marc2332/dioxus/blob/30e0f41cd667000e632bf482f2005ad416c942b0/packages/native-core/examples/memory-leak.rs

Environment:

• Dioxus version: master
• Rust version: 1.71
• OS info: Windows
• App platform: No renderer

[ealmloff]

Without native-core also seems to leak:

#![allow(non_snake_case)]
use dioxus::prelude::*;

fn main() -> Result<(), Box<dyn std::error::Error>> {
    fn app(cx: Scope) -> Element {
        let count = use_state(cx, || 0);

        use_future(cx, (count,), |(count,)| async move {
            loop {
                tokio::time::sleep(std::time::Duration::from_millis(2)).await;
                if *count.get() == 50 {
                    count.set(0);
                } else {
                    count.set(*count + 1);
                }
            }
        });

        let aaa = cx.use_hook(|| "aaaaaaaaaaaaaa".to_string()).clone();

        cx.render(rsx! {
            div { color: "red",
                "{count}"
                div {
                    for el in 0..*count.get() {
                        {
                            let aaa = aaa.clone();
                            rsx!(
                                rect {
                                    key: "{el}",
                                    onclick: move |_| {
                                        let _x = aaa.clone();
                                        println!("hi");
                                    },
                                    label {
                                        "{el}"
                                    }
                                }
                            )
                        }
                    }
                }
                div {
                    for el in 0..*count.get() {
                        {
                            let aaa = aaa.clone();
                            rsx!(
                                rect {
                                    key: "{el}",
                                    onclick: move |_| {
                                        let _x = aaa.clone();
                                        println!("hi");
                                    },
                                    label {
                                        "{el}"
                                    }
                                }
                            )
                        }
                    }
                }
            }
        })
    }

    fn Comp(cx: Scope) -> Element {
        cx.render(rsx! { div { border: "", "hello world" } })
    }

    // create the vdom, the real_dom, and the binding layer between them
    let mut vdom = VirtualDom::new(app);

    let _ = vdom.rebuild();

    // we need to run the vdom in a async runtime
    tokio::runtime::Builder::new_current_thread()
        .enable_all()
        .build()?
        .block_on(async {
            loop {
                // wait for the vdom to update
                vdom.wait_for_work().await;

                // get the mutations from the vdom
                let mutations = vdom.render_immediate();
            }
        })
}

[ealmloff]

The example without native core is now fixed, but the original example with native core still leaks memory

[ealmloff]

This could be related to leudz/shipyard#185 or the issue miri catches in #1580

[ealmloff]

It looks like this is an upstream intentional leak in shipyard. See leudz/shipyard#185 for more information. The original leak in the example is fixed in Dioxus. Adding or removing the event handlers produces the same result. There is still a small memory leak when adding and removing a large number of elements caused by the shipyard issue

[marc2332]

Looks like I still get memory leaks, example here:

#![allow(non_snake_case)]
use std::time::Duration;

use dioxus::prelude::*;
use dioxus_native_core::exports::shipyard;
use dioxus_native_core::exports::shipyard::{Component};
use dioxus_native_core::node_ref::*;
use dioxus_native_core::prelude::*;
use dioxus_native_core_macro::partial_derive_state;
use tokio::time::interval;

struct FontSize(f64);

// All states need to derive Component
#[derive(Default, Debug, Copy, Clone, PartialEq, Component)]
struct Size(f64, f64);

/// Derive some of the boilerplate for the State implementation
#[partial_derive_state]
impl State for Size {
    type ParentDependencies = ();

    // The size of the current node depends on the size of its children
    type ChildDependencies = (Self,);

    type NodeDependencies = ();

    // Size only cares about the width, height, and text parts of the current node
    const NODE_MASK: NodeMaskBuilder<'static> = NodeMaskBuilder::new()
        // Get access to the width and height attributes
        .with_attrs(AttributeMaskBuilder::Some(&["width", "height"]))
        // Get access to the text of the node
        .with_text();

    fn update<'a>(
        &mut self,
        node_view: NodeView<()>,
        _node: <Self::NodeDependencies as Dependancy>::ElementBorrowed<'a>,
        _parent: Option<<Self::ParentDependencies as Dependancy>::ElementBorrowed<'a>>,
        children: Vec<<Self::ChildDependencies as Dependancy>::ElementBorrowed<'a>>,
        context: &SendAnyMap,
    ) -> bool {
        let font_size = context.get::<FontSize>().unwrap().0;
        let mut width;
        let mut height;
        if let Some(text) = node_view.text() {
            // if the node has text, use the text to size our object
            width = text.len() as f64 * font_size;
            height = font_size;
        } else {
            // otherwise, the size is the maximum size of the children
            width = children
                .iter()
                .map(|(item,)| item.0)
                .reduce(|accum, item| if accum >= item { accum } else { item })
                .unwrap_or(0.0);

            height = children
                .iter()
                .map(|(item,)| item.1)
                .reduce(|accum, item| if accum >= item { accum } else { item })
                .unwrap_or(0.0);
        }
        // if the node contains a width or height attribute it overrides the other size
        for a in node_view.attributes().into_iter().flatten() {
            match &*a.attribute.name {
                "width" => width = a.value.as_float().unwrap(),
                "height" => height = a.value.as_float().unwrap(),
                // because Size only depends on the width and height, no other attributes will be passed to the member
                _ => panic!(),
            }
        }
        // to determine what other parts of the dom need to be updated we return a boolean that marks if this member changed
        let changed = (width != self.0) || (height != self.1);
        *self = Self(width, height);
        changed
    }
}

#[derive(Debug, Clone, Copy, PartialEq, Default, Component)]
struct TextColor {
    r: u8,
    g: u8,
    b: u8,
}

#[partial_derive_state]
impl State for TextColor {
    // TextColor depends on the TextColor part of the parent
    type ParentDependencies = (Self,);

    type ChildDependencies = ();

    type NodeDependencies = ();

    // TextColor only cares about the color attribute of the current node
    const NODE_MASK: NodeMaskBuilder<'static> =
        // Get access to the color attribute
        NodeMaskBuilder::new().with_attrs(AttributeMaskBuilder::Some(&["color"]));

    fn update<'a>(
        &mut self,
        node_view: NodeView<()>,
        _node: <Self::NodeDependencies as Dependancy>::ElementBorrowed<'a>,
        parent: Option<<Self::ParentDependencies as Dependancy>::ElementBorrowed<'a>>,
        _children: Vec<<Self::ChildDependencies as Dependancy>::ElementBorrowed<'a>>,
        _context: &SendAnyMap,
    ) -> bool {
        // TextColor only depends on the color tag, so getting the first tag is equivilent to looking through all tags
        let new = match node_view
            .attributes()
            .and_then(|mut attrs| attrs.next())
            .and_then(|attr| attr.value.as_text())
        {
            // if there is a color tag, translate it
            Some("red") => TextColor { r: 255, g: 0, b: 0 },
            Some("green") => TextColor { r: 0, g: 255, b: 0 },
            Some("blue") => TextColor { r: 0, g: 0, b: 255 },
            Some(color) => panic!("unknown color {color}"),
            // otherwise check if the node has a parent and inherit that color
            None => match parent {
                Some((parent,)) => *parent,
                None => Self::default(),
            },
        };
        // check if the member has changed
        let changed = new != *self;
        *self = new;
        changed
    }
}

#[derive(Debug, Clone, Copy, PartialEq, Default, Component)]
struct Border(bool);

#[partial_derive_state]
impl State for Border {
    // TextColor depends on the TextColor part of the parent
    type ParentDependencies = (Self,);

    type ChildDependencies = ();

    type NodeDependencies = ();

    // Border does not depended on any other member in the current node
    const NODE_MASK: NodeMaskBuilder<'static> =
        // Get access to the border attribute
        NodeMaskBuilder::new().with_attrs(AttributeMaskBuilder::Some(&["border"]));

    fn update<'a>(
        &mut self,
        node_view: NodeView<()>,
        _node: <Self::NodeDependencies as Dependancy>::ElementBorrowed<'a>,
        _parent: Option<<Self::ParentDependencies as Dependancy>::ElementBorrowed<'a>>,
        _children: Vec<<Self::ChildDependencies as Dependancy>::ElementBorrowed<'a>>,
        _context: &SendAnyMap,
    ) -> bool {
        // check if the node contians a border attribute
        let new = Self(
            node_view
                .attributes()
                .and_then(|mut attrs| attrs.next().map(|a| a.attribute.name == "border"))
                .is_some(),
        );
        // check if the member has changed
        let changed = new != *self;
        *self = new;
        changed
    }
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    fn app(cx: Scope) -> Element {
        let num = use_ref(cx, || 0);

        use_effect(cx, (), move |_| {
            let num = num.clone();
            async move {
                let mut int = interval(Duration::from_millis(15));
                loop {
                    int.tick().await;
                    if *num.read() == 0 {
                        *num.write() = 600; // Leaks
                    } else {
                        *num.write() = 0;
                    }
                }
            }
        });

        println!("recreated");

        render!(
            div {
                for i in *num.read()..300 {
                    rsx!(
                        CoolButton {
                            key: "{i}",
                        }
                    )
                }
            }
        )
    }

    #[allow(non_snake_case)]
    fn CoolButton(cx: Scope) -> Element {
        let hovering = use_state(cx, || false);

        let onmouseenter = {
            to_owned![hovering]; // <-- Culprit 
            move |_| {
                hovering.set(true);
            }
        };

        let onmouseleave = move |_| {
            hovering.set(false);
        };

        render!(
            div {
                onmouseenter: onmouseenter,
                onmouseleave: onmouseleave,
            }
        )
    }

    // create the vdom, the real_dom, and the binding layer between them
    let mut vdom = VirtualDom::new(app);
    let mut rdom: RealDom = RealDom::new([
        Border::to_type_erased(),
        TextColor::to_type_erased(),
        Size::to_type_erased(),
    ]);
    let mut dioxus_intigration_state = DioxusState::create(&mut rdom);

    let mutations = vdom.rebuild();
    // update the structure of the real_dom tree
    dioxus_intigration_state.apply_mutations(&mut rdom, mutations);
    let mut ctx = SendAnyMap::new();
    // set the font size to 3.3
    ctx.insert(FontSize(3.3));
    // update the State for nodes in the real_dom tree
    let _to_rerender = rdom.update_state(ctx);

    // we need to run the vdom in a async runtime
    tokio::runtime::Builder::new_current_thread()
        .enable_all()
        .build()?
        .block_on(async {
            loop {
                // wait for the vdom to update
                vdom.wait_for_work().await;

                // get the mutations from the vdom
                let mutations = vdom.render_immediate();

                // update the structure of the real_dom tree
                dioxus_intigration_state.apply_mutations(&mut rdom, mutations);

                // update the state of the real_dom tree
                let mut ctx = SendAnyMap::new();
                // set the font size to 3.3
                ctx.insert(FontSize(3.3));
                let _to_rerender = rdom.update_state(ctx);

                println!("DOM size: {}", rdom.tree_ref().len());
            }
        })
}