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

Memory leak/Detached DOM nodes when node is removed #3760

Open
gammahead opened this issue Feb 19, 2025 · 8 comments · May be fixed by #3782
Open

Memory leak/Detached DOM nodes when node is removed #3760

gammahead opened this issue Feb 19, 2025 · 8 comments · May be fixed by #3782
Labels
bug Something isn't working core relating to the core implementation of the virtualdom

Comments

@gammahead
Copy link

Problem

Rendering a Signal<Vec<T>> with something like the following causes unbounded DOM node growth when the Vec<T> can change length up and down between renders. It creates a bunch of detached nodes as can be seen in the image.

Image
rsx! { 
  for item in vec_signal { 
    Child { item }
  }
}

Steps To Reproduce

Steps to reproduce the behavior:

use dioxus::prelude::*;
use uuid::Uuid;
use wasm_bindgen::prelude::wasm_bindgen;
use web_sys::js_sys;

#[wasm_bindgen]
pub fn sleep(ms: i32) -> js_sys::Promise {
    js_sys::Promise::new(&mut |resolve, _| {
        web_sys::window()
            .unwrap()
            .set_timeout_with_callback_and_timeout_and_arguments_0(&resolve, ms)
            .unwrap();
    })
}

pub async fn shleep(ms: i32) {
    wasm_bindgen_futures::JsFuture::from(sleep(ms))
        .await
        .unwrap();
}

fn main() {
    dioxus::launch(app);
}

fn app() -> Element {
    let mut nums = use_signal(|| vec![]);

    let _tx = use_coroutine(move |_rx: UnboundedReceiver<()>| async move {
        let mut i = 0;
        loop {
            i += 1;
            shleep(100).await;
            if i % 2 == 0 {
                nums.set((i..i + 1).collect());
            } else {
                nums.set((i..i + 2).collect()); // switch this to i + 101 to make the detached nodes grow faster
            }
        }
    });

    rsx! {
        div {
            Child { nums }
        }
    }
}

#[component]
fn Child(nums: Signal<Vec<i32>>) -> Element {
    rsx! {
        for num in nums() {
            div { "Num: {num}" }
        }
    }
}

Expected behavior

Nodes should drop without hanging around in a detached state

Screenshots

Environment:

  • Dioxus version: 0.6.3
  • Rust version: 1.82.0
  • OS info: macOS
  • App platform: web
@gammahead gammahead added the bug Something isn't working label Feb 19, 2025
@marc2332
Copy link
Contributor

Does adding a key attribute to the looped elements help?

@gammahead
Copy link
Author

Does adding a key attribute to the looped elements help?

Does that mean wrapping it with a 'Fragment'? I'm AFK right now, but if that's what you mean, I don't think it makes a difference. I use 'Fragment' in my original code and it has same problem.

I don't know if I'm barking up the wrong tree here but I think it's a node deletion problem because I see detached nodes from other parts of the DOM popping up outside the list as well. I saw some of the logic explicitly wants to keep some state around so maybe it's some combo of trying to reuse elements while adding and deleting same time

@marc2332
Copy link
Contributor

Does adding a key attribute to the looped elements help?

Does that mean wrapping it with a 'Fragment'? I'm AFK right now, but if that's what you mean, I don't think it makes a difference. I use 'Fragment' in my original code and it has same problem.

I don't know if I'm barking up the wrong tree here but I think it's a node deletion problem because I see detached nodes from other parts of the DOM popping up outside the list as well. I saw some of the logic explicitly wants to keep some state around so maybe it's some combo of trying to reuse elements while adding and deleting same time

I mean:

rsx! { 
  for (i, item) in vec_signal.into_iter().enumerate() { 
    Child { key: "{i}", item }
  }
}

@gammahead
Copy link
Author

Ah I didn't know that's possible - but no, it doesn't help

@gammahead gammahead changed the title Memory leak/Detached DOM nodes when component arg with type Signal<Vec<T>> changes lengths across mutations Memory leak/Detached DOM nodes when node is removed Feb 20, 2025
@gammahead

This comment has been minimized.

@ealmloff
Copy link
Member

The dioxus interpreter uses a slot map to store element references. The slots should automatically get reused over time and clear the old references as new nodes are allocated. If they are not getting cleared, this is probably a bug in dioxus core

@ealmloff ealmloff added the core relating to the core implementation of the virtualdom label Feb 20, 2025
@gammahead
Copy link
Author

Just kidding, the new example code is not bad. The number of detached nodes does grow, but they eventually get garbage collected. I'll try isolate under what conditions GC fails to clean up the nodes.

@gammahead
Copy link
Author

I believe the problem ultimately boils down to https://github.com/DioxusLabs/dioxus/blob/bdeedc13eb504d3e05edb48575ceda52aea09eff/packages/core/src/diff/node.rs#L335C8-L335C31 calling remove_dynamic_node(to=None, ...), resulting in a noop on the Text(_) | Placeholder(_) branch. The ElementId for the removed node is never reclaimed or reused

@gammahead gammahead linked a pull request Feb 21, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core relating to the core implementation of the virtualdom
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants