Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Open
gammahead opened this issue Feb 19, 2025 · 15 comments · May be fixed by #3782
Open

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

gammahead opened this issue Feb 19, 2025 · 15 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.

Sign in to view
@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
Open
@bennukem
Copy link

I am using lealeft(more massive js) in a element. use_eval with or without future. When I change my view and back in the element around 10 times, the app totally crash at leaving the map element. I am thinking it s connected to this issue. The crash append on Android virtual or real device.

@iamcco
Copy link

iamcco commented May 3, 2025

Same issue here, the memory usage keeps increasing

@gammahead
Copy link
Author

I had a PR open for this, but I didn't get it over the finish line. @iamcco and/or @bennukem Can you try updating your dioxus dependency to dioxus = { git = "https://github.com/gammahead/dioxus" } and see if your memory issues are resolved? If yes, I will prioritize finishing #3782

@iamcco
Copy link

iamcco commented May 4, 2025

I had a PR open for this, but I didn't get it over the finish line. @iamcco and/or @bennukem Can you try updating your dioxus dependency to dioxus = { git = "https://github.com/gammahead/dioxus" } and see if your memory issues are resolved? If yes, I will prioritize finishing #3782

wow, it does resolve the issue, the memory does not keep increasing anymore.

@HaHa421
Copy link

HaHa421 commented May 4, 2025

Same here , my app rewrites the dom almost every frame during worst case tests , it wouldn't work decently without this patch (use case is a virtual/infinite scroll)

@gammahead
Copy link
Author

Yeah my use case was virtual scrolling as well. Glad to hear the patch works. I'll try to complete the PR

@bennukem
Copy link

bennukem commented May 4, 2025

Hello guys !
Works like a charm with dioxus = { git = "https://github.com/gammahead/dioxus" }
Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working core relating to the core implementation of the virtualdom
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reclaim element id for all removed nodes gammahead/dioxus
6 participants
@gammahead @iamcco @bennukem @marc2332 @ealmloff @HaHa421