diff --git a/crates/gpui/examples/on_window_close_quit.rs b/crates/gpui/examples/on_window_close_quit.rs index e71a142d991c87ccbccb9c078fdb50d1fa3dba49..347401c6d924f146fec539c862878d21c4b18e67 100644 --- a/crates/gpui/examples/on_window_close_quit.rs +++ b/crates/gpui/examples/on_window_close_quit.rs @@ -42,7 +42,7 @@ fn run_example() { let mut bounds = Bounds::centered(None, size(px(500.), px(500.0)), cx); cx.bind_keys([KeyBinding::new("cmd-w", CloseWindow, None)]); - cx.on_window_closed(|cx| { + cx.on_window_closed(|cx, _window_id| { if cx.windows().is_empty() { cx.quit(); } diff --git a/crates/gpui/examples/painting.rs b/crates/gpui/examples/painting.rs index 18ef6b9fa3741297ddfebc1b5df3ea4a3594fc05..11c3b333717c6b816cdf2f7d5170ceae0cfd1b1f 100644 --- a/crates/gpui/examples/painting.rs +++ b/crates/gpui/examples/painting.rs @@ -457,7 +457,7 @@ fn run_example() { |window, cx| cx.new(|cx| PaintingViewer::new(window, cx)), ) .unwrap(); - cx.on_window_closed(|cx| { + cx.on_window_closed(|cx, _window_id| { cx.quit(); }) .detach(); diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 5536410b087b8359aab2bcc14c590ba1afd46798..3453364a20ebf59bef6940656f79cbfdaf732c22 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -241,7 +241,7 @@ type Listener = Box bool + 'static>; pub(crate) type KeystrokeObserver = Box bool + 'static>; type QuitHandler = Box LocalBoxFuture<'static, ()> + 'static>; -type WindowClosedHandler = Box; +type WindowClosedHandler = Box; type ReleaseListener = Box; type NewEntityListener = Box, &mut App) + 'static>; @@ -1567,7 +1567,7 @@ impl App { cx.windows.remove(id); cx.window_closed_observers.clone().retain(&(), |callback| { - callback(cx); + callback(cx, id); true }); @@ -2041,7 +2041,10 @@ impl App { /// Register a callback to be invoked when a window is closed /// The window is no longer accessible at the point this callback is invoked. - pub fn on_window_closed(&self, mut on_closed: impl FnMut(&mut App) + 'static) -> Subscription { + pub fn on_window_closed( + &self, + mut on_closed: impl FnMut(&mut App, WindowId) + 'static, + ) -> Subscription { let (subscription, activate) = self.window_closed_observers.insert((), Box::new(on_closed)); activate(); subscription @@ -2357,13 +2360,12 @@ impl AppContext for App { let entity = build_entity(&mut Context::new_context(cx, slot.downgrade())); cx.push_effect(Effect::EntityCreated { - entity: handle.clone().into_any(), + entity: handle.into_any(), tid: TypeId::of::(), window: cx.window_update_stack.last().cloned(), }); - cx.entities.insert(slot, entity); - handle + cx.entities.insert(slot, entity) }) } diff --git a/crates/gpui/src/app/entity_map.rs b/crates/gpui/src/app/entity_map.rs index 766610aac9e8de619694662da9d4c62472a62b1d..cc4eaee492618812f1ee361d549b5e0052dafc68 100644 --- a/crates/gpui/src/app/entity_map.rs +++ b/crates/gpui/src/app/entity_map.rs @@ -894,6 +894,9 @@ pub(crate) struct HandleId { /// created, all participating strong entities in this cycle will effectively /// leak as they cannot be released anymore. /// +/// Cycles can also happen if an entity owns a task or subscription that it +/// itself owns a strong reference to the entity again. +/// /// # Usage /// /// You can use `WeakEntity::assert_released` or `AnyWeakEntity::assert_released` @@ -919,7 +922,7 @@ pub(crate) struct HandleId { /// ``` /// /// This will capture and display backtraces for each leaked handle, helping you -/// identify where handles were created but not released. +/// identify where leaked handles were created. /// /// # How It Works /// @@ -1002,11 +1005,13 @@ impl LeakDetector { /// otherwise it suggests setting the environment variable to get more info. pub fn assert_released(&mut self, entity_id: EntityId) { use std::fmt::Write as _; + if let Some(data) = self.entity_handles.remove(&entity_id) { let mut out = String::new(); for (_, backtrace) in data.handles { if let Some(mut backtrace) = backtrace { backtrace.resolve(); + let backtrace = BacktraceFormatter(backtrace); writeln!(out, "Leaked handle:\n{:?}", backtrace).unwrap(); } else { writeln!( @@ -1016,7 +1021,7 @@ impl LeakDetector { .unwrap(); } } - panic!("{out}"); + panic!("Handles for {} leaked:\n{out}", data.type_name); } } @@ -1054,6 +1059,7 @@ impl LeakDetector { if let Some(backtrace) = backtrace { let mut backtrace = backtrace.clone(); backtrace.resolve(); + let backtrace = BacktraceFormatter(backtrace); writeln!( out, "Leaked handle for entity {} ({entity_id:?}):\n{:?}", @@ -1091,6 +1097,7 @@ impl Drop for LeakDetector { for (_handle, backtrace) in data.handles { if let Some(mut backtrace) = backtrace { backtrace.resolve(); + let backtrace = BacktraceFormatter(backtrace); writeln!( out, "Leaked handle for entity {} ({entity_id:?}):\n{:?}", @@ -1111,6 +1118,71 @@ impl Drop for LeakDetector { } } +#[cfg(any(test, feature = "leak-detection"))] +struct BacktraceFormatter(backtrace::Backtrace); + +#[cfg(any(test, feature = "leak-detection"))] +impl fmt::Debug for BacktraceFormatter { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + use backtrace::{BacktraceFmt, BytesOrWideString, PrintFmt}; + + let style = if fmt.alternate() { + PrintFmt::Full + } else { + PrintFmt::Short + }; + + // When printing paths we try to strip the cwd if it exists, otherwise + // we just print the path as-is. Note that we also only do this for the + // short format, because if it's full we presumably want to print + // everything. + let cwd = std::env::current_dir(); + let mut print_path = move |fmt: &mut fmt::Formatter<'_>, path: BytesOrWideString<'_>| { + let path = path.into_path_buf(); + if style != PrintFmt::Full { + if let Ok(cwd) = &cwd { + if let Ok(suffix) = path.strip_prefix(cwd) { + return fmt::Display::fmt(&suffix.display(), fmt); + } + } + } + fmt::Display::fmt(&path.display(), fmt) + }; + + let mut f = BacktraceFmt::new(fmt, style, &mut print_path); + f.add_context()?; + let mut strip = true; + for frame in self.0.frames() { + if let [symbol, ..] = frame.symbols() + && let Some(name) = symbol.name() + && let Some(filename) = name.as_str() + { + match filename { + "test::run_test_in_process" + | "scheduler::executor::spawn_local_with_source_location::impl$1::poll > > > >,alloc::alloc::Global> > >" => { + strip = true + } + "gpui::app::entity_map::LeakDetector::handle_created" => { + strip = false; + continue; + } + "zed::main" => { + strip = true; + f.frame().backtrace_frame(frame)?; + } + _ => {} + } + } + if strip { + continue; + } + f.frame().backtrace_frame(frame)?; + } + f.finish()?; + Ok(()) + } +} + #[cfg(test)] mod test { use crate::EntityMap; diff --git a/crates/gpui_windows/src/platform.rs b/crates/gpui_windows/src/platform.rs index 182107138579be858272329a75a9daededd438e4..7e9f1e77487b4185fbad9e1dc66cfcb1c8191e61 100644 --- a/crates/gpui_windows/src/platform.rs +++ b/crates/gpui_windows/src/platform.rs @@ -1326,7 +1326,15 @@ unsafe extern "system" fn window_procedure( } let inner = unsafe { &*ptr }; let result = if let Some(inner) = inner.upgrade() { - inner.handle_msg(hwnd, msg, wparam, lparam) + if cfg!(debug_assertions) { + let inner = std::panic::AssertUnwindSafe(inner); + match std::panic::catch_unwind(|| { inner }.handle_msg(hwnd, msg, wparam, lparam)) { + Ok(result) => result, + Err(_) => std::process::abort(), + } + } else { + inner.handle_msg(hwnd, msg, wparam, lparam) + } } else { unsafe { DefWindowProcW(hwnd, msg, wparam, lparam) } }; diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index f2107fb63a7dbd86b3a01c4d222adbd46bd4a083..d130056edfd288394b45559425207545c10d262e 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -1516,7 +1516,7 @@ impl SettingsWindow { }) .detach(); - cx.on_window_closed(|cx| { + cx.on_window_closed(|cx, _window_id| { if let Some(existing_window) = cx .windows() .into_iter() diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index fef75ea29f762df60f6d60892b78e28ac7aad503..a509bf67920d20ad7004ac2a14f3ee8dff5aa4e2 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -13,6 +13,8 @@ workspace = true [features] tracy = ["ztracing/tracy"] +# LEAK_BACKTRACE=1 cargo run --features zed/track-project-leak --profile release-fast +track-project-leak = ["gpui/leak-detection"] test-support = [ "gpui/test-support", "gpui_platform/screen-capture", diff --git a/crates/zed/src/reliability.rs b/crates/zed/src/reliability.rs index 2f284027929b19e5b0d5ac084267cf5548cda667..e6c3821507cffb0d6e46f9634a646a009b73ddc3 100644 --- a/crates/zed/src/reliability.rs +++ b/crates/zed/src/reliability.rs @@ -22,7 +22,11 @@ use crate::STARTUP_TIME; const MAX_HANG_TRACES: usize = 3; pub fn init(client: Arc, cx: &mut App) { - monitor_hangs(cx); + if cfg!(debug_assertions) { + log::info!("Debug assertions enabled, skipping hang monitoring"); + } else { + monitor_hangs(cx); + } cx.on_flags_ready({ let client = client.clone(); diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 66f0239de655492176861e289cafb5fa5a3475de..1a51d08540a95381e4494ae724806967dd8ed1ec 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -291,7 +291,7 @@ fn bind_on_window_closed(cx: &mut App) -> Option { .on_last_window_closed .is_quit_app() .then(|| { - cx.on_window_closed(|cx| { + cx.on_window_closed(|cx, _window_id| { if cx.windows().is_empty() { cx.quit(); } @@ -300,7 +300,7 @@ fn bind_on_window_closed(cx: &mut App) -> Option { } #[cfg(not(target_os = "macos"))] { - Some(cx.on_window_closed(|cx| { + Some(cx.on_window_closed(|cx, _window_id| { if cx.windows().is_empty() { cx.quit(); } @@ -372,6 +372,33 @@ pub fn initialize_workspace( return; }; + #[cfg(feature = "track-project-leak")] + { + let multi_workspace_handle = cx.weak_entity(); + let workspace_handle = _multi_workspace.workspace().downgrade(); + let project_handle = _multi_workspace.workspace().read(cx).project().downgrade(); + let window_id_2 = window.window_handle().window_id(); + cx.on_window_closed(move |cx, window_id| { + let multi_workspace_handle = multi_workspace_handle.clone(); + let workspace_handle = workspace_handle.clone(); + let project_handle = project_handle.clone(); + if window_id != window_id_2 { + return; + } + cx.spawn(async move |cx| { + cx.background_executor() + .timer(std::time::Duration::from_millis(1500)) + .await; + + multi_workspace_handle.assert_released(); + workspace_handle.assert_released(); + project_handle.assert_released(); + }) + .detach(); + }) + .detach(); + } + let multi_workspace_handle = cx.entity().downgrade(); window.on_window_should_close(cx, move |window, cx| { multi_workspace_handle