Fix the event handler memory leak (#3508)
* Fix the event handler memory leak * Add a regression test for memory leaks * add a component to the memory leak test
This commit is contained in:
1 parent
2ad5d1beb8
commit
19ca7075d6
4 files changed
+185
-9
No files matched your search
@@ -34,6 +34,7 @@ reqwest = { workspace = true }
|
||||
tracing-subscriber = { workspace = true, default-features = true }
|
||||
tracing-fluent-assertions = "0.3.0"
|
||||
pretty_assertions = "1.3.0"
|
||||
sysinfo = "0.33.1"
|
||||
|
||||
[dev-dependencies.web-sys]
|
||||
version = "0.3.56"
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
#![allow(non_snake_case)]
|
||||
use dioxus::prelude::dioxus_core::NoOpMutations;
|
||||
use dioxus::prelude::*;
|
||||
use sysinfo::{ProcessRefreshKind, RefreshKind, System};
|
||||
|
||||
// Regression test for https://github.com/DioxusLabs/dioxus/issues/3421
|
||||
#[tokio::test]
|
||||
async fn test_for_memory_leaks() {
|
||||
fn app() -> Element {
|
||||
let mut count = use_signal(|| 0);
|
||||
|
||||
use_hook(|| {
|
||||
spawn(async move {
|
||||
loop {
|
||||
tokio::time::sleep(std::time::Duration::from_nanos(1)).await;
|
||||
let val = *count.peek_unchecked();
|
||||
if val == 70 {
|
||||
count.set(0);
|
||||
} else {
|
||||
count.set(val + 1);
|
||||
}
|
||||
}
|
||||
})
|
||||
});
|
||||
|
||||
rsx! {
|
||||
for el in 0..*count.read() {
|
||||
div {
|
||||
key: "{el}",
|
||||
div {
|
||||
onclick: move |_| { println!("click"); },
|
||||
}
|
||||
AcceptsEventHandlerAndReadOnlySignal {
|
||||
event_handler: move |_| { println!("click"); },
|
||||
signal: el,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Event handlers and ReadOnlySignals have extra logic on component boundaries that has caused memory leaks
|
||||
// in the past
|
||||
#[component]
|
||||
fn AcceptsEventHandlerAndReadOnlySignal(
|
||||
event_handler: EventHandler<MouseEvent>,
|
||||
signal: ReadOnlySignal<i32>,
|
||||
) -> Element {
|
||||
rsx! {
|
||||
div {
|
||||
onclick: event_handler,
|
||||
"{signal}"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// create the vdom, the real_dom, and the binding layer between them
|
||||
let mut vdom = VirtualDom::new(app);
|
||||
|
||||
vdom.rebuild(&mut NoOpMutations);
|
||||
|
||||
let pid = sysinfo::get_current_pid().expect("failed to get PID");
|
||||
|
||||
let refresh =
|
||||
RefreshKind::nothing().with_processes(ProcessRefreshKind::nothing().with_memory());
|
||||
let mut system = System::new_with_specifics(refresh);
|
||||
|
||||
let mut get_memory_usage = || {
|
||||
system.refresh_specifics(refresh);
|
||||
let this_process = system.process(pid).expect("failed to get process");
|
||||
this_process.memory()
|
||||
};
|
||||
|
||||
let initial_memory_usage = get_memory_usage();
|
||||
|
||||
// we need to run the vdom in a async runtime
|
||||
for i in 0..=10000 {
|
||||
// wait for the vdom to update
|
||||
vdom.wait_for_work().await;
|
||||
|
||||
// get the mutations from the vdom
|
||||
vdom.render_immediate(&mut NoOpMutations);
|
||||
|
||||
if i % 1000 == 0 {
|
||||
let new_memory_usage = get_memory_usage();
|
||||
println!("iteration: {} memory usage: {}", i, new_memory_usage);
|
||||
|
||||
// Memory usage might increase as arenas fill up, but it shouldn't double from the initial render
|
||||
assert!(new_memory_usage < initial_memory_usage * 2);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -26,10 +26,16 @@ macro_rules! impl_event {
|
||||
)?
|
||||
#[inline]
|
||||
pub fn $name<__Marker>(mut _f: impl ::dioxus_core::prelude::SuperInto<::dioxus_core::prelude::EventHandler<::dioxus_core::Event<$data>>, __Marker>) -> ::dioxus_core::Attribute {
|
||||
let event_handler = _f.super_into();
|
||||
// super into will make a closure that is owned by the current owner (either the child component or the parent component).
|
||||
// We can't change that behavior in a minor version because it would cause issues with Components that accept event handlers.
|
||||
// Instead we run super into with an owner that is moved into the listener closure so it will be dropped when the closure is dropped.
|
||||
let owner = <::generational_box::UnsyncStorage as ::generational_box::AnyStorage>::owner();
|
||||
let event_handler = ::dioxus_core::prelude::with_owner(owner.clone(), || _f.super_into());
|
||||
::dioxus_core::Attribute::new(
|
||||
impl_event!(@name $name $($js_name)?),
|
||||
::dioxus_core::AttributeValue::listener(move |e: ::dioxus_core::Event<crate::PlatformEventData>| {
|
||||
// Force the owner to be moved into the event handler
|
||||
_ = &owner;
|
||||
event_handler.call(e.map(|e| e.into()));
|
||||
}),
|
||||
None,
|
||||
|
||||
Reference in new issue
Block a user