From ae9139ba5af5c60d406ebce131d8b5c395d8fbc0 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Sun, 28 Dec 2025 19:40:13 -0800 Subject: [PATCH] Remove new types --- crates/gpui/src/app.rs | 27 ++++--- crates/gpui/src/app/async_context.rs | 9 ++- crates/gpui/src/app/test_context.rs | 2 +- crates/gpui/src/executor.rs | 50 ++++++------- crates/gpui/src/platform.rs | 79 +++++---------------- crates/gpui/src/platform/mac/dispatcher.rs | 8 +-- crates/gpui/src/platform/test/dispatcher.rs | 10 ++- 7 files changed, 64 insertions(+), 121 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index a8aa974ae53f2edca35b94ffe24f3b08852facbb..c99ee073b85d86c5971f3274a4d149895c821b4e 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -34,15 +34,15 @@ use util::{ResultExt, debug_panic}; #[cfg(any(feature = "inspector", debug_assertions))] use crate::InspectorElementRegistry; use crate::{ - Action, ActionBuildError, ActionRegistry, Any, AnyView, AnyWindowHandle, AppContext, - AppLiveness, Asset, AssetSource, BackgroundExecutor, Bounds, ClipboardItem, CursorStyle, - DispatchPhase, DisplayId, EventEmitter, FocusHandle, FocusMap, ForegroundExecutor, Global, - KeyBinding, KeyContext, Keymap, Keystroke, LayoutId, Menu, MenuItem, OwnedMenu, - PathPromptOptions, Pixels, Platform, PlatformDisplay, PlatformKeyboardLayout, - PlatformKeyboardMapper, Point, Priority, PromptBuilder, PromptButton, PromptHandle, - PromptLevel, Render, RenderImage, RenderablePromptHandle, Reservation, ScreenCaptureSource, - SharedString, SubscriberSet, Subscription, SvgRenderer, Task, TextSystem, Window, - WindowAppearance, WindowHandle, WindowId, WindowInvalidator, + Action, ActionBuildError, ActionRegistry, Any, AnyView, AnyWindowHandle, AppContext, Asset, + AssetSource, BackgroundExecutor, Bounds, ClipboardItem, CursorStyle, DispatchPhase, DisplayId, + EventEmitter, FocusHandle, FocusMap, ForegroundExecutor, Global, KeyBinding, KeyContext, + Keymap, Keystroke, LayoutId, Menu, MenuItem, OwnedMenu, PathPromptOptions, Pixels, Platform, + PlatformDisplay, PlatformKeyboardLayout, PlatformKeyboardMapper, Point, Priority, + PromptBuilder, PromptButton, PromptHandle, PromptLevel, Render, RenderImage, + RenderablePromptHandle, Reservation, ScreenCaptureSource, SharedString, SubscriberSet, + Subscription, SvgRenderer, Task, TextSystem, Window, WindowAppearance, WindowHandle, WindowId, + WindowInvalidator, colors::{Colors, GlobalColors}, current_platform, hash, init_app_menus, }; @@ -580,9 +580,7 @@ impl GpuiMode { /// You need a reference to an `App` to access the state of a [Entity]. pub struct App { pub(crate) this: Weak, - /// Tracks whether this app is still alive. Used to cancel foreground tasks - /// when the app is dropped. - pub(crate) liveness: AppLiveness, + pub(crate) liveness: std::sync::Arc<()>, pub(crate) platform: Rc, pub(crate) mode: GpuiMode, text_system: Arc, @@ -661,7 +659,7 @@ impl App { let app = Rc::new_cyclic(|this| AppCell { app: RefCell::new(App { this: this.clone(), - liveness: AppLiveness::new(), + liveness: std::sync::Arc::new(()), platform: platform.clone(), text_system, mode: GpuiMode::Production, @@ -1478,10 +1476,9 @@ impl App { /// Creates an `AsyncApp`, which can be cloned and has a static lifetime /// so it can be held across `await` points. pub fn to_async(&self) -> AsyncApp { - let liveness_token = self.liveness.token(); AsyncApp { app: self.this.clone(), - liveness_token, + liveness_token: std::sync::Arc::downgrade(&self.liveness), background_executor: self.background_executor.clone(), foreground_executor: self.foreground_executor.clone(), } diff --git a/crates/gpui/src/app/async_context.rs b/crates/gpui/src/app/async_context.rs index c07ada62fc694551b30459a04042b4e5208cea06..3f9d21ef65ab16350c8d16fa3197ac507e5d503e 100644 --- a/crates/gpui/src/app/async_context.rs +++ b/crates/gpui/src/app/async_context.rs @@ -1,8 +1,7 @@ use crate::{ - AnyView, AnyWindowHandle, App, AppCell, AppContext, AppLivenessToken, BackgroundExecutor, - BorrowAppContext, Entity, EventEmitter, Focusable, ForegroundExecutor, Global, PromptButton, - PromptLevel, Render, Reservation, Result, Subscription, Task, VisualContext, Window, - WindowHandle, + AnyView, AnyWindowHandle, App, AppCell, AppContext, BackgroundExecutor, BorrowAppContext, + Entity, EventEmitter, Focusable, ForegroundExecutor, Global, PromptButton, PromptLevel, Render, + Reservation, Result, Subscription, Task, VisualContext, Window, WindowHandle, }; use anyhow::{Context as _, anyhow}; use derive_more::{Deref, DerefMut}; @@ -17,7 +16,7 @@ use super::{Context, WeakEntity}; #[derive(Clone)] pub struct AsyncApp { pub(crate) app: Weak, - pub(crate) liveness_token: AppLivenessToken, + pub(crate) liveness_token: std::sync::Weak<()>, pub(crate) background_executor: BackgroundExecutor, pub(crate) foreground_executor: ForegroundExecutor, } diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index 71119d2373fb59a08ad4e2f4fd579b16839ea839..d7a1e4704584595d769989b421ccd767fba58985 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -405,7 +405,7 @@ impl TestAppContext { pub fn to_async(&self) -> AsyncApp { AsyncApp { app: Rc::downgrade(&self.app), - liveness_token: self.app.borrow().liveness.token(), + liveness_token: std::sync::Arc::downgrade(&self.app.borrow().liveness), background_executor: self.background_executor.clone(), foreground_executor: self.foreground_executor.clone(), } diff --git a/crates/gpui/src/executor.rs b/crates/gpui/src/executor.rs index 5d6c8641e03a1bdc217e8024fbd1daa3098f3428..c57cafde87f26b21ec8d432a623dba442d277008 100644 --- a/crates/gpui/src/executor.rs +++ b/crates/gpui/src/executor.rs @@ -1,6 +1,4 @@ -use crate::{ - App, AppLivenessToken, PlatformDispatcher, RunnableMeta, RunnableVariant, TaskTiming, profiler, -}; +use crate::{App, PlatformDispatcher, RunnableMeta, RunnableVariant, TaskTiming, profiler}; use async_task::Runnable; use futures::channel::mpsc; use parking_lot::{Condvar, Mutex}; @@ -787,7 +785,7 @@ impl ForegroundExecutor { #[track_caller] pub(crate) fn spawn_context( &self, - app: AppLivenessToken, + app: std::sync::Weak<()>, future: impl Future + 'static, ) -> Task where @@ -799,7 +797,7 @@ impl ForegroundExecutor { #[track_caller] pub(crate) fn inner_spawn( &self, - app: Option, + app: Option>, priority: Priority, future: impl Future + 'static, ) -> Task @@ -814,7 +812,7 @@ impl ForegroundExecutor { dispatcher: Arc, future: AnyLocalFuture, location: &'static core::panic::Location<'static>, - app: Option, + app: Option>, priority: Priority, ) -> Task { let (runnable, task) = spawn_local_with_source_location( @@ -980,7 +978,7 @@ mod test { let http_client = http_client::FakeHttpClient::with_404_response(); let app = App::new_app(platform, asset_source, http_client); - let liveness_token = app.borrow().liveness.token(); + let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness); let task_ran = Rc::new(RefCell::new(false)); @@ -1015,7 +1013,7 @@ mod test { let http_client = http_client::FakeHttpClient::with_404_response(); let app = App::new_app(platform, asset_source, http_client); - let liveness_token = app.borrow().liveness.token(); + let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness); let app_weak = Rc::downgrade(&app); let task_ran = Rc::new(RefCell::new(false)); @@ -1052,7 +1050,7 @@ mod test { let http_client = http_client::FakeHttpClient::with_404_response(); let app = App::new_app(platform, asset_source, http_client); - let liveness_token = app.borrow().liveness.token(); + let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness); let app_weak = Rc::downgrade(&app); let outer_completed = Rc::new(RefCell::new(false)); @@ -1070,25 +1068,20 @@ mod test { let inner_executor = foreground_executor.clone(); let inner_liveness_token = liveness_token.clone(); - // Spawn outer task that will spawn and await an inner task foreground_executor .spawn_context(liveness_token, async move { - let inner_flag_clone = Rc::clone(&inner_flag); - - // Spawn inner task that blocks on a channel - let inner_task = inner_executor.spawn_context(inner_liveness_token, async move { - // Wait for signal (which will never come - we'll drop the app instead) - rx.await.ok(); - *inner_flag_clone.borrow_mut() = true; + let inner_task = inner_executor.spawn_context(inner_liveness_token, { + let inner_flag = Rc::clone(&inner_flag); + async move { + rx.await.ok(); + *inner_flag.borrow_mut() = true; + } }); - // Mark that we've reached the await point *await_flag.borrow_mut() = true; - // Await inner task - this should not panic when both are cancelled inner_task.await; - // Mark outer as complete (should never reach here) *outer_flag.borrow_mut() = true; }) .detach(); @@ -1178,7 +1171,7 @@ mod test { let http_client = http_client::FakeHttpClient::with_404_response(); let app = App::new_app(platform, asset_source, http_client); - let liveness_token = app.borrow().liveness.token(); + let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness); let app_weak = Rc::downgrade(&app); let task = foreground_executor.spawn_context(liveness_token, async move { 42 }); @@ -1204,7 +1197,7 @@ mod test { let http_client = http_client::FakeHttpClient::with_404_response(); let app = App::new_app(platform, asset_source, http_client); - let liveness_token = app.borrow().liveness.token(); + let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness); let app_weak = Rc::downgrade(&app); let task = foreground_executor @@ -1223,23 +1216,22 @@ mod test { #[test] fn test_app_liveness_token_can_be_dropped_on_background_thread() { - let dispatcher = TestDispatcher::new(StdRng::seed_from_u64(0)); - let arc_dispatcher = Arc::new(dispatcher.clone()); - let background_executor = BackgroundExecutor::new(arc_dispatcher.clone()); - let foreground_executor = ForegroundExecutor::new(arc_dispatcher); + let dispatcher = Arc::new(TestDispatcher::new(StdRng::seed_from_u64(0))); + let background_executor = BackgroundExecutor::new(dispatcher.clone()); + let foreground_executor = ForegroundExecutor::new(dispatcher); let platform = TestPlatform::new(background_executor, foreground_executor); let asset_source = Arc::new(()); let http_client = http_client::FakeHttpClient::with_404_response(); let app = App::new_app(platform, asset_source, http_client); - let liveness_token = app.borrow().liveness.token(); + let liveness_token = std::sync::Arc::downgrade(&app.borrow().liveness); - // Drop the token on a real background thread. + // Dispatcher is single threaded when testing, so we need to spawn a real thread std::thread::spawn(move || { drop(liveness_token); }) .join() - .expect("Dropping AppLivenessToken on background thread should not panic"); + .expect("Dropping Weak<()> on background thread should not panic"); } } diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 315ab7a80e25fa16357dce02c7914a98222b688f..e2a06eb77d07a0baeeb9e66ebe105b34b6073a88 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -572,74 +572,33 @@ pub(crate) trait PlatformWindow: HasWindowHandle + HasDisplayHandle { } } -/// Tracks whether an App is still alive. This is used to cancel foreground tasks -/// when the app is dropped. -/// -#[derive(Clone)] +/// This type is public so that our test macro can generate and use it, but it should not +/// be considered part of our public API. #[doc(hidden)] -pub struct AppLiveness { - sentinel: std::sync::Arc<()>, -} - -impl AppLiveness { - /// Creates a new AppLiveness, initially alive. - pub fn new() -> Self { - Self { - sentinel: std::sync::Arc::new(()), - } - } - - /// Returns a token that can be stored in task metadata to check liveness. - pub fn token(&self) -> AppLivenessToken { - AppLivenessToken { - sentinel: std::sync::Arc::downgrade(&self.sentinel), - } - } -} - -impl Default for AppLiveness { - fn default() -> Self { - Self::new() - } -} - -impl std::fmt::Debug for AppLiveness { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("AppLiveness").field("alive", &true).finish() - } -} - -/// A token that can be stored in task metadata to check if the app is still alive. -#[derive(Clone)] -pub struct AppLivenessToken { - sentinel: std::sync::Weak<()>, -} - -impl AppLivenessToken { - /// Returns true if the app is still alive. - pub fn is_alive(&self) -> bool { - self.sentinel.strong_count() > 0 - } +pub struct RunnableMeta { + /// Location of the runnable + pub location: &'static core::panic::Location<'static>, + /// Weak reference to check if the app is still alive before running this task + pub app: Option>, } -impl std::fmt::Debug for AppLivenessToken { +impl std::fmt::Debug for RunnableMeta { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("AppLivenessToken") - .field("alive", &self.is_alive()) + f.debug_struct("RunnableMeta") + .field("location", &self.location) + .field("app_alive", &self.is_app_alive()) .finish() } } -/// This type is public so that our test macro can generate and use it, but it should not -/// be considered part of our public API. -#[doc(hidden)] -#[derive(Debug)] -pub struct RunnableMeta { - /// Location of the runnable - pub location: &'static core::panic::Location<'static>, - /// Token to check if the app is still alive before running. - /// This is `Some` for foreground tasks spawned with app tracking. - pub app: Option, +impl RunnableMeta { + /// Returns true if the app is still alive (or if no app tracking is configured). + pub fn is_app_alive(&self) -> bool { + match &self.app { + Some(weak) => weak.strong_count() > 0, + None => true, + } + } } #[doc(hidden)] diff --git a/crates/gpui/src/platform/mac/dispatcher.rs b/crates/gpui/src/platform/mac/dispatcher.rs index a9a5aa1c6919422e3cb9760bc28e87ebc69a1bfc..c10998ed3da49d1837929ab2ff93d19b035812df 100644 --- a/crates/gpui/src/platform/mac/dispatcher.rs +++ b/crates/gpui/src/platform/mac/dispatcher.rs @@ -254,11 +254,9 @@ extern "C" fn trampoline(runnable: *mut c_void) { let metadata = task.metadata(); let location = metadata.location; - if let Some(ref app_token) = metadata.app { - if !app_token.is_alive() { - drop(task); - return; - } + if !metadata.is_app_alive() { + drop(task); + return; } let start = Instant::now(); diff --git a/crates/gpui/src/platform/test/dispatcher.rs b/crates/gpui/src/platform/test/dispatcher.rs index 5c35c7c1160cc2f1d875c7ac904d59b25b215f6f..8ff761cb64d71a7830f2760b682ea7a84759b9bb 100644 --- a/crates/gpui/src/platform/test/dispatcher.rs +++ b/crates/gpui/src/platform/test/dispatcher.rs @@ -178,12 +178,10 @@ impl TestDispatcher { // todo(localcc): add timings to tests match runnable { RunnableVariant::Meta(runnable) => { - if let Some(ref app_token) = runnable.metadata().app { - if !app_token.is_alive() { - drop(runnable); - self.state.lock().is_main_thread = was_main_thread; - return true; - } + if !runnable.metadata().is_app_alive() { + drop(runnable); + self.state.lock().is_main_thread = was_main_thread; + return true; } runnable.run() }