From af5d1964f17860ce9a4a0b89aad66dc16909b6bd Mon Sep 17 00:00:00 2001 From: Anthony Eid Date: Fri, 24 Jul 2026 17:26:54 +0300 Subject: [PATCH] gpui: Fix deadlock in performance profiler and reenable it --- crates/gpui/src/profiler.rs | 56 +++++++++++++++++++-------------- crates/remote_server/Cargo.toml | 2 +- crates/zed/Cargo.toml | 2 +- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/crates/gpui/src/profiler.rs b/crates/gpui/src/profiler.rs index c9e9e8c58784da..e66a0025cf2013 100644 --- a/crates/gpui/src/profiler.rs +++ b/crates/gpui/src/profiler.rs @@ -23,8 +23,7 @@ use crate::{SharedString, TasksIncluded, WindowId}; #[cfg(feature = "profiler")] #[doc(hidden)] pub fn get_all_timings(included: gpui::TasksIncluded) -> Vec { - let global_thread_timings = GLOBAL_THREAD_TIMINGS.lock(); - ThreadTaskTimings::collect(&global_thread_timings, included) + ThreadTaskTimings::collect(upgraded_thread_timings(), included) } #[cfg(feature = "profiler")] @@ -36,8 +35,7 @@ pub fn get_current_thread_timings(included: TasksIncluded) -> gpui::ThreadTaskTi #[cfg(feature = "profiler")] #[doc(hidden)] pub fn take_all_stats(included: TasksIncluded) -> Vec { - let global_timings = GLOBAL_THREAD_TIMINGS.lock(); - ThreadTaskStatistics::collect_and_reset(&global_timings, included) + ThreadTaskStatistics::collect_and_reset(upgraded_thread_timings(), included) } #[cfg(not(feature = "profiler"))] @@ -128,14 +126,13 @@ pub struct ThreadTaskTimings { } impl ThreadTaskTimings { - /// Convert global thread timings into their structured format. - pub fn collect(timings: &[GlobalThreadTimings], included: TasksIncluded) -> Vec { + /// Convert upgraded per-thread timings into their structured format. + pub fn collect( + timings: Vec<(ThreadId, Arc)>, + included: TasksIncluded, + ) -> Vec { timings - .iter() - .filter_map(|t| match t.timings.upgrade() { - Some(timings) => Some((t.thread_id, timings)), - _ => None, - }) + .into_iter() .map(|(thread_id, timings)| { let timings = timings.lock(); let thread_name = timings.thread_name.clone(); @@ -179,15 +176,11 @@ pub struct ThreadTaskStatistics { impl ThreadTaskStatistics { pub fn collect_and_reset( - timings: &[GlobalThreadTimings], + timings: Vec<(ThreadId, Arc)>, include_running: TasksIncluded, ) -> Vec { timings - .iter() - .filter_map(|t| match t.timings.upgrade() { - Some(timings) => Some((t.thread_id, timings)), - _ => None, - }) + .into_iter() .map(|(thread_id, timings)| { let mut timings = timings.lock(); let thread_name = timings.thread_name.clone(); @@ -511,6 +504,23 @@ impl TaskStatistics { pub static GLOBAL_THREAD_TIMINGS: spin::Mutex> = spin::Mutex::new(Vec::new()); +/// Upgrades all live per-thread timing handles, holding the global registry +/// lock only for the duration of the upgrades. +/// +/// The upgraded `Arc`s must never be dropped while `GLOBAL_THREAD_TIMINGS` is +/// locked: dropping the last strong reference runs [`ThreadTimings::drop`], +/// which locks `GLOBAL_THREAD_TIMINGS` again and would deadlock the +/// non-reentrant spinlock. A thread exiting concurrently can hand off its last +/// reference to us at any time, so callers of this function process (lock, +/// read, drop) the returned handles only after the global lock is released. +fn upgraded_thread_timings() -> Vec<(ThreadId, Arc)> { + let global_thread_timings = GLOBAL_THREAD_TIMINGS.lock(); + global_thread_timings + .iter() + .filter_map(|t| Some((t.thread_id, t.timings.upgrade()?))) + .collect() +} + thread_local! { #[doc(hidden)] pub static THREAD_TIMINGS: LazyCell> = LazyCell::new(|| { @@ -679,13 +689,11 @@ pub fn set_trace_enabled(enabled: bool) -> bool { } if !enabled { - for global in GLOBAL_THREAD_TIMINGS.lock().iter() { - if let Some(timings) = global.timings.upgrade() { - let mut timings = timings.lock(); - timings.timings.clear(); - timings.timings.shrink_to_fit(); - timings.total_pushed = 0; - } + for (_, timings) in upgraded_thread_timings() { + let mut timings = timings.lock(); + timings.timings.clear(); + timings.timings.shrink_to_fit(); + timings.total_pushed = 0; } } true diff --git a/crates/remote_server/Cargo.toml b/crates/remote_server/Cargo.toml index 89b0dfef67dc4c..6f22970504b804 100644 --- a/crates/remote_server/Cargo.toml +++ b/crates/remote_server/Cargo.toml @@ -39,7 +39,7 @@ fs.workspace = true futures.workspace = true git.workspace = true git_hosting_providers.workspace = true -gpui.workspace = true +gpui = { workspace = true, features = ["profiler"] } gpui_platform.workspace = true gpui_tokio.workspace = true http_client.workspace = true diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index d050b61d29f78d..68ced31c2b5ff4 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -117,7 +117,7 @@ git_hosting_providers.workspace = true git_ui = { workspace = true, features = ["call"] } go_to_line.workspace = true system_specs.workspace = true -gpui = { workspace = true, features = ["input-latency-histogram"] } +gpui = { workspace = true, features = ["input-latency-histogram", "profiler"] } gpui_platform = {workspace = true, features=["screen-capture", "font-kit", "wayland", "x11"]} hdrhistogram.workspace = true image = { workspace = true, optional = true }