From 098f215fe252e91b4475ac68134566770fbca935 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Thu, 20 Apr 2023 10:07:25 +0200 Subject: [PATCH 1/9] Graph Editor leaks no more --- app/gui/src/lib.rs | 64 ++- .../src/component/node/growth_animation.rs | 10 +- app/gui/view/graph-editor/src/lib.rs | 475 ++++++++++-------- .../graph-editor/src/new_node_position.rs | 8 +- lib/rust/frp/src/network.rs | 72 ++- 5 files changed, 390 insertions(+), 239 deletions(-) diff --git a/app/gui/src/lib.rs b/app/gui/src/lib.rs index 4d32d845f341..7a9757dce3fb 100644 --- a/app/gui/src/lib.rs +++ b/app/gui/src/lib.rs @@ -64,6 +64,12 @@ extern crate core; use prelude::*; +use wasm_bindgen::prelude::*; + +mod profile_workflow; +#[cfg(test)] +mod tests; + // ============== // === Export === @@ -82,13 +88,9 @@ pub mod transport; pub use crate::ide::*; pub use engine_protocol; +use enso_executor::web::EventLoopExecutor; pub use ide_view as view; - - -#[cfg(test)] -mod tests; - /// Common types that should be visible across the whole IDE crate. pub mod prelude { pub use ast::prelude::*; @@ -126,6 +128,12 @@ pub mod prelude { pub use wasm_bindgen_test::wasm_bindgen_test_configure; } + + +// ==================== +// === Entry Points === +// ==================== + // These imports are required to have all entry points (such as examples) and `before_main` // functions (such as the dynamic-asset loader), available in the IDE. #[allow(unused_imports)] @@ -136,13 +144,23 @@ mod imported_for_entry_points { } #[allow(unused_imports)] use imported_for_entry_points::*; -mod profile_workflow; -// =================== -// === Entry Point === -// =================== +// ==================== +// === Global State === +// ==================== + +thread_local! { + static EXECUTOR: RefCell> = default(); + static IDE: RefCell>> = default(); +} + + + +// ======================= +// === IDE Entry Point === +// ======================= /// IDE startup function. #[entry_point(ide)] @@ -159,16 +177,32 @@ pub fn main() { "debug_mode_is_active", analytics::AnonymousData(debug_mode), ); - let config = - crate::config::Startup::from_web_arguments().expect("Failed to read configuration"); - let executor = crate::executor::setup_global_executor(); - let initializer = crate::ide::initializer::Initializer::new(config); + let config = config::Startup::from_web_arguments().expect("Failed to read configuration"); + let executor = executor::setup_global_executor(); + EXECUTOR.with(move |global_executor| global_executor.replace(Some(executor))); + let initializer = Initializer::new(config); executor::global::spawn(async move { let ide = initializer.start().await; ensogl::system::web::document .get_element_by_id("loader") .map(|t| t.parent_node().map(|p| p.remove_child(&t).unwrap())); - std::mem::forget(ide); + IDE.with(move |global_ide| global_ide.replace(Some(ide))); }); - std::mem::forget(executor); +} + + + +// ================ +// === IDE Drop === +// ================ + +#[wasm_bindgen] +pub fn drop() { + IDE.with(RefCell::take); + EXECUTOR.with(RefCell::take); + // warn!("Graphs: {:#?}", ide_view::graph_editor::VALGRIND.with(|v| v.take())); + warn!( + "Networks: {:#?}", + enso_frp::network::VALGRIND.with(|v| v.take().creation_backtraces.into_iter().next()) + ); } diff --git a/app/gui/view/graph-editor/src/component/node/growth_animation.rs b/app/gui/view/graph-editor/src/component/node/growth_animation.rs index add06cf2d00c..dc09d8c99af0 100644 --- a/app/gui/view/graph-editor/src/component/node/growth_animation.rs +++ b/app/gui/view/graph-editor/src/component/node/growth_animation.rs @@ -8,7 +8,7 @@ use ensogl::prelude::*; use crate::application::command::FrpNetworkProvider; -use crate::GraphEditorModelWithNetwork; +use crate::GraphEditorModel; use crate::NodeId; use enso_frp as frp; @@ -33,11 +33,7 @@ const ANIMATION_LENGTH_COEFFIENT: f32 = 15.0; /// Initialize edited node growth/shrink animator. It would handle scene layer change for the edited /// node as well. -pub fn initialize_edited_node_animator( - model: &GraphEditorModelWithNetwork, - frp: &crate::Frp, - scene: &Scene, -) { +pub fn initialize_edited_node_animator(model: &GraphEditorModel, frp: &crate::Frp, scene: &Scene) { let network = &frp.network(); let out = &frp.output; let searcher_cam = scene.layers.node_searcher.camera(); @@ -112,7 +108,7 @@ pub fn initialize_edited_node_animator( // === Helpers === -impl GraphEditorModelWithNetwork { +impl GraphEditorModel { /// Move node to the `edited_node` scene layer, so that it is rendered by the separate camera. #[profile(Debug)] fn move_node_to_edited_node_layer(&self, node_id: NodeId) { diff --git a/app/gui/view/graph-editor/src/lib.rs b/app/gui/view/graph-editor/src/lib.rs index d395057029b1..064063ce4f07 100644 --- a/app/gui/view/graph-editor/src/lib.rs +++ b/app/gui/view/graph-editor/src/lib.rs @@ -124,6 +124,62 @@ fn traffic_lights_gap_width() -> f32 { } } +#[derive(Debug, Default)] +pub struct Valgrind { + creation_backtraces: HashMap, + next_id: usize, +} + +thread_local! { + pub static VALGRIND: RefCell = default(); +} + +#[derive(Debug)] +struct TheGuardian { + name: &'static str, + instance_id: usize, +} + +impl TheGuardian { + fn new(name: &'static str) -> Self { + warn!("CREATED \"{name}\""); + let bt = backtrace(); + let instance_id = VALGRIND.with(move |v| { + let mut v = v.borrow_mut(); + let id = v.next_id; + v.next_id += 1; + v.creation_backtraces.insert(id, bt); + id + }); + Self { name, instance_id } + } +} + +impl Clone for TheGuardian { + fn clone(&self) -> Self { + let bt = backtrace(); + let instance_id = VALGRIND.with(move |v| { + let mut v = v.borrow_mut(); + let id = v.next_id; + v.next_id += 1; + v.creation_backtraces.insert(id, bt); + id + }); + Self { name: self.name, instance_id } + } +} + +impl_clone_ref_as_clone!(TheGuardian); + +impl Drop for TheGuardian { + fn drop(&mut self) { + warn!("DROPPING \"{}\"", self.name); + VALGRIND.with(|v| { + v.borrow_mut().creation_backtraces.remove(&self.instance_id); + }) + } +} + // ================= // === SharedVec === @@ -764,7 +820,7 @@ ensogl::define_endpoints_2! { impl FrpNetworkProvider for GraphEditor { fn network(&self) -> &frp::Network { - &self.model.network + &self.frp.network } } @@ -1391,11 +1447,12 @@ pub fn crumbs_overlap(src: &[span_tree::Crumb], tgt: &[span_tree::Crumb]) -> boo // === GraphEditorModelWithNetwork === // =================================== -#[derive(Debug, Clone, CloneRef)] +#[derive(Clone, CloneRef, Debug)] #[allow(missing_docs)] // FIXME[everyone] Public-facing API should be documented. pub struct GraphEditorModelWithNetwork { - pub model: GraphEditorModel, - pub network: frp::Network, + pub model: GraphEditorModel, + pub network: frp::WeakNetwork, + the_guardian: TheGuardian, } impl Deref for GraphEditorModelWithNetwork { @@ -1409,9 +1466,9 @@ impl Deref for GraphEditorModelWithNetwork { impl GraphEditorModelWithNetwork { /// Constructor. pub fn new(app: &Application, cursor: cursor::Cursor, frp: &Frp) -> Self { - let network = frp.network().clone_ref(); // FIXME make weak + let network = frp.network().clone_ref().downgrade(); let model = GraphEditorModel::new(app, cursor, frp); - Self { model, network } + Self { model, network, the_guardian: TheGuardian::new("GraphEditorModelWithNetwork") } } fn is_node_connected_at_input(&self, node_id: NodeId, crumbs: &span_tree::Crumbs) -> bool { @@ -1444,11 +1501,12 @@ impl GraphEditorModelWithNetwork { let edge_id = edge.id(); self.add_child(&edge); self.edges.insert(edge.clone_ref()); - let network = &self.network; - frp::extend! { network - eval_ edge.view.frp.shape_events.mouse_down_primary (edge_click.emit(edge_id)); - eval_ edge.view.frp.shape_events.mouse_over (edge_over.emit(edge_id)); - eval_ edge.view.frp.shape_events.mouse_out (edge_out.emit(edge_id)); + if let Some(network) = &self.network.upgrade_or_warn() { + frp::extend! { network + eval_ edge.view.frp.shape_events.mouse_down_primary (edge_click.emit(edge_id)); + eval_ edge.view.frp.shape_events.mouse_over (edge_over.emit(edge_id)); + eval_ edge.view.frp.shape_events.mouse_out (edge_out.emit(edge_id)); + } } edge_id } @@ -1463,7 +1521,7 @@ impl GraphEditorModelWithNetwork { let first_detached = self.edges.detached_target.is_empty(); self.edges.detached_target.insert(edge_id); if first_detached { - self.frp.private.output.on_some_edges_targets_unset.emit(()); + self.frp.output.on_some_edges_targets_unset.emit(()); } edge_id } @@ -1478,7 +1536,7 @@ impl GraphEditorModelWithNetwork { let first_detached = self.edges.detached_source.is_empty(); self.edges.detached_source.insert(edge_id); if first_detached { - self.frp.private.output.on_some_edges_sources_unset.emit(()); + self.frp.output.on_some_edges_sources_unset.emit(()); } edge_id } @@ -1561,193 +1619,197 @@ impl GraphEditorModelWithNetwork { let model = &self.model; let NodeCreationContext { pointer_style, output_press, input_press, output } = ctx; - frp::new_bridge_network! { [self.network, node_network] graph_node_bridge - eval_ node.background_press(touch.nodes.down.emit(node_id)); + if let Some(network) = self.network.upgrade_or_warn() { + frp::new_bridge_network! { [network, node_network] graph_node_bridge + eval_ node.background_press(touch.nodes.down.emit(node_id)); - hovered <- node.output.hover.map (move |t| Some(Switch::new(node_id,*t))); - output.node_hovered <+ hovered; + hovered <- node.output.hover.map (move |t| Some(Switch::new(node_id,*t))); + output.node_hovered <+ hovered; - eval node.comment ([model](comment) - model.frp.private.output.node_comment_set.emit((node_id,comment.clone())) - ); + eval node.comment ([model](comment) + model.frp.output.node_comment_set.emit((node_id,comment.clone())) + ); - node.set_output_expression_visibility <+ self.frp.nodes_labels_visible; + node.set_output_expression_visibility <+ self.frp.output.nodes_labels_visible; - pointer_style <+ node_model.input.frp.pointer_style; + pointer_style <+ node_model.input.frp.pointer_style; - eval node_model.output.frp.on_port_press ([output_press](crumbs){ - let target = EdgeEndpoint::new(node_id,crumbs.clone()); - output_press.emit(target); - }); + eval node_model.output.frp.on_port_press ([output_press](crumbs){ + let target = EdgeEndpoint::new(node_id,crumbs.clone()); + output_press.emit(target); + }); - eval node_model.input.frp.on_port_press ([input_press](crumbs) - let target = EdgeEndpoint::new(node_id,crumbs.clone()); - input_press.emit(target); - ); + eval node_model.input.frp.on_port_press ([input_press](crumbs) + let target = EdgeEndpoint::new(node_id,crumbs.clone()); + input_press.emit(target); + ); - eval node_model.input.frp.on_port_hover ([model](t) { - let crumbs = t.on(); - let target = crumbs.map(|c| EdgeEndpoint::new(node_id,c.clone())); - model.frp.private.output.hover_node_input.emit(target); - }); + eval node_model.input.frp.on_port_hover ([model](t) { + let crumbs = t.on(); + let target = crumbs.map(|c| EdgeEndpoint::new(node_id,c.clone())); + model.frp.output.hover_node_input.emit(target); + }); - eval node_model.output.frp.on_port_hover ([model](hover) { - let output = hover.on().map(|crumbs| EdgeEndpoint::new(node_id,crumbs.clone())); - model.frp.private.output.hover_node_output.emit(output); - }); + eval node_model.output.frp.on_port_hover ([model](hover) { + let output = hover.on().map(|crumbs| EdgeEndpoint::new(node_id,crumbs.clone())); + model.frp.output.hover_node_output.emit(output); + }); - let neutral_color = model.styles_frp.get_color(theme::code::types::any::selection); - - _eval <- all_with(&node_model.input.frp.on_port_type_change,&neutral_color, - f!(((crumbs,_),neutral_color) - model.with_input_edge_id(node_id,crumbs,|id| - model.refresh_edge_color(id,neutral_color.into()) - ) - )); - - _eval <- all_with(&node_model.input.frp.on_port_type_change,&neutral_color, - f!(((crumbs,_),neutral_color) - model.with_output_edge_id(node_id,crumbs,|id| - model.refresh_edge_color(id,neutral_color.into()) - ) - )); - - let is_editing = &node_model.input.frp.editing; - expression_change_temporary <- node.on_expression_modified.gate(is_editing); - expression_change_permanent <- node.on_expression_modified.gate_not(is_editing); - - temporary_expression <- expression_change_temporary.map2( - &node_model.input.set_expression, - move |(crumbs, code), expr| expr.code_with_replaced_span(crumbs, code) - ); - eval temporary_expression([model] (code) { - model.frp.private.output.node_expression_set.emit((node_id, code)); - }); - eval expression_change_permanent([model]((crumbs,code)) { - let args = (node_id, crumbs.clone(), code.clone()); - model.frp.private.output.node_expression_span_set.emit(args) - }); + let neutral_color = model.styles_frp.get_color(theme::code::types::any::selection); + + _eval <- all_with(&node_model.input.frp.on_port_type_change,&neutral_color, + f!(((crumbs,_),neutral_color) + model.with_input_edge_id(node_id,crumbs,|id| + model.refresh_edge_color(id,neutral_color.into()) + ) + )); + + _eval <- all_with(&node_model.input.frp.on_port_type_change,&neutral_color, + f!(((crumbs,_),neutral_color) + model.with_output_edge_id(node_id,crumbs,|id| + model.refresh_edge_color(id,neutral_color.into()) + ) + )); + + let is_editing = &node_model.input.frp.editing; + expression_change_temporary <- node.on_expression_modified.gate(is_editing); + expression_change_permanent <- node.on_expression_modified.gate_not(is_editing); + + temporary_expression <- expression_change_temporary.map2( + &node_model.input.set_expression, + move |(crumbs, code), expr| expr.code_with_replaced_span(crumbs, code) + ); + eval temporary_expression([model] (code) { + model.frp.output.node_expression_set.emit((node_id, code)); + }); + eval expression_change_permanent([model]((crumbs,code)) { + let args = (node_id, crumbs.clone(), code.clone()); + model.frp.output.node_expression_span_set.emit(args) + }); - eval node.requested_widgets([model]((call_id, target_id)) { - let args = (node_id, *call_id, *target_id); - model.frp.private.output.widgets_requested.emit(args) - }); + eval node.requested_widgets([model]((call_id, target_id)) { + let args = (node_id, *call_id, *target_id); + model.frp.output.widgets_requested.emit(args) + }); - let node_expression_edit = node.model().input.expression_edit.clone_ref(); - model.frp.private.output.node_expression_edited <+ node_expression_edit.map( - move |(expr, selection)| (node_id, expr.clone_ref(), selection.clone()) - ); - model.frp.private.output.request_import <+ node.request_import; + let node_expression_edit = node.model().input.expression_edit.clone_ref(); + model.frp.output.node_expression_edited <+ node_expression_edit.map( + move |(expr, selection)| (node_id, expr.clone_ref(), selection.clone()) + ); + model.frp.output.request_import <+ node.request_import; - // === Actions === + // === Actions === - model.frp.private.output.node_action_context_switch <+ node.view.context_switch.map( - f!([] (active) (node_id, *active)) - ); + model.frp.output.node_action_context_switch <+ node.view.context_switch.map( + f!([] (active) (node_id, *active)) + ); - eval node.view.freeze ((is_frozen) { - model.frp.private.output.node_action_freeze.emit((node_id,*is_frozen)); - }); + eval node.view.freeze ((is_frozen) { + model.frp.output.node_action_freeze.emit((node_id,*is_frozen)); + }); - let set_node_disabled = &node.set_disabled; - eval node.view.skip ([set_node_disabled,model](is_skipped) { - model.frp.private.output.node_action_skip.emit((node_id,*is_skipped)); - set_node_disabled.emit(is_skipped); - }); + let set_node_disabled = &node.set_disabled; + eval node.view.skip ([set_node_disabled,model](is_skipped) { + model.frp.output.node_action_skip.emit((node_id,*is_skipped)); + set_node_disabled.emit(is_skipped); + }); - // === Visualizations === + // === Visualizations === - visualization_shown <- node.visualization_visible.gate(&node.visualization_visible); - visualization_hidden <- node.visualization_visible.gate_not(&node.visualization_visible); + visualization_shown <- node.visualization_visible.gate(&node.visualization_visible); + visualization_hidden <- node.visualization_visible.gate_not(&node.visualization_visible); - let vis_is_selected = node_model.visualization.frp.is_selected.clone_ref(); + let vis_is_selected = node_model.visualization.frp.is_selected.clone_ref(); - selected <- vis_is_selected.on_true(); - deselected <- vis_is_selected.on_false(); - output.on_visualization_select <+ selected.constant(Switch::On(node_id)); - output.on_visualization_select <+ deselected.constant(Switch::Off(node_id)); + selected <- vis_is_selected.on_true(); + deselected <- vis_is_selected.on_false(); + output.on_visualization_select <+ selected.constant(Switch::On(node_id)); + output.on_visualization_select <+ deselected.constant(Switch::Off(node_id)); - preprocessor_changed <- - node_model.visualization.frp.preprocessor.map(move |preprocessor| { - (node_id,preprocessor.clone()) - }); - output.visualization_preprocessor_changed <+ preprocessor_changed.gate(&node.visualization_visible); + preprocessor_changed <- + node_model.visualization.frp.preprocessor.map(move |preprocessor| { + (node_id,preprocessor.clone()) + }); + output.visualization_preprocessor_changed <+ preprocessor_changed.gate(&node.visualization_visible); - metadata <- any(...); - metadata <+ node_model.visualization.frp.preprocessor.map(visualization::Metadata::new); + metadata <- any(...); + metadata <+ node_model.visualization.frp.preprocessor.map(visualization::Metadata::new); - // Ensure the graph editor knows about internal changes to the visualisation. If the - // visualisation changes that should indicate that the old one has been disabled and a - // new one has been enabled. - // TODO: Create a better API for updating the controller about visualisation changes - // (see #896) - output.visualization_hidden <+ visualization_hidden.constant(node_id); - output.visualization_shown <+ - visualization_shown.map2(&metadata,move |_,metadata| (node_id,metadata.clone())); + // Ensure the graph editor knows about internal changes to the visualisation. If the + // visualisation changes that should indicate that the old one has been disabled and a + // new one has been enabled. + // TODO: Create a better API for updating the controller about visualisation changes + // (see #896) + output.visualization_hidden <+ visualization_hidden.constant(node_id); + output.visualization_shown <+ + visualization_shown.map2(&metadata,move |_,metadata| (node_id,metadata.clone())); - init <- source::<()>(); - enabled_visualization_path <- init.all_with3( - &node.visualization_enabled, &node.visualization_path, - move |_init, is_enabled, path| (node_id, is_enabled.and_option(path.clone())) - ); - output.enabled_visualization_path <+ enabled_visualization_path; + init <- source::<()>(); + enabled_visualization_path <- init.all_with3( + &node.visualization_enabled, &node.visualization_path, + move |_init, is_enabled, path| (node_id, is_enabled.and_option(path.clone())) + ); + output.enabled_visualization_path <+ enabled_visualization_path; - // === View Mode === + // === View Mode === - node.set_view_mode <+ self.model.frp.view_mode; + node.set_view_mode <+ self.model.frp.output.view_mode; - // === Profiling === + // === Profiling === - let profiling_min_duration = &self.model.profiling_statuses.min_duration; - node.set_profiling_min_global_duration <+ self.model.profiling_statuses.min_duration; - node.set_profiling_min_global_duration(profiling_min_duration.value()); - let profiling_max_duration = &self.model.profiling_statuses.max_duration; - node.set_profiling_max_global_duration <+ self.model.profiling_statuses.max_duration; - node.set_profiling_max_global_duration(profiling_max_duration.value()); + let profiling_min_duration = &self.model.profiling_statuses.min_duration; + node.set_profiling_min_global_duration <+ self.model.profiling_statuses.min_duration; + node.set_profiling_min_global_duration(profiling_min_duration.value()); + let profiling_max_duration = &self.model.profiling_statuses.max_duration; + node.set_profiling_max_global_duration <+ self.model.profiling_statuses.max_duration; + node.set_profiling_max_global_duration(profiling_max_duration.value()); - // === Execution Environment === + // === Execution Environment === - node.set_execution_environment <+ self.model.frp.set_execution_environment; - } + node.set_execution_environment <+ self.model.frp.input.set_execution_environment; + } - // === Panning camera to created node === - - // Node position and bounding box are not available immediately after the node is created, - // but only after the Node's display object is updated. Therefore, in order to pan the - // camera to the bounding box of a newly created node, we need to wait until: - // 1. the position of the newly created node becomes updated, and then - // 2. the bounding box of the node becomes updated. - // When the sequence is detected, and if the node is being edited, we pan the camera to it. - // Regardless whether the node is being edited, we drop the network, as we don't want to - // pan the camera for any later updates of the bounding box. - let pan_network = frp::Network::new("network_for_camera_pan_to_new_node"); - let pan_network_container = RefCell::new(Some(pan_network.clone())); - frp::new_bridge_network! { [self.network, node_network, pan_network] graph_node_pan_bridge - pos_updated <- node.output.position.constant(true); - bbox_updated_after_pos_updated <- node.output.bounding_box.gate(&pos_updated); - let node_being_edited = &self.frp.node_being_edited; - _eval <- bbox_updated_after_pos_updated.map2(node_being_edited, f!([model](_, node) { - pan_network_container.replace(None); - if *node == Some(node_id) { - model.pan_camera_to_node(node_id); - } - })); + // === Panning camera to created node === + + // Node position and bounding box are not available immediately after the node is + // created, but only after the Node's display object is updated. Therefore, + // in order to pan the camera to the bounding box of a newly created node, + // we need to wait until: 1. the position of the newly created node becomes + // updated, and then 2. the bounding box of the node becomes updated. + // When the sequence is detected, and if the node is being edited, we pan the camera to + // it. Regardless whether the node is being edited, we drop the network, as + // we don't want to pan the camera for any later updates of the bounding + // box. + let pan_network = frp::Network::new("network_for_camera_pan_to_new_node"); + let pan_network_container = RefCell::new(Some(pan_network.clone())); + frp::new_bridge_network! { [network, node_network, pan_network] graph_node_pan_bridge + pos_updated <- node.output.position.constant(true); + bbox_updated_after_pos_updated <- node.output.bounding_box.gate(&pos_updated); + let node_being_edited = &self.frp.output.node_being_edited; + _eval <- bbox_updated_after_pos_updated.map2(node_being_edited, f!([model](_, node) { + pan_network_container.replace(None); + if *node == Some(node_id) { + model.pan_camera_to_node(node_id); + } + })); + } + + node.set_view_mode(self.model.frp_public.output.view_mode.value()); + let initial_metadata = visualization::Metadata { + preprocessor: node_model.visualization.frp.preprocessor.value(), + }; + metadata.emit(initial_metadata); + init.emit(()); } - node.set_view_mode(self.model.frp.view_mode.value()); - let initial_metadata = visualization::Metadata { - preprocessor: node_model.visualization.frp.preprocessor.value(), - }; - metadata.emit(initial_metadata); - init.emit(()); self.nodes.insert(node_id, node.clone_ref()); node } @@ -1775,12 +1837,14 @@ pub struct GraphEditorModel { tooltip: Tooltip, touch_state: TouchState, visualisations: Visualisations, - frp: Frp, + frp: api::Private, + frp_public: api::Public, profiling_statuses: profiling::Statuses, profiling_button: component::profiling::Button, styles_frp: StyleWatchFrp, selection_controller: selection::Controller, execution_mode_selector: execution_mode_selector::ExecutionModeSelector, + the_guardian: TheGuardian, } @@ -1801,7 +1865,6 @@ impl GraphEditorModel { let execution_mode_selector = execution_mode_selector::ExecutionModeSelector::new(app); let app = app.clone_ref(); - let frp = frp.clone_ref(); let navigator = Navigator::new(scene, &scene.camera()); let tooltip = Tooltip::new(&app); let profiling_statuses = profiling::Statuses::new(); @@ -1830,14 +1893,16 @@ impl GraphEditorModel { tooltip, touch_state, visualisations, - frp, navigator, profiling_statuses, profiling_button, add_node_button, + frp: frp.private.clone_ref(), + frp_public: frp.public.clone_ref(), styles_frp, selection_controller, execution_mode_selector, + the_guardian: TheGuardian::new("GraphEditorModel"), } .init() } @@ -1871,8 +1936,8 @@ impl GraphEditorModel { impl GraphEditorModel { /// Create a new node and return a unique identifier. pub fn add_node(&self) -> NodeId { - self.frp.add_node.emit(()); - let (node_id, _, _) = self.frp.node_added.value(); + self.frp_public.input.add_node.emit(()); + let (node_id, _, _) = self.frp_public.output.node_added.value(); node_id } @@ -1885,7 +1950,7 @@ impl GraphEditorModel { /// Create a new node and place it at `pos`. pub fn add_node_at(&self, pos: Vector2) -> NodeId { let node_id = self.add_node(); - self.frp.set_node_position((node_id, pos)); + self.frp_public.input.set_node_position.emit((node_id, pos)); node_id } } @@ -1973,7 +2038,7 @@ impl GraphEditorModel { let node_id = node_id.into(); self.nodes.remove(&node_id); self.nodes.selected.remove_item(&node_id); - self.frp.private.output.on_visualization_select.emit(Switch::Off(node_id)); + self.frp.output.on_visualization_select.emit(Switch::Off(node_id)); } fn node_in_edges(&self, node_id: impl Into) -> Vec { @@ -2093,7 +2158,7 @@ impl GraphEditorModel { self.refresh_edge_position(edge_id); self.refresh_edge_source_size(edge_id); if first_detached { - self.frp.private.output.on_some_edges_sources_unset.emit(()); + self.frp.output.on_some_edges_sources_unset.emit(()); } } } @@ -2110,7 +2175,7 @@ impl GraphEditorModel { self.edges.detached_target.remove(&edge_id); let all_attached = self.edges.detached_target.is_empty(); if all_attached { - self.frp.private.output.on_all_edges_targets_set.emit(()); + self.frp.output.on_all_edges_targets_set.emit(()); } edge.view.frp.target_attached.emit(true); @@ -2130,7 +2195,7 @@ impl GraphEditorModel { edge.view.frp.target_attached.emit(false); self.refresh_edge_position(edge_id); if first_detached { - self.frp.private.output.on_some_edges_targets_unset.emit(()); + self.frp.output.on_some_edges_targets_unset.emit(()); } }; } @@ -2171,10 +2236,10 @@ impl GraphEditorModel { let no_detached_sources = self.edges.detached_source.is_empty(); let no_detached_targets = self.edges.detached_target.is_empty(); if no_detached_targets { - self.frp.private.output.on_all_edges_targets_set.emit(()); + self.frp.output.on_all_edges_targets_set.emit(()); } if no_detached_sources { - self.frp.private.output.on_all_edges_sources_set.emit(()); + self.frp.output.on_all_edges_sources_set.emit(()); } } @@ -2472,7 +2537,7 @@ impl GraphEditorModel { } fn edge_hover_type(&self) -> Option { - let hover_tgt = self.frp.hover_node_input.value(); + let hover_tgt = self.frp_public.output.hover_node_input.value(); hover_tgt.and_then(|tgt| { self.with_node(tgt.node_id, |node| node.model().input.port_type(&tgt.port)).flatten() }) @@ -2495,7 +2560,7 @@ impl GraphEditorModel { // FIXME : StyleWatch is unsuitable here, as it was designed as an internal tool for shape // system (#795) let styles = StyleWatch::new(&self.scene().style_sheet); - match self.frp.view_mode.value() { + match self.frp_public.output.view_mode.value() { view::Mode::Normal => { let edge_type = self .edge_hover_type() @@ -2620,8 +2685,9 @@ impl display::Object for GraphEditorModel { #[derive(Debug, Clone, CloneRef)] #[allow(missing_docs)] // FIXME[everyone] Public-facing API should be documented. pub struct GraphEditor { - pub model: GraphEditorModelWithNetwork, - pub frp: Frp, + pub model: GraphEditorModelWithNetwork, + pub frp: Frp, + the_guardian: TheGuardian, } impl GraphEditor { @@ -2750,11 +2816,12 @@ fn new_graph_editor(app: &Application) -> GraphEditor { let scene = &world.default_scene; let cursor = &app.cursor; let frp = Frp::new(); + frp.network().guardian.enable(); let model = GraphEditorModelWithNetwork::new(app, cursor.clone_ref(), &frp); let network = frp.network(); let nodes = &model.nodes; let edges = &model.edges; - let inputs = &model.frp; + let inputs = &frp.private.input; let mouse = &scene.mouse.frp_deprecated; let touch = &model.touch_state; let vis_registry = &model.vis_registry; @@ -2810,7 +2877,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { // Go level down on node double click. enter_on_node <= target_to_enter.map(|target| target.is_symbol().as_some(())); - output_port_is_hovered <- inputs.hover_node_output.map(Option::is_some); + output_port_is_hovered <- frp.output.hover_node_output.map(Option::is_some); enter_node <- enter_on_node.gate_not(&output_port_is_hovered); node_switch_to_enter <- out.node_hovered.sample(&enter_node).unwrap(); node_to_enter <- node_switch_to_enter.map(|switch| switch.on().cloned()).unwrap(); @@ -2886,12 +2953,12 @@ fn new_graph_editor(app: &Application) -> GraphEditor { on_connect_follow_mode <- any(on_output_connect_follow_mode,on_input_connect_follow_mode); connect_drag_mode <- any(on_connect_drag_mode,on_connect_follow_mode); - on_detached_edge <- any(&inputs.on_some_edges_targets_unset,&inputs.on_some_edges_sources_unset); + on_detached_edge <- any(&frp.private.output.on_some_edges_targets_unset,&frp.private.output.on_some_edges_sources_unset); has_detached_edge <- bool(&out.on_all_edges_endpoints_set,&on_detached_edge); out.has_detached_edge <+ has_detached_edge; - eval node_input_touch.down ((target) model.frp.press_node_input.emit(target)); - eval node_output_touch.down ((target) model.frp.press_node_output.emit(target)); + frp.press_node_input <+ node_input_touch.down; + frp.press_node_output <+ node_output_touch.down; } @@ -2955,10 +3022,10 @@ fn new_graph_editor(app: &Application) -> GraphEditor { output_down <- node_output_touch.down.constant(()); input_down <- node_input_touch.down.constant(()); - has_detached_edge_on_output_down <- has_detached_edge.sample(&inputs.hover_node_output); + has_detached_edge_on_output_down <- has_detached_edge.sample(&frp.output.hover_node_output); - port_input_mouse_up <- inputs.hover_node_input.sample(&mouse.up_primary).unwrap(); - port_output_mouse_up <- inputs.hover_node_output.sample(&mouse.up_primary).unwrap(); + port_input_mouse_up <- frp.output.hover_node_input.sample(&mouse.up_primary).unwrap(); + port_output_mouse_up <- frp.output.hover_node_output.sample(&mouse.up_primary).unwrap(); attach_all_edge_inputs <- any (port_input_mouse_up, inputs.press_node_input, inputs.set_detached_edge_targets); attach_all_edge_outputs <- any (port_output_mouse_up, inputs.press_node_output, inputs.set_detached_edge_sources); @@ -2967,7 +3034,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { create_edge_from_input <- node_input_touch.down.map(|value| value.clone()); on_new_edge <- any(&output_down,&input_down); - let selection_mode = selection::get_mode(network,inputs); + let selection_mode = selection::get_mode(network,&frp); keep_selection <- selection_mode.map(|t| *t != selection::Mode::Normal); deselect_edges <- on_new_edge.gate_not(&keep_selection); eval_ deselect_edges ( model.clear_all_detached_edges() ); @@ -3045,7 +3112,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { frp::extend! { network let node_added_with_button = model.add_node_button.clicked.clone_ref(); - input_start_node_creation_from_port <- inputs.hover_node_output.sample( + input_start_node_creation_from_port <- out.hover_node_output.sample( &inputs.start_node_creation_from_port); start_node_creation_from_port <- input_start_node_creation_from_port.filter_map( |v| v.clone()); @@ -3188,13 +3255,12 @@ fn new_graph_editor(app: &Application) -> GraphEditor { // === Remove Node === frp::extend! { network + all_nodes <= inputs.remove_all_nodes . map(f_!(model.all_nodes())); + selected_nodes <= inputs.remove_selected_nodes . map(f_!(model.nodes.all_selected())); + nodes_to_remove <- any (all_nodes, selected_nodes); + frp.public.input.remove_all_node_edges <+ nodes_to_remove; - all_nodes <= inputs.remove_all_nodes . map(f_!(model.all_nodes())); - selected_nodes <= inputs.remove_selected_nodes . map(f_!(model.nodes.all_selected())); - nodes_to_remove <- any (all_nodes, selected_nodes); - eval nodes_to_remove ((node_id) inputs.remove_all_node_edges.emit(node_id)); - - out.node_removed <+ nodes_to_remove; + out.node_removed <+ nodes_to_remove; } @@ -3374,13 +3440,13 @@ fn new_graph_editor(app: &Application) -> GraphEditor { frp::extend! { network - detached_edge <- any(&inputs.on_some_edges_targets_unset,&inputs.on_some_edges_sources_unset); + detached_edge <- any(&out.on_some_edges_targets_unset,&out.on_some_edges_sources_unset); update_edge <- any(detached_edge,on_new_edge_source,on_new_edge_target); cursor_pos_on_update <- cursor_pos_in_scene.sample(&update_edge); edge_refresh_cursor_pos <- any(cursor_pos_on_update,cursor_pos_in_scene); - is_hovering_output <- inputs.hover_node_output.map(|target| target.is_some()).sampler(); - hover_node <- inputs.hover_node_output.unwrap(); + is_hovering_output <- out.hover_node_output.map(|target| target.is_some()).sampler(); + hover_node <- out.hover_node_output.unwrap(); edge_refresh_on_node_hover <- all(edge_refresh_cursor_pos,hover_node).gate(&is_hovering_output); edge_refresh_cursor_pos_no_hover <- edge_refresh_cursor_pos.gate_not(&is_hovering_output); @@ -3476,17 +3542,6 @@ fn new_graph_editor(app: &Application) -> GraphEditor { // === Vis Update Data === frp::extend! { network - // TODO remove this once real data is available. - let sample_data_generator = MockDataGenerator3D::default(); - def _set_dumy_data = inputs.debug_set_test_visualization_data_for_selected_node.map(f!([nodes,inputs](_) { - for node_id in &*nodes.selected.raw.borrow() { - let data = Rc::new(sample_data_generator.generate_data()); // FIXME: why rc? - let content = serde_json::to_value(data).unwrap(); - let data = visualization::Data::from(content); - inputs.set_visualization_data.emit((*node_id,data)); - } - })); - eval inputs.set_visualization_data ([nodes]((node_id,data)) { if let Some(node) = nodes.get_cloned(node_id) { node.model().visualization.frp.set_data.emit(data); @@ -3601,9 +3656,9 @@ fn new_graph_editor(app: &Application) -> GraphEditor { node_to_enter <= inputs.enter_selected_node.map(f_!(model.nodes.last_selected())); out.node_entered <+ node_to_enter; - removed_edges_on_enter <= out.node_entered.map(f_!(model.model.clear_all_detached_edges())); + removed_edges_on_enter <= out.node_entered.map(f_!(model.clear_all_detached_edges())); out.node_exited <+ inputs.exit_node; - removed_edges_on_exit <= out.node_exited.map(f_!(model.model.clear_all_detached_edges())); + removed_edges_on_exit <= out.node_exited.map(f_!(model.clear_all_detached_edges())); out.on_edge_drop <+ any(removed_edges_on_enter,removed_edges_on_exit); @@ -3645,7 +3700,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { out.on_edge_only_source_not_set <+ out.on_edge_target_set_with_source_not_set._0(); out.on_edge_only_source_not_set <+ out.on_edge_source_unset._0(); - let neutral_color = model.model.styles_frp.get_color(theme::code::types::any::selection); + let neutral_color = model.styles_frp.get_color(theme::code::types::any::selection); eval out.on_edge_source_set ([model,neutral_color]((id, _)) model.refresh_edge_color(*id,neutral_color.value().into())); eval out.on_edge_target_set ([model,neutral_color]((id, _)) @@ -3923,7 +3978,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { frp.edit_mode_off.emit(()); frp.set_debug_mode.emit(false); - GraphEditor { model, frp } + GraphEditor { model, frp, the_guardian: TheGuardian::new("GraphEditor") } } diff --git a/app/gui/view/graph-editor/src/new_node_position.rs b/app/gui/view/graph-editor/src/new_node_position.rs index 753a6b44967e..f56c1cfa8f6f 100644 --- a/app/gui/view/graph-editor/src/new_node_position.rs +++ b/app/gui/view/graph-editor/src/new_node_position.rs @@ -117,7 +117,7 @@ pub fn below_line_and_left_aligned( line_y: f32, align_x: f32, ) -> Vector2 { - let y_gap = graph_editor.frp.default_y_gap_between_nodes.value(); + let y_gap = graph_editor.frp_public.output.default_y_gap_between_nodes.value(); let y_offset = y_gap + node::HEIGHT / 2.0; let starting_point = Vector2(align_x, line_y - y_offset); let direction = Vector2(-1.0, 0.0); @@ -245,10 +245,10 @@ pub fn on_ray( starting_point: Vector2, direction: Vector2, ) -> Option { - let x_gap = graph_editor.frp.default_x_gap_between_nodes.value(); - let y_gap = graph_editor.frp.default_y_gap_between_nodes.value(); + let x_gap = graph_editor.frp_public.output.default_x_gap_between_nodes.value(); + let y_gap = graph_editor.frp_public.output.default_y_gap_between_nodes.value(); // This is how much horizontal space we are looking for. - let min_spacing = graph_editor.frp.min_x_spacing_for_new_nodes.value(); + let min_spacing = graph_editor.frp_public.output.min_x_spacing_for_new_nodes.value(); let nodes = graph_editor.nodes.all.raw.borrow(); // The "occupied area" for given node consists of: // - area taken by node view (obviously); diff --git a/lib/rust/frp/src/network.rs b/lib/rust/frp/src/network.rs index c54622029da1..eccbf8d032f2 100644 --- a/lib/rust/frp/src/network.rs +++ b/lib/rust/frp/src/network.rs @@ -23,6 +23,69 @@ pub struct NetworkId(usize); // === Network === // =============== +#[derive(Debug, Default)] +pub struct Valgrind { + pub creation_backtraces: HashMap, + pub next_id: usize, +} + +thread_local! { + pub static VALGRIND: RefCell = default(); +} + +#[derive(Debug)] +pub struct TheGuardian { + name: &'static str, + instance_id: Cell>, +} + +impl TheGuardian { + pub fn new(name: &'static str) -> Self { + Self { name, instance_id: default() } + } + + pub fn enable(&self) { + let bt = backtrace(); + let instance_id = VALGRIND.with(move |v| { + let mut v = v.borrow_mut(); + let id = v.next_id; + v.next_id += 1; + v.creation_backtraces.insert(id, bt); + id + }); + self.instance_id.set(Some(instance_id)); + } +} + +impl Clone for TheGuardian { + fn clone(&self) -> Self { + let instance_id = self.instance_id.get().is_some().then(|| { + let bt = backtrace(); + let instance_id = VALGRIND.with(move |v| { + let mut v = v.borrow_mut(); + let id = v.next_id; + v.next_id += 1; + v.creation_backtraces.insert(id, bt); + id + }); + instance_id + }); + Self { name: self.name, instance_id: Cell::new(instance_id) } + } +} + +impl_clone_ref_as_clone!(TheGuardian); + +impl Drop for TheGuardian { + fn drop(&mut self) { + VALGRIND.with(|v| { + if let Some(instance_id) = self.instance_id.get() { + v.borrow_mut().creation_backtraces.remove(&instance_id); + } + }) + } +} + // === Definition === /// Network manages lifetime of set of FRP nodes. FRP networks are designed to be static. You can @@ -30,7 +93,8 @@ pub struct NetworkId(usize); /// Moreover, you should not grow the FRP network after it is constructed. #[derive(Clone, CloneRef, Debug)] pub struct Network { - data: Rc, + data: Rc, + pub guardian: TheGuardian, } /// Weak version of `Network`. @@ -86,7 +150,8 @@ impl Network { /// Non-generic constructor. fn new_with_string(label: String) -> Self { let data = Rc::new(NetworkData::new(label)); - Self { data } + let guardian = TheGuardian::new("Network"); + Self { data, guardian } } /// Get the weak version. @@ -166,7 +231,8 @@ impl Network { impl WeakNetwork { /// Upgrade to strong reference. pub fn upgrade(&self) -> Option { - self.data.upgrade().map(|data| Network { data }) + let guardian = TheGuardian::new("UpgradedNetwork"); + self.data.upgrade().map(|data| Network { data, guardian }) } /// Upgrade to string reference, printing warning if returning `None`. To be used in places From 03a8e4e6c515361cfc0bb4b1e1f2e3ac4fb79b94 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Thu, 20 Apr 2023 11:07:16 +0200 Subject: [PATCH 2/9] Fixed all leaks, but code needs to be cleaned up --- app/gui/src/lib.rs | 5 +-- .../graph-editor/src/component/breadcrumbs.rs | 32 +++++++++++-------- .../src/component/breadcrumbs/project_name.rs | 10 ++++-- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/app/gui/src/lib.rs b/app/gui/src/lib.rs index 7a9757dce3fb..26ab3a250d53 100644 --- a/app/gui/src/lib.rs +++ b/app/gui/src/lib.rs @@ -201,8 +201,5 @@ pub fn drop() { IDE.with(RefCell::take); EXECUTOR.with(RefCell::take); // warn!("Graphs: {:#?}", ide_view::graph_editor::VALGRIND.with(|v| v.take())); - warn!( - "Networks: {:#?}", - enso_frp::network::VALGRIND.with(|v| v.take().creation_backtraces.into_iter().next()) - ); + warn!("Networks: {:#?}", enso_frp::network::VALGRIND.with(|v| v.take().creation_backtraces)); } diff --git a/app/gui/view/graph-editor/src/component/breadcrumbs.rs b/app/gui/view/graph-editor/src/component/breadcrumbs.rs index dacbd089e094..be20b52d72bf 100644 --- a/app/gui/view/graph-editor/src/component/breadcrumbs.rs +++ b/app/gui/view/graph-editor/src/component/breadcrumbs.rs @@ -9,6 +9,7 @@ use crate::LocalCall; use engine_protocol::language_server::MethodPointer; use enso_frp as frp; +use enso_frp::TheGuardian; use ensogl::application::Application; use ensogl::display; use ensogl::display::camera::Camera2d; @@ -413,8 +414,9 @@ impl display::Object for BreadcrumbsModel { #[derive(Debug, Clone, CloneRef)] #[allow(missing_docs)] pub struct Breadcrumbs { - model: Rc, - frp: Frp, + model: BreadcrumbsModel, + frp: Frp, + guardian: TheGuardian, } impl Breadcrumbs { @@ -422,7 +424,8 @@ impl Breadcrumbs { pub fn new(app: Application) -> Self { let scene = app.display.default_scene.clone_ref(); let frp = Frp::new(); - let model = Rc::new(BreadcrumbsModel::new(app, &frp)); + frp.network().guardian.enable(); + let model = BreadcrumbsModel::new(app, &frp); let network = &frp.network; // === Breadcrumb selection === @@ -431,9 +434,8 @@ impl Breadcrumbs { // === Selecting === - _breadcrumb_selection <- frp.select_breadcrumb.map(f!([model,frp](index) - frp.source.breadcrumb_select.emit(model.select_breadcrumb(*index)); - )); + frp.source.breadcrumb_select <+ + frp.select_breadcrumb.map(f!((index) model.select_breadcrumb(*index))); // === Stack Operations === @@ -485,14 +487,14 @@ impl Breadcrumbs { // === User Interaction === - eval_ model.project_name.frp.output.mouse_down(frp.select_breadcrumb.emit(0)); - eval_ frp.cancel_project_name_editing(model.project_name.frp.cancel_editing.emit(())); - eval_ frp.outside_press(model.project_name.frp.outside_press.emit(())); + frp.select_breadcrumb <+ model.project_name.frp.output.mouse_down.constant(0); + model.project_name.frp.cancel_editing <+ frp.cancel_project_name_editing; + model.project_name.frp.outside_press <+ frp.outside_press; popped_count <= frp.output.breadcrumb_select.map(|selected| (0..selected.0).collect_vec()); local_calls <= frp.output.breadcrumb_select.map(|selected| selected.1.clone()); - eval_ popped_count(frp.source.breadcrumb_pop.emit(())); - eval local_calls((local_call) frp.source.breadcrumb_push.emit(local_call)); + frp.source.breadcrumb_pop <+ popped_count.constant(()); + frp.source.breadcrumb_push <+ local_calls; // === Select === @@ -507,8 +509,8 @@ impl Breadcrumbs { popped_count <= selected.map(|selected| (0..selected.0).collect_vec()); local_calls <= selected.map(|selected| selected.1.clone()); - eval_ popped_count(frp.input.debug_pop_breadcrumb.emit(())); - eval local_calls((local_call) frp.input.debug_push_breadcrumb.emit(local_call)); + frp.input.debug_pop_breadcrumb <+ popped_count.constant(()); + frp.input.debug_push_breadcrumb <+ local_calls; // === Relayout === @@ -523,7 +525,9 @@ impl Breadcrumbs { } - Self { model, frp } + let guardian = TheGuardian::new("Breadcrumbs"); + // guardian.enable(); + Self { model, frp, guardian } } } diff --git a/app/gui/view/graph-editor/src/component/breadcrumbs/project_name.rs b/app/gui/view/graph-editor/src/component/breadcrumbs/project_name.rs index fdfc35ab2fa3..da47e5943265 100644 --- a/app/gui/view/graph-editor/src/component/breadcrumbs/project_name.rs +++ b/app/gui/view/graph-editor/src/component/breadcrumbs/project_name.rs @@ -9,6 +9,7 @@ use crate::component::breadcrumbs::TEXT_SIZE; use crate::component::breadcrumbs::VERTICAL_MARGIN; use enso_frp as frp; +use enso_frp::TheGuardian; use ensogl::application::shortcut; use ensogl::application::Application; use ensogl::data::color; @@ -238,8 +239,9 @@ impl display::Object for ProjectNameModel { #[derive(Debug, Clone, CloneRef)] #[allow(missing_docs)] pub struct ProjectName { - model: Rc, - pub frp: Frp, + model: Rc, + pub frp: Frp, + guardian: TheGuardian, } impl ProjectName { @@ -365,7 +367,9 @@ impl ProjectName { frp.deselect(); frp.input.set_name.emit(UNINITIALIZED_PROJECT_NAME.to_string()); - Self { model, frp } + let guardian = TheGuardian::new("ProjectName"); + // guardian.enable(); + Self { model, frp, guardian } } } From cc2d78d2148be11e1e176b31b663eb5e119718c2 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Thu, 20 Apr 2023 12:09:05 +0200 Subject: [PATCH 3/9] Cleanup --- app/gui/src/lib.rs | 12 ++- .../graph-editor/src/component/breadcrumbs.rs | 11 +-- .../src/component/breadcrumbs/project_name.rs | 10 +-- app/gui/view/graph-editor/src/lib.rs | 75 ++----------------- lib/rust/frp/src/network.rs | 72 +----------------- lib/rust/prelude/src/debug.rs | 69 +++++++++++++++++ 6 files changed, 95 insertions(+), 154 deletions(-) diff --git a/app/gui/src/lib.rs b/app/gui/src/lib.rs index 26ab3a250d53..488098f65f31 100644 --- a/app/gui/src/lib.rs +++ b/app/gui/src/lib.rs @@ -196,10 +196,18 @@ pub fn main() { // === IDE Drop === // ================ +/// Drop all structure created so far. +/// +/// All connections will be closed and all visuals will be removed. #[wasm_bindgen] pub fn drop() { IDE.with(RefCell::take); EXECUTOR.with(RefCell::take); - // warn!("Graphs: {:#?}", ide_view::graph_editor::VALGRIND.with(|v| v.take())); - warn!("Networks: {:#?}", enso_frp::network::VALGRIND.with(|v| v.take().creation_backtraces)); + leak_detector::TRACKED_OBJECTS.with(|objects| { + let objects = objects.borrow(); + if !objects.is_empty() { + error!("Tracked objects leaked after dropping entire application!"); + error!("{objects:#?}"); + } + }) } diff --git a/app/gui/view/graph-editor/src/component/breadcrumbs.rs b/app/gui/view/graph-editor/src/component/breadcrumbs.rs index be20b52d72bf..3f2f229492d6 100644 --- a/app/gui/view/graph-editor/src/component/breadcrumbs.rs +++ b/app/gui/view/graph-editor/src/component/breadcrumbs.rs @@ -9,7 +9,6 @@ use crate::LocalCall; use engine_protocol::language_server::MethodPointer; use enso_frp as frp; -use enso_frp::TheGuardian; use ensogl::application::Application; use ensogl::display; use ensogl::display::camera::Camera2d; @@ -414,9 +413,8 @@ impl display::Object for BreadcrumbsModel { #[derive(Debug, Clone, CloneRef)] #[allow(missing_docs)] pub struct Breadcrumbs { - model: BreadcrumbsModel, - frp: Frp, - guardian: TheGuardian, + model: BreadcrumbsModel, + frp: Frp, } impl Breadcrumbs { @@ -424,7 +422,6 @@ impl Breadcrumbs { pub fn new(app: Application) -> Self { let scene = app.display.default_scene.clone_ref(); let frp = Frp::new(); - frp.network().guardian.enable(); let model = BreadcrumbsModel::new(app, &frp); let network = &frp.network; @@ -525,9 +522,7 @@ impl Breadcrumbs { } - let guardian = TheGuardian::new("Breadcrumbs"); - // guardian.enable(); - Self { model, frp, guardian } + Self { model, frp } } } diff --git a/app/gui/view/graph-editor/src/component/breadcrumbs/project_name.rs b/app/gui/view/graph-editor/src/component/breadcrumbs/project_name.rs index da47e5943265..fdfc35ab2fa3 100644 --- a/app/gui/view/graph-editor/src/component/breadcrumbs/project_name.rs +++ b/app/gui/view/graph-editor/src/component/breadcrumbs/project_name.rs @@ -9,7 +9,6 @@ use crate::component::breadcrumbs::TEXT_SIZE; use crate::component::breadcrumbs::VERTICAL_MARGIN; use enso_frp as frp; -use enso_frp::TheGuardian; use ensogl::application::shortcut; use ensogl::application::Application; use ensogl::data::color; @@ -239,9 +238,8 @@ impl display::Object for ProjectNameModel { #[derive(Debug, Clone, CloneRef)] #[allow(missing_docs)] pub struct ProjectName { - model: Rc, - pub frp: Frp, - guardian: TheGuardian, + model: Rc, + pub frp: Frp, } impl ProjectName { @@ -367,9 +365,7 @@ impl ProjectName { frp.deselect(); frp.input.set_name.emit(UNINITIALIZED_PROJECT_NAME.to_string()); - let guardian = TheGuardian::new("ProjectName"); - // guardian.enable(); - Self { model, frp, guardian } + Self { model, frp } } } diff --git a/app/gui/view/graph-editor/src/lib.rs b/app/gui/view/graph-editor/src/lib.rs index 064063ce4f07..14265bfb0a28 100644 --- a/app/gui/view/graph-editor/src/lib.rs +++ b/app/gui/view/graph-editor/src/lib.rs @@ -48,7 +48,6 @@ use crate::component::node; use crate::component::type_coloring; use crate::component::visualization; use crate::component::visualization::instance::PreprocessorConfiguration; -use crate::component::visualization::MockDataGenerator3D; use crate::data::enso; pub use crate::node::profiling::Status as NodeProfilingStatus; @@ -124,61 +123,6 @@ fn traffic_lights_gap_width() -> f32 { } } -#[derive(Debug, Default)] -pub struct Valgrind { - creation_backtraces: HashMap, - next_id: usize, -} - -thread_local! { - pub static VALGRIND: RefCell = default(); -} - -#[derive(Debug)] -struct TheGuardian { - name: &'static str, - instance_id: usize, -} - -impl TheGuardian { - fn new(name: &'static str) -> Self { - warn!("CREATED \"{name}\""); - let bt = backtrace(); - let instance_id = VALGRIND.with(move |v| { - let mut v = v.borrow_mut(); - let id = v.next_id; - v.next_id += 1; - v.creation_backtraces.insert(id, bt); - id - }); - Self { name, instance_id } - } -} - -impl Clone for TheGuardian { - fn clone(&self) -> Self { - let bt = backtrace(); - let instance_id = VALGRIND.with(move |v| { - let mut v = v.borrow_mut(); - let id = v.next_id; - v.next_id += 1; - v.creation_backtraces.insert(id, bt); - id - }); - Self { name: self.name, instance_id } - } -} - -impl_clone_ref_as_clone!(TheGuardian); - -impl Drop for TheGuardian { - fn drop(&mut self) { - warn!("DROPPING \"{}\"", self.name); - VALGRIND.with(|v| { - v.borrow_mut().creation_backtraces.remove(&self.instance_id); - }) - } -} // ================= @@ -1450,9 +1394,8 @@ pub fn crumbs_overlap(src: &[span_tree::Crumb], tgt: &[span_tree::Crumb]) -> boo #[derive(Clone, CloneRef, Debug)] #[allow(missing_docs)] // FIXME[everyone] Public-facing API should be documented. pub struct GraphEditorModelWithNetwork { - pub model: GraphEditorModel, - pub network: frp::WeakNetwork, - the_guardian: TheGuardian, + pub model: GraphEditorModel, + pub network: frp::WeakNetwork, } impl Deref for GraphEditorModelWithNetwork { @@ -1468,7 +1411,7 @@ impl GraphEditorModelWithNetwork { pub fn new(app: &Application, cursor: cursor::Cursor, frp: &Frp) -> Self { let network = frp.network().clone_ref().downgrade(); let model = GraphEditorModel::new(app, cursor, frp); - Self { model, network, the_guardian: TheGuardian::new("GraphEditorModelWithNetwork") } + Self { model, network } } fn is_node_connected_at_input(&self, node_id: NodeId, crumbs: &span_tree::Crumbs) -> bool { @@ -1844,7 +1787,6 @@ pub struct GraphEditorModel { styles_frp: StyleWatchFrp, selection_controller: selection::Controller, execution_mode_selector: execution_mode_selector::ExecutionModeSelector, - the_guardian: TheGuardian, } @@ -1874,7 +1816,7 @@ impl GraphEditorModel { ensogl_drop_manager::Manager::new(&scene.dom.root.clone_ref().into(), scene); let styles_frp = StyleWatchFrp::new(&scene.style_sheet); let selection_controller = selection::Controller::new( - &frp, + frp, &app.cursor, &scene.mouse.frp_deprecated, &touch_state, @@ -1902,7 +1844,6 @@ impl GraphEditorModel { styles_frp, selection_controller, execution_mode_selector, - the_guardian: TheGuardian::new("GraphEditorModel"), } .init() } @@ -2685,9 +2626,8 @@ impl display::Object for GraphEditorModel { #[derive(Debug, Clone, CloneRef)] #[allow(missing_docs)] // FIXME[everyone] Public-facing API should be documented. pub struct GraphEditor { - pub model: GraphEditorModelWithNetwork, - pub frp: Frp, - the_guardian: TheGuardian, + pub model: GraphEditorModelWithNetwork, + pub frp: Frp, } impl GraphEditor { @@ -2816,7 +2756,6 @@ fn new_graph_editor(app: &Application) -> GraphEditor { let scene = &world.default_scene; let cursor = &app.cursor; let frp = Frp::new(); - frp.network().guardian.enable(); let model = GraphEditorModelWithNetwork::new(app, cursor.clone_ref(), &frp); let network = frp.network(); let nodes = &model.nodes; @@ -3978,7 +3917,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { frp.edit_mode_off.emit(()); frp.set_debug_mode.emit(false); - GraphEditor { model, frp, the_guardian: TheGuardian::new("GraphEditor") } + GraphEditor { model, frp } } diff --git a/lib/rust/frp/src/network.rs b/lib/rust/frp/src/network.rs index eccbf8d032f2..c54622029da1 100644 --- a/lib/rust/frp/src/network.rs +++ b/lib/rust/frp/src/network.rs @@ -23,69 +23,6 @@ pub struct NetworkId(usize); // === Network === // =============== -#[derive(Debug, Default)] -pub struct Valgrind { - pub creation_backtraces: HashMap, - pub next_id: usize, -} - -thread_local! { - pub static VALGRIND: RefCell = default(); -} - -#[derive(Debug)] -pub struct TheGuardian { - name: &'static str, - instance_id: Cell>, -} - -impl TheGuardian { - pub fn new(name: &'static str) -> Self { - Self { name, instance_id: default() } - } - - pub fn enable(&self) { - let bt = backtrace(); - let instance_id = VALGRIND.with(move |v| { - let mut v = v.borrow_mut(); - let id = v.next_id; - v.next_id += 1; - v.creation_backtraces.insert(id, bt); - id - }); - self.instance_id.set(Some(instance_id)); - } -} - -impl Clone for TheGuardian { - fn clone(&self) -> Self { - let instance_id = self.instance_id.get().is_some().then(|| { - let bt = backtrace(); - let instance_id = VALGRIND.with(move |v| { - let mut v = v.borrow_mut(); - let id = v.next_id; - v.next_id += 1; - v.creation_backtraces.insert(id, bt); - id - }); - instance_id - }); - Self { name: self.name, instance_id: Cell::new(instance_id) } - } -} - -impl_clone_ref_as_clone!(TheGuardian); - -impl Drop for TheGuardian { - fn drop(&mut self) { - VALGRIND.with(|v| { - if let Some(instance_id) = self.instance_id.get() { - v.borrow_mut().creation_backtraces.remove(&instance_id); - } - }) - } -} - // === Definition === /// Network manages lifetime of set of FRP nodes. FRP networks are designed to be static. You can @@ -93,8 +30,7 @@ impl Drop for TheGuardian { /// Moreover, you should not grow the FRP network after it is constructed. #[derive(Clone, CloneRef, Debug)] pub struct Network { - data: Rc, - pub guardian: TheGuardian, + data: Rc, } /// Weak version of `Network`. @@ -150,8 +86,7 @@ impl Network { /// Non-generic constructor. fn new_with_string(label: String) -> Self { let data = Rc::new(NetworkData::new(label)); - let guardian = TheGuardian::new("Network"); - Self { data, guardian } + Self { data } } /// Get the weak version. @@ -231,8 +166,7 @@ impl Network { impl WeakNetwork { /// Upgrade to strong reference. pub fn upgrade(&self) -> Option { - let guardian = TheGuardian::new("UpgradedNetwork"); - self.data.upgrade().map(|data| Network { data, guardian }) + self.data.upgrade().map(|data| Network { data }) } /// Upgrade to string reference, printing warning if returning `None`. To be used in places diff --git a/lib/rust/prelude/src/debug.rs b/lib/rust/prelude/src/debug.rs index a8487fc0ba9a..491c31ed2f60 100644 --- a/lib/rust/prelude/src/debug.rs +++ b/lib/rust/prelude/src/debug.rs @@ -126,3 +126,72 @@ impl Drop for TraceCopies { } } } + + + +// ==================== +// === LeakDetector === +// ==================== + +/// A module containing an utility for detecting leaks. +pub mod leak_detector { + use crate::*; + + thread_local! { + /// The structure mapping the existing tracking copies with [`Trace`] structure to their + /// creation backtraces. You can print + pub static TRACKED_OBJECTS: RefCell> = default(); + } + + /// A utility for tracing all copies of CloneRef-able entity and keeping list of existing ones. + /// + /// This is a wrapper for [`TraceCopies`] which also register creation of each copy in + /// [`TRACKED_OBJECTS`] global variable. The variable may be then checked for leaks in moments + /// when we expect it to be empty. + #[derive(Debug)] + pub struct Trace { + instance: TraceCopies, + } + + impl Trace { + /// Create enabled structure with appointed entity name (shared between all copies). + pub fn enabled(name: impl Into) -> Self { + let instance = TraceCopies::enabled(name); + Self::register_tracked_object(&instance); + Self { instance } + } + + /// Assign a name to the entity (shared between all copies), start printing logs and + /// register its creation backtrace. + pub fn enable(&self, name: impl Into) { + self.instance.enable(name); + Self::register_tracked_object(&self.instance); + } + + fn register_tracked_object(instance: &TraceCopies) { + let id = instance.clone_id; + let bt = backtrace(); + TRACKED_OBJECTS.with(|objs| objs.borrow_mut().insert(id, bt)); + } + } + + impl Clone for Trace { + fn clone(&self) -> Self { + let instance = self.instance.clone(); + let enabled = instance.handle.borrow().is_some(); + if enabled { + Self::register_tracked_object(&instance); + } + Self { instance } + } + } + + impl_clone_ref_as_clone!(Trace); + + impl Drop for Trace { + fn drop(&mut self) { + let id = self.instance.clone_id; + TRACKED_OBJECTS.with(|objs| objs.borrow_mut().remove(&id)); + } + } +} From db4b6e7622a56b130f36f510ad47b794abf48c95 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Thu, 20 Apr 2023 12:34:26 +0200 Subject: [PATCH 4/9] Apply @vitvakatu review --- lib/rust/prelude/src/debug.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/rust/prelude/src/debug.rs b/lib/rust/prelude/src/debug.rs index 491c31ed2f60..e4e60ec0b105 100644 --- a/lib/rust/prelude/src/debug.rs +++ b/lib/rust/prelude/src/debug.rs @@ -139,7 +139,10 @@ pub mod leak_detector { thread_local! { /// The structure mapping the existing tracking copies with [`Trace`] structure to their - /// creation backtraces. You can print + /// creation backtraces. + /// + /// You may check/print it at various points where you expect no traced objects should + /// persist. pub static TRACKED_OBJECTS: RefCell> = default(); } From d732d28d9f57b454c226e78f01ea7cc9b531391a Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Mon, 24 Apr 2023 14:37:35 +0200 Subject: [PATCH 5/9] Extend the documentation a bit --- lib/rust/prelude/src/debug.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/rust/prelude/src/debug.rs b/lib/rust/prelude/src/debug.rs index e4e60ec0b105..847dbf06c321 100644 --- a/lib/rust/prelude/src/debug.rs +++ b/lib/rust/prelude/src/debug.rs @@ -134,6 +134,11 @@ impl Drop for TraceCopies { // ==================== /// A module containing an utility for detecting leaks. +/// +/// If you suspect a particular struct is leaking, i.e. its instances are still present when we +/// expect them to be removed, you may add a [`Trace`] field to it. Then, at the point where we +/// expect all instances to be dropped, we may check the [`TRACKED_OBJECTS`] global variable, what +/// instances are still alive and their creation backtraces. pub mod leak_detector { use crate::*; @@ -158,6 +163,8 @@ pub mod leak_detector { impl Trace { /// Create enabled structure with appointed entity name (shared between all copies). + /// + /// See [`TraceCopes::enabled`]. pub fn enabled(name: impl Into) -> Self { let instance = TraceCopies::enabled(name); Self::register_tracked_object(&instance); @@ -166,6 +173,8 @@ pub mod leak_detector { /// Assign a name to the entity (shared between all copies), start printing logs and /// register its creation backtrace. + /// + /// See [`TraceCopes::enable`]. pub fn enable(&self, name: impl Into) { self.instance.enable(name); Self::register_tracked_object(&self.instance); From 72a10cf85d5bd8eaafd8a36c5fd074d9f9a78076 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Mon, 24 Apr 2023 14:55:19 +0200 Subject: [PATCH 6/9] Extend docs even more and fix compilation after merge --- app/gui/view/graph-editor/src/lib.rs | 12 ++++++++++-- lib/rust/prelude/src/debug.rs | 8 ++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/app/gui/view/graph-editor/src/lib.rs b/app/gui/view/graph-editor/src/lib.rs index 5523b2a2757e..be5015aa7e8c 100644 --- a/app/gui/view/graph-editor/src/lib.rs +++ b/app/gui/view/graph-editor/src/lib.rs @@ -1701,6 +1701,14 @@ impl GraphEditorModelWithNetwork { visualization_shown.map2(&metadata,move |_,metadata| (node_id,metadata.clone())); + init <- source::<()>(); + enabled_visualization_path <- init.all_with3( + &node.visualization_enabled, &node.visualization_path, + move |_init, is_enabled, path| (node_id, is_enabled.and_option(path.clone())) + ); + output.enabled_visualization_path <+ enabled_visualization_path; + + // === View Mode === node.set_view_mode <+ self.model.frp.output.view_mode; @@ -1708,7 +1716,7 @@ impl GraphEditorModelWithNetwork { // === Read-only mode === - node.set_read_only <+ self.model.frp.set_read_only; + node.set_read_only <+ self.model.frp.input.set_read_only; // === Profiling === @@ -2786,7 +2794,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { // Drop the currently dragged edge if read-only mode is enabled. read_only_enabled <- inputs.set_read_only.on_true(); - inputs.drop_dragged_edge <+ read_only_enabled; + frp.drop_dragged_edge <+ read_only_enabled; } diff --git a/lib/rust/prelude/src/debug.rs b/lib/rust/prelude/src/debug.rs index 847dbf06c321..ecd99f06d688 100644 --- a/lib/rust/prelude/src/debug.rs +++ b/lib/rust/prelude/src/debug.rs @@ -153,7 +153,7 @@ pub mod leak_detector { /// A utility for tracing all copies of CloneRef-able entity and keeping list of existing ones. /// - /// This is a wrapper for [`TraceCopies`] which also register creation of each copy in + /// This is a wrapper for [`TraceCopies`] which also register each enabled copy in /// [`TRACKED_OBJECTS`] global variable. The variable may be then checked for leaks in moments /// when we expect it to be empty. #[derive(Debug)] @@ -164,7 +164,7 @@ pub mod leak_detector { impl Trace { /// Create enabled structure with appointed entity name (shared between all copies). /// - /// See [`TraceCopes::enabled`]. + /// See [`TraceCopies::enabled`] and [`Trace::enable`]. pub fn enabled(name: impl Into) -> Self { let instance = TraceCopies::enabled(name); Self::register_tracked_object(&instance); @@ -172,9 +172,9 @@ pub mod leak_detector { } /// Assign a name to the entity (shared between all copies), start printing logs and - /// register its creation backtrace. + /// register its creation backtrace in [`TRACKED_OBJECTS`]. /// - /// See [`TraceCopes::enable`]. + /// See [`TraceCopies::enable`]. pub fn enable(&self, name: impl Into) { self.instance.enable(name); Self::register_tracked_object(&self.instance); From 574de9e46955be74ca767a60e145bcda22cba0bb Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Mon, 24 Apr 2023 15:35:37 +0200 Subject: [PATCH 7/9] Extend one log --- app/gui/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/gui/src/lib.rs b/app/gui/src/lib.rs index 488098f65f31..b147b54ed264 100644 --- a/app/gui/src/lib.rs +++ b/app/gui/src/lib.rs @@ -207,7 +207,7 @@ pub fn drop() { let objects = objects.borrow(); if !objects.is_empty() { error!("Tracked objects leaked after dropping entire application!"); - error!("{objects:#?}"); + error!("Leaked objects: {objects:#?}"); } }) } From c65c100c6d76d75e7c7a91235e1d270c7d20ab48 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Tue, 25 Apr 2023 15:51:22 +0200 Subject: [PATCH 8/9] Remove DOM elements --- app/gui/src/lib.rs | 8 +++++++- app/gui/src/presenter.rs | 1 - lib/rust/ensogl/core/src/display/scene.rs | 19 +++++++++++++------ lib/rust/executor/src/web.rs | 14 +++++++++----- lib/rust/prelude/src/debug.rs | 8 ++++++-- 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/app/gui/src/lib.rs b/app/gui/src/lib.rs index b147b54ed264..491ba641382e 100644 --- a/app/gui/src/lib.rs +++ b/app/gui/src/lib.rs @@ -201,7 +201,13 @@ pub fn main() { /// All connections will be closed and all visuals will be removed. #[wasm_bindgen] pub fn drop() { - IDE.with(RefCell::take); + let ide = IDE.with(RefCell::take); + if let Some(Ok(ide)) = &ide { + //TODO[ao] #6420 We should not do this, but somehow the `dom` field in the scene is + // leaking. + ide.ensogl_app.display.default_scene.dom.root.remove(); + } + mem::drop(ide); EXECUTOR.with(RefCell::take); leak_detector::TRACKED_OBJECTS.with(|objects| { let objects = objects.borrow(); diff --git a/app/gui/src/presenter.rs b/app/gui/src/presenter.rs index 1a3123b2f8ba..8a3b2bac2867 100644 --- a/app/gui/src/presenter.rs +++ b/app/gui/src/presenter.rs @@ -167,7 +167,6 @@ impl Presenter { root_frp.switch_view_to_project <+ welcome_view_frp.open_project.constant(()); } - Self { model, network }.init() } diff --git a/lib/rust/ensogl/core/src/display/scene.rs b/lib/rust/ensogl/core/src/display/scene.rs index 3b2ff0eaf999..a1e9cb8282f3 100644 --- a/lib/rust/ensogl/core/src/display/scene.rs +++ b/lib/rust/ensogl/core/src/display/scene.rs @@ -326,8 +326,9 @@ impl Default for Keyboard { // === Dom === // =========== -/// DOM element manager -#[derive(Clone, CloneRef, Debug)] +/// DOM element manager. Creates root div element containing [`DomLayers`] upon construction and +/// removes them once dropped. +#[derive(Clone, Debug)] pub struct Dom { /// Root DOM element of the scene. pub root: web::dom::WithKnownShape, @@ -363,6 +364,12 @@ impl Default for Dom { } } +impl Drop for Dom { + fn drop(&mut self) { + self.root.remove(); + } +} + // ================= @@ -496,14 +503,14 @@ impl Dirty { #[derive(Clone, CloneRef, Debug)] #[allow(missing_docs)] pub struct Renderer { - dom: Dom, + dom: Rc, variables: UniformScope, pub pipeline: Rc>, pub composer: Rc>>, } impl Renderer { - fn new(dom: &Dom, variables: &UniformScope) -> Self { + fn new(dom: &Rc, variables: &UniformScope) -> Self { let dom = dom.clone_ref(); let variables = variables.clone_ref(); let pipeline = default(); @@ -776,7 +783,7 @@ pub struct UpdateStatus { #[derive(Clone, CloneRef, Debug)] pub struct SceneData { pub display_object: display::object::Root, - pub dom: Dom, + pub dom: Rc, pub context: Rc>>, pub context_lost_handler: Rc>>, pub variables: UniformScope, @@ -810,7 +817,7 @@ impl SceneData { ) -> Self { debug!("Initializing."); let display_mode = display_mode.clone_ref(); - let dom = Dom::new(); + let dom = default(); let display_object = display::object::Root::new_named("Scene"); let variables = world::with_context(|t| t.variables.clone_ref()); let dirty = Dirty::new(on_mut); diff --git a/lib/rust/executor/src/web.rs b/lib/rust/executor/src/web.rs index a8b9d3208564..79eb282830d8 100644 --- a/lib/rust/executor/src/web.rs +++ b/lib/rust/executor/src/web.rs @@ -62,14 +62,18 @@ impl EventLoopExecutor { /// attempt achieving as much progress on this executor's tasks as possible /// without stalling. pub fn runner(&self) -> impl FnMut(animation::TimeInfo) { - let executor = self.executor.clone(); + // We pass weak handle to the runner to ensure all scheduled futures will be dropped + // once executor is dropped. + let executor = Rc::downgrade(&self.executor); move |_| { let _profiler = profiler::start_debug!(profiler::APP_LIFETIME, "EventLoopExecutor::runner"); - // Safe, because this is the only place borrowing executor and loop - // callback shall never be re-entrant. - let mut executor = executor.borrow_mut(); - executor.run_until_stalled(); + if let Some(executor) = executor.upgrade() { + // Safe, because this is the only place borrowing executor and loop + // callback shall never be re-entrant. + let mut executor = executor.borrow_mut(); + executor.run_until_stalled(); + } } } diff --git a/lib/rust/prelude/src/debug.rs b/lib/rust/prelude/src/debug.rs index ecd99f06d688..ea3d268b1f00 100644 --- a/lib/rust/prelude/src/debug.rs +++ b/lib/rust/prelude/src/debug.rs @@ -56,7 +56,7 @@ pub use internal::backtrace; /// mark each copy with unique id (the original copy has id of 0). Once enabled, it will print /// backtrace of each clone, clone_ref or drop operation with assigned name (the same for all /// copies) and copy id. -#[derive(Debug, Default)] +#[derive(Debug)] pub struct TraceCopies { clone_id: u64, handle: Rc>>, @@ -75,6 +75,10 @@ fn next_clone_id() -> u64 { } impl TraceCopies { + pub fn new() -> Self { + Self { clone_id: next_clone_id(), handle: default() } + } + /// Create enabled structure with appointed entity name (shared between all copies). pub fn enabled(name: impl Into) -> Self { let this: Self = default(); @@ -156,7 +160,7 @@ pub mod leak_detector { /// This is a wrapper for [`TraceCopies`] which also register each enabled copy in /// [`TRACKED_OBJECTS`] global variable. The variable may be then checked for leaks in moments /// when we expect it to be empty. - #[derive(Debug)] + #[derive(Debug, Default)] pub struct Trace { instance: TraceCopies, } From 85423ba884f1115015c66cd257096ce09fb8d74f Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Tue, 25 Apr 2023 15:58:33 +0200 Subject: [PATCH 9/9] Fix compilation --- lib/rust/prelude/src/debug.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/rust/prelude/src/debug.rs b/lib/rust/prelude/src/debug.rs index ea3d268b1f00..3e5af343bd01 100644 --- a/lib/rust/prelude/src/debug.rs +++ b/lib/rust/prelude/src/debug.rs @@ -94,6 +94,12 @@ impl TraceCopies { } } +impl Default for TraceCopies { + fn default() -> Self { + Self::new() + } +} + impl Clone for TraceCopies { fn clone(&self) -> Self { let borrow = self.handle.borrow();