zed: Add `track-project-leak` feature for easier leak debugging (#52172)

Lukas Wirth created

Release Notes:

- N/A or Added/Fixed/Improved ...

Change summary

crates/gpui/examples/on_window_close_quit.rs |  2 
crates/gpui/examples/painting.rs             |  2 
crates/gpui/src/app.rs                       | 14 ++-
crates/gpui/src/app/entity_map.rs            | 76 +++++++++++++++++++++
crates/gpui_windows/src/platform.rs          | 10 ++
crates/settings_ui/src/settings_ui.rs        |  2 
crates/zed/Cargo.toml                        |  2 
crates/zed/src/reliability.rs                |  6 +
crates/zed/src/zed.rs                        | 31 ++++++++
9 files changed, 130 insertions(+), 15 deletions(-)

Detailed changes

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();
             }

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();

crates/gpui/src/app.rs 🔗

@@ -241,7 +241,7 @@ type Listener = Box<dyn FnMut(&dyn Any, &mut App) -> bool + 'static>;
 pub(crate) type KeystrokeObserver =
     Box<dyn FnMut(&KeystrokeEvent, &mut Window, &mut App) -> bool + 'static>;
 type QuitHandler = Box<dyn FnOnce(&mut App) -> LocalBoxFuture<'static, ()> + 'static>;
-type WindowClosedHandler = Box<dyn FnMut(&mut App)>;
+type WindowClosedHandler = Box<dyn FnMut(&mut App, WindowId)>;
 type ReleaseListener = Box<dyn FnOnce(&mut dyn Any, &mut App) + 'static>;
 type NewEntityListener = Box<dyn FnMut(AnyEntity, &mut Option<&mut Window>, &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::<T>(),
                 window: cx.window_update_stack.last().cloned(),
             });
 
-            cx.entities.insert(slot, entity);
-            handle
+            cx.entities.insert(slot, entity)
         })
     }
 

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<core::pin::Pin<alloc::boxed::Box<dyn$<core::future::future::Future<assoc$<Output,enum2$<core::result::Result<workspace::OpenResult,anyhow::Error> > > > >,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;

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) }
     };

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()

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",

crates/zed/src/reliability.rs 🔗

@@ -22,7 +22,11 @@ use crate::STARTUP_TIME;
 const MAX_HANG_TRACES: usize = 3;
 
 pub fn init(client: Arc<Client>, 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();

crates/zed/src/zed.rs 🔗

@@ -291,7 +291,7 @@ fn bind_on_window_closed(cx: &mut App) -> Option<gpui::Subscription> {
             .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<gpui::Subscription> {
     }
     #[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