@@ -51,14 +51,15 @@
},
{
"id": "1.8",
- "description": "Write unit test verifying task cancellation when app is dropped",
- "file": "crates/gpui/tests/task_cancellation.rs",
- "status": "completed"
+ "description": "Write unit tests verifying task cancellation when app is dropped (moved to executor.rs to access pub(crate) items)",
+ "file": "crates/gpui/src/executor.rs",
+ "status": "completed",
+ "notes": "Tests moved from tests/task_cancellation.rs to executor.rs as #[cfg(test)] module because TestAppContext holds strong Rc<AppCell> preventing proper app-drop testing"
},
{
"id": "1.9",
"description": "Write unit test for nested tasks both cancelling cleanly",
- "file": "crates/gpui/tests/task_cancellation.rs",
+ "file": "crates/gpui/src/executor.rs",
"status": "completed"
},
{
@@ -125,6 +125,30 @@ impl<T> Task<T> {
Task(TaskState::Spawned(task)) => task.detach(),
}
}
+
+ /// Converts this task into a fallible task that returns `Option<T>`.
+ ///
+ /// Unlike the standard `Task<T>`, a [`FallibleTask`] will return `None`
+ /// if the app was dropped while the task is executing.
+ ///
+ /// # Example
+ ///
+ /// ```ignore
+ /// // Background task that gracefully handles app shutdown:
+ /// cx.background_spawn(async move {
+ /// let result = foreground_task.fallible().await;
+ /// if let Some(value) = result {
+ /// // Process the value
+ /// }
+ /// // If None, app was shut down - just exit gracefully
+ /// }).detach();
+ /// ```
+ pub fn fallible(self) -> FallibleTask<T> {
+ FallibleTask(match self.0 {
+ TaskState::Ready(val) => FallibleTaskState::Ready(val),
+ TaskState::Spawned(task) => FallibleTaskState::Spawned(task.fallible()),
+ })
+ }
}
impl<E, T> Task<Result<T, E>>
@@ -154,6 +178,55 @@ impl<T> Future for Task<T> {
}
}
+/// A task that returns `Option<T>` instead of panicking when cancelled.
+#[must_use]
+pub struct FallibleTask<T>(FallibleTaskState<T>);
+
+enum FallibleTaskState<T> {
+ /// A task that is ready to return a value
+ Ready(Option<T>),
+
+ /// A task that is currently running (wraps async_task::FallibleTask).
+ Spawned(async_task::FallibleTask<T, RunnableMeta>),
+}
+
+impl<T> FallibleTask<T> {
+ /// Creates a new fallible task that will resolve with the value.
+ pub fn ready(val: T) -> Self {
+ FallibleTask(FallibleTaskState::Ready(Some(val)))
+ }
+
+ /// Detaching a task runs it to completion in the background.
+ pub fn detach(self) {
+ match self.0 {
+ FallibleTaskState::Ready(_) => {}
+ FallibleTaskState::Spawned(task) => task.detach(),
+ }
+ }
+}
+
+impl<T> Future for FallibleTask<T> {
+ type Output = Option<T>;
+
+ fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
+ match unsafe { self.get_unchecked_mut() } {
+ FallibleTask(FallibleTaskState::Ready(val)) => Poll::Ready(val.take()),
+ FallibleTask(FallibleTaskState::Spawned(task)) => Pin::new(task).poll(cx),
+ }
+ }
+}
+
+impl<T> std::fmt::Debug for FallibleTask<T> {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match &self.0 {
+ FallibleTaskState::Ready(_) => f.debug_tuple("FallibleTask::Ready").finish(),
+ FallibleTaskState::Spawned(task) => {
+ f.debug_tuple("FallibleTask::Spawned").field(task).finish()
+ }
+ }
+ }
+}
+
/// A task label is an opaque identifier that you can use to
/// refer to a task in tests.
#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
@@ -898,3 +971,264 @@ impl Drop for Scope<'_> {
self.executor.block(self.rx.next());
}
}
+
+#[cfg(test)]
+mod test {
+ use super::*;
+ use crate::{App, TestDispatcher, TestPlatform};
+ use rand::SeedableRng;
+ use std::{cell::RefCell, rc::Weak};
+
+ #[test]
+ fn sanity_test_tasks_run() {
+ 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 platform = TestPlatform::new(background_executor, foreground_executor.clone());
+ 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 task_ran = Rc::new(RefCell::new(false));
+
+ foreground_executor
+ .spawn_with_app(Rc::downgrade(&app), {
+ let task_ran = Rc::clone(&task_ran);
+ async move {
+ *task_ran.borrow_mut() = true;
+ }
+ })
+ .detach();
+
+ // Run dispatcher while app is still alive
+ dispatcher.run_until_parked();
+
+ // Task should have run
+ assert!(
+ *task_ran.borrow(),
+ "Task should run normally when app is alive"
+ );
+ }
+
+ #[test]
+ fn test_task_cancelled_when_app_dropped() {
+ 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 platform = TestPlatform::new(background_executor, foreground_executor.clone());
+ 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 app_weak = Rc::downgrade(&app);
+
+ let task_ran = Rc::new(RefCell::new(false));
+ let task_ran_clone = Rc::clone(&task_ran);
+
+ foreground_executor
+ .spawn_with_app(Weak::clone(&app_weak), async move {
+ *task_ran_clone.borrow_mut() = true;
+ })
+ .detach();
+
+ assert!(
+ Rc::weak_count(&app) > 0,
+ "Task should hold a weak reference"
+ );
+
+ drop(app);
+
+ assert!(app_weak.upgrade().is_none(), "App should have been dropped");
+
+ dispatcher.run_until_parked();
+
+ // The task should have been cancelled, not run
+ assert!(
+ !*task_ran.borrow(),
+ "Task should have been cancelled when app was dropped, but it ran!"
+ );
+ }
+
+ #[test]
+ fn test_nested_tasks_both_cancel() {
+ 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 platform = TestPlatform::new(background_executor, foreground_executor.clone());
+ 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 app_weak = Rc::downgrade(&app);
+
+ let outer_completed = Rc::new(RefCell::new(false));
+ let inner_completed = Rc::new(RefCell::new(false));
+ let reached_await = Rc::new(RefCell::new(false));
+
+ let outer_flag = Rc::clone(&outer_completed);
+ let inner_flag = Rc::clone(&inner_completed);
+ let await_flag = Rc::clone(&reached_await);
+
+ // Channel to block the inner task until we're ready
+ let (tx, rx) = futures::channel::oneshot::channel::<()>();
+
+ // We need clones of executor and app_weak for the inner spawn
+ let inner_executor = foreground_executor.clone();
+ let inner_app_weak = app_weak.clone();
+
+ // Spawn outer task that will spawn and await an inner task
+ foreground_executor
+ .spawn_with_app(Weak::clone(&app_weak), async move {
+ let inner_flag_clone = Rc::clone(&inner_flag);
+
+ // Spawn inner task that blocks on a channel
+ let inner_task = inner_executor.spawn_with_app(inner_app_weak, async move {
+ // Wait for signal (which will never come - we'll drop the app instead)
+ rx.await.ok();
+ *inner_flag_clone.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();
+
+ // Run dispatcher until outer task reaches the await point
+ // The inner task will be blocked on the channel
+ dispatcher.run_until_parked();
+
+ // Verify we actually reached the await point before dropping the app
+ assert!(
+ *reached_await.borrow(),
+ "Outer task should have reached the await point"
+ );
+
+ // Neither task should have completed yet
+ assert!(
+ !*outer_completed.borrow(),
+ "Outer task should not have completed yet"
+ );
+ assert!(
+ !*inner_completed.borrow(),
+ "Inner task should not have completed yet"
+ );
+
+ // Drop the channel sender and app while outer is awaiting inner
+ drop(tx);
+ drop(app);
+ assert!(app_weak.upgrade().is_none(), "App should have been dropped");
+
+ // Run dispatcher - both tasks should be cancelled
+ dispatcher.run_until_parked();
+
+ // Neither task should have completed (both were cancelled)
+ assert!(
+ !*outer_completed.borrow(),
+ "Outer task should have been cancelled, not completed"
+ );
+ assert!(
+ !*inner_completed.borrow(),
+ "Inner task should have been cancelled, not completed"
+ );
+ }
+
+ #[test]
+ fn test_task_without_app_tracking_still_runs() {
+ 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 platform = TestPlatform::new(background_executor, foreground_executor.clone());
+ 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 app_weak = Rc::downgrade(&app);
+
+ let task_ran = Rc::new(RefCell::new(false));
+ let task_ran_clone = Rc::clone(&task_ran);
+
+ let _task = foreground_executor.spawn(async move {
+ *task_ran_clone.borrow_mut() = true;
+ });
+
+ drop(app);
+
+ assert!(app_weak.upgrade().is_none(), "App should have been dropped");
+
+ dispatcher.run_until_parked();
+
+ assert!(
+ *task_ran.borrow(),
+ "Task without app tracking should still run after app is dropped"
+ );
+ }
+
+ #[test]
+ #[should_panic]
+ fn test_polling_cancelled_task_panics() {
+ 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 platform = TestPlatform::new(background_executor.clone(), foreground_executor.clone());
+ 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 app_weak = Rc::downgrade(&app);
+
+ let task = foreground_executor.spawn_with_app(Weak::clone(&app_weak), async move { 42 });
+
+ drop(app);
+
+ assert!(app_weak.upgrade().is_none(), "App should have been dropped");
+
+ dispatcher.run_until_parked();
+
+ background_executor.block(task);
+ }
+
+ #[test]
+ fn test_polling_cancelled_task_returns_none_with_fallible() {
+ 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 platform = TestPlatform::new(background_executor.clone(), foreground_executor.clone());
+ 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 app_weak = Rc::downgrade(&app);
+
+ let task = foreground_executor
+ .spawn_with_app(Weak::clone(&app_weak), async move { 42 })
+ .fallible();
+
+ drop(app);
+
+ assert!(app_weak.upgrade().is_none(), "App should have been dropped");
+
+ dispatcher.run_until_parked();
+
+ let result = background_executor.block(task);
+ assert_eq!(result, None, "Cancelled task should return None");
+ }
+}