From 677e8174b53c9a5eaec975bf7cc7582231f176fc Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 26 Aug 2026 14:33:10 +0000 Subject: [PATCH] Flush all persistence threads for the rest for various shutdown cases (#63213) Follow-up to https://github.com/zed-industries/zed/pull/60477 Closes #25022 We have more places that need a similar synchronization before quitting the app: restarts on update, reloading the workspace. On top of it, two more findings: * `on_app_will_quit` was redundant: the global `on_app_quit` hook flushes a strict superset of the same tasks and runs before windows are cleared, while the entity hook held only a weak handle and could never flush the `Window`-dependent workspace state anyway. * previously, shutdown was run as `cx.spawn`, yet main threads got parked on app shutdown: now using `cx.background_spawn` instead Release Notes: - Improved data persistence during various app shutdown cases --------- Co-authored-by: Kostia R --- .../src/component_preview.rs | 1 - crates/editor/src/items.rs | 43 +- crates/git_ui/src/branch_diff.rs | 1 - crates/git_ui/src/git_graph.rs | 1 - crates/git_ui/src/project_diff.rs | 1 - crates/git_ui/src/staged_diff.rs | 1 - crates/git_ui/src/unstaged_diff.rs | 1 - crates/image_viewer/src/image_viewer.rs | 1 - crates/keymap_editor/src/keymap_editor.rs | 1 - .../src/markdown_preview_view.rs | 9 +- crates/onboarding/src/onboarding.rs | 1 - crates/terminal_view/src/terminal_view.rs | 1 - crates/workspace/src/item.rs | 6 +- crates/workspace/src/multi_workspace.rs | 131 ++---- crates/workspace/src/persistence.rs | 53 ++- crates/workspace/src/welcome.rs | 1 - crates/workspace/src/workspace.rs | 219 +++++++-- crates/zed/src/zed.rs | 422 ++++++++++++++---- crates/zed/src/zed/move_to_applications.rs | 15 +- 19 files changed, 669 insertions(+), 240 deletions(-) diff --git a/crates/component_preview/src/component_preview.rs b/crates/component_preview/src/component_preview.rs index 73ad50d6a5..0b36e6ef97 100644 --- a/crates/component_preview/src/component_preview.rs +++ b/crates/component_preview/src/component_preview.rs @@ -863,7 +863,6 @@ impl SerializableItem for ComponentPreview { _workspace: &mut Workspace, item_id: ItemId, _closing: bool, - _window: &mut Window, cx: &mut Context, ) -> Option>> { let active_page = self.active_page_id(cx); diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 3461733c81..ecc3587687 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1454,7 +1454,6 @@ impl SerializableItem for Editor { workspace: &mut Workspace, item_id: ItemId, closing: bool, - window: &mut Window, cx: &mut Context, ) -> Option>> { let buffer_serialization = self.buffer_serialization?; @@ -1494,31 +1493,25 @@ impl SerializableItem for Editor { let snapshot = buffer.read(cx).snapshot(); let db = EditorDb::global(cx); - Some(cx.spawn_in(window, async move |_this, cx| { - cx.background_spawn(async move { - let (contents, language) = if serialize_dirty_buffers && is_dirty { - let contents = snapshot.text(); - let language = snapshot.language().map(|lang| lang.name().to_string()); - (Some(contents), language) - } else { - (None, None) - }; + Some(cx.background_spawn(async move { + let (contents, language) = if serialize_dirty_buffers && is_dirty { + let contents = snapshot.text(); + let language = snapshot.language().map(|lang| lang.name().to_string()); + (Some(contents), language) + } else { + (None, None) + }; - let editor = SerializedEditor { - abs_path, - contents, - language, - mtime, - }; - log::debug!("Serializing editor {item_id:?} in workspace {workspace_id:?}"); - db.save_serialized_editor(item_id, workspace_id, editor) - .await - .context("failed to save serialized editor") - }) - .await - .context("failed to save contents of buffer")?; - - Ok(()) + let editor = SerializedEditor { + abs_path, + contents, + language, + mtime, + }; + log::debug!("Serializing editor {item_id:?} in workspace {workspace_id:?}"); + db.save_serialized_editor(item_id, workspace_id, editor) + .await + .context("failed to save serialized editor") })) } diff --git a/crates/git_ui/src/branch_diff.rs b/crates/git_ui/src/branch_diff.rs index d4684ba981..c1f6f95bfa 100644 --- a/crates/git_ui/src/branch_diff.rs +++ b/crates/git_ui/src/branch_diff.rs @@ -682,7 +682,6 @@ impl SerializableItem for BranchDiff { workspace: &mut Workspace, item_id: workspace::ItemId, _closing: bool, - _window: &mut Window, cx: &mut Context, ) -> Option>> { let workspace_id = workspace.database_id()?; diff --git a/crates/git_ui/src/git_graph.rs b/crates/git_ui/src/git_graph.rs index 9f8ea3f5ee..d304a9db4d 100644 --- a/crates/git_ui/src/git_graph.rs +++ b/crates/git_ui/src/git_graph.rs @@ -4307,7 +4307,6 @@ impl workspace::SerializableItem for GitGraph { workspace: &mut Workspace, item_id: workspace::ItemId, _closing: bool, - _window: &mut Window, cx: &mut Context, ) -> Option>> { let workspace_id = workspace.database_id()?; diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index 3d76d4a7d8..a540038f73 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -622,7 +622,6 @@ impl SerializableItem for ProjectDiff { _: &mut Workspace, _: workspace::ItemId, _: bool, - _: &mut Window, _: &mut Context, ) -> Option>> { Some(Task::ready(Ok(()))) diff --git a/crates/git_ui/src/staged_diff.rs b/crates/git_ui/src/staged_diff.rs index d337dab979..b37cb1fee7 100644 --- a/crates/git_ui/src/staged_diff.rs +++ b/crates/git_ui/src/staged_diff.rs @@ -514,7 +514,6 @@ impl SerializableItem for StagedDiff { _: &mut Workspace, _: workspace::ItemId, _: bool, - _: &mut Window, _: &mut Context, ) -> Option>> { Some(Task::ready(Ok(()))) diff --git a/crates/git_ui/src/unstaged_diff.rs b/crates/git_ui/src/unstaged_diff.rs index 468cfd97a6..48bcb4e5ab 100644 --- a/crates/git_ui/src/unstaged_diff.rs +++ b/crates/git_ui/src/unstaged_diff.rs @@ -588,7 +588,6 @@ impl SerializableItem for UnstagedDiff { _: &mut Workspace, _: workspace::ItemId, _: bool, - _: &mut Window, _: &mut Context, ) -> Option>> { Some(Task::ready(Ok(()))) diff --git a/crates/image_viewer/src/image_viewer.rs b/crates/image_viewer/src/image_viewer.rs index 623ee0f9e3..74eb016d4a 100644 --- a/crates/image_viewer/src/image_viewer.rs +++ b/crates/image_viewer/src/image_viewer.rs @@ -737,7 +737,6 @@ impl SerializableItem for ImageView { workspace: &mut Workspace, item_id: ItemId, _closing: bool, - _window: &mut Window, cx: &mut Context, ) -> Option>> { let workspace_id = workspace.database_id()?; diff --git a/crates/keymap_editor/src/keymap_editor.rs b/crates/keymap_editor/src/keymap_editor.rs index dfa4d9bdcf..7ce1740115 100644 --- a/crates/keymap_editor/src/keymap_editor.rs +++ b/crates/keymap_editor/src/keymap_editor.rs @@ -3961,7 +3961,6 @@ impl SerializableItem for KeymapEditor { workspace: &mut Workspace, item_id: workspace::ItemId, _closing: bool, - _window: &mut Window, cx: &mut ui::Context, ) -> Option>> { let workspace_id = workspace.database_id()?; diff --git a/crates/markdown_preview/src/markdown_preview_view.rs b/crates/markdown_preview/src/markdown_preview_view.rs index 48b8085234..2f18d914a3 100644 --- a/crates/markdown_preview/src/markdown_preview_view.rs +++ b/crates/markdown_preview/src/markdown_preview_view.rs @@ -2000,7 +2000,6 @@ impl SerializableItem for MarkdownPreviewView { workspace: &mut Workspace, item_id: ItemId, _closing: bool, - _window: &mut Window, cx: &mut Context, ) -> Option>> { let workspace_id = workspace.database_id()?; @@ -3121,12 +3120,12 @@ mod tests { } let serialize_task = multi_workspace - .update(cx, |multi_workspace, window, cx| { + .update(cx, |multi_workspace, _window, cx| { let workspace = multi_workspace.workspace().clone(); workspace.update(cx, |workspace, cx| { preview .update(cx, |preview, cx| { - preview.serialize(workspace, cx.entity_id().as_u64(), false, window, cx) + preview.serialize(workspace, cx.entity_id().as_u64(), false, cx) }) .unwrap() }) @@ -3269,12 +3268,12 @@ mod tests { wait_for_preview_serialization(cx).await; let serialize_task = multi_workspace - .update(cx, |multi_workspace, window, cx| { + .update(cx, |multi_workspace, _window, cx| { let workspace = multi_workspace.workspace().clone(); workspace.update(cx, |workspace, cx| { preview .update(cx, |preview, cx| { - preview.serialize(workspace, cx.entity_id().as_u64(), false, window, cx) + preview.serialize(workspace, cx.entity_id().as_u64(), false, cx) }) .unwrap() }) diff --git a/crates/onboarding/src/onboarding.rs b/crates/onboarding/src/onboarding.rs index 34d27a0240..220c2b0005 100644 --- a/crates/onboarding/src/onboarding.rs +++ b/crates/onboarding/src/onboarding.rs @@ -630,7 +630,6 @@ impl workspace::SerializableItem for Onboarding { workspace: &mut Workspace, item_id: workspace::ItemId, _closing: bool, - _window: &mut Window, cx: &mut ui::Context, ) -> Option>> { let workspace_id = workspace.database_id()?; diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 8c8a0c6619..5f8cf0c3a3 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -1866,7 +1866,6 @@ impl SerializableItem for TerminalView { _workspace: &mut Workspace, item_id: workspace::ItemId, _closing: bool, - _: &mut Window, cx: &mut Context, ) -> Option>> { let terminal = self.terminal().read(cx); diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index 088e12a6dc..ab4195f6a4 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -430,7 +430,6 @@ pub trait SerializableItem: Item { workspace: &mut Workspace, item_id: ItemId, closing: bool, - window: &mut Window, cx: &mut Context, ) -> Option>>; @@ -443,7 +442,6 @@ pub trait SerializableItemHandle: ItemHandle { &self, workspace: &mut Workspace, closing: bool, - window: &mut Window, cx: &mut App, ) -> Option>>; fn should_serialize(&self, event: &dyn Any, cx: &App) -> bool; @@ -461,11 +459,10 @@ where &self, workspace: &mut Workspace, closing: bool, - window: &mut Window, cx: &mut App, ) -> Option>> { self.update(cx, |this, cx| { - this.serialize(workspace, cx.entity_id().as_u64(), closing, window, cx) + this.serialize(workspace, cx.entity_id().as_u64(), closing, cx) }) } @@ -1876,7 +1873,6 @@ pub mod test { _workspace: &mut Workspace, _item_id: ItemId, _closing: bool, - _window: &mut Window, _cx: &mut Context, ) -> Option>> { if let Some(serialize) = self.serialize.take() { diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 283fc9d09a..529e5b16c7 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{Context as _, Result}; use fs::Fs; use gpui::{ @@ -12,7 +12,6 @@ use remote::RemoteConnectionOptions; use settings::Settings; pub use settings::SidebarSide; use std::cell::Cell; -use std::future::Future; use std::path::PathBuf; use std::rc::Rc; use ui::prelude::*; @@ -348,7 +347,6 @@ impl MultiWorkspace { task.detach(); } }); - let quit_subscription = cx.on_app_quit(Self::app_will_quit); let settings_subscription = cx.observe_global_in::(window, { let mut previous_multi_workspace_enabled = !DisableAiSettings::get_global(cx) .disable_ai @@ -381,11 +379,7 @@ impl MultiWorkspace { sidebar_overlay: None, pending_removal_tasks: Vec::new(), _serialize_task: None, - _subscriptions: vec![ - release_subscription, - quit_subscription, - settings_subscription, - ], + _subscriptions: vec![release_subscription, settings_subscription], previous_focus_handle: None, } } @@ -559,44 +553,25 @@ impl MultiWorkspace { } pub fn close_window(&mut self, _: &CloseWindow, window: &mut Window, cx: &mut Context) { - cx.spawn_in(window, async move |this, cx| { - let workspaces = this.update(cx, |multi_workspace, _cx| { - multi_workspace.workspaces().cloned().collect::>() - })?; - - let mut prepared = anyhow::Ok(true); - for workspace in workspaces { - prepared = match workspace.update_in(cx, |workspace, window, cx| { - workspace.prepare_to_close(CloseIntent::CloseWindow, window, cx) - }) { - Ok(task) => task.await, - Err(error) => Err(error), - }; - if !matches!(prepared, Ok(true)) { - break; - } - } - - if !matches!(prepared, Ok(true)) { - this.update(cx, |multi_workspace, cx| { - for workspace in multi_workspace.workspaces() { - workspace.update(cx, |workspace, _| { - workspace.removing = false; - }); - } - })?; - prepared?; + let Some(window_handle) = window.window_handle().downcast::() else { + log::error!("cannot close a window whose root is not a MultiWorkspace"); + return; + }; + cx.spawn(async move |_, cx| { + if !crate::prepare_window_to_close(window_handle, CloseIntent::CloseWindow, cx) + .await + .context("preparing the window to close")? + { return anyhow::Ok(()); } - let flush_tasks = this.update_in(cx, |multi_workspace, window, cx| { - multi_workspace.flush_pending_serialization(window, cx) - })?; - futures::future::join_all(flush_tasks).await; + crate::flush_windows_serialization(&[window_handle], cx).await; - cx.update(|window, _cx| { - window.remove_window(); - })?; + window_handle + .update(cx, |_, window, _cx| { + window.remove_window(); + }) + .context("removing the closed window")?; anyhow::Ok(()) }) @@ -1467,39 +1442,41 @@ impl MultiWorkspace { pub fn serialize(&mut self, cx: &mut Context) { self._serialize_task = Some(cx.spawn(async move |this, cx| { - let Some((window_id, state)) = this - .read_with(cx, |this, cx| { - let state = MultiWorkspaceState { - active_workspace_id: this.workspace().read(cx).database_id(), - project_groups: this - .project_groups - .iter() - .map(|group| { - crate::persistence::model::SerializedProjectGroup::from_group( - &group.key, - group.expanded, - ) - }) - .collect::>(), - sidebar_open: this.sidebar_open, - sidebar_state: this.sidebar.as_ref().and_then(|s| s.serialized_state(cx)), - }; - (this.window_id, state) - }) - .ok() - else { + let Ok(task) = this.update(cx, |this, cx| this.serialize_now(cx)) else { return; }; - let kvp = cx.update(|cx| db::kvp::KeyValueStore::global(cx)); - crate::persistence::write_multi_workspace_state(&kvp, window_id, state).await; + task.await; })); } - /// Returns the in-flight serialization task (if any) so the caller can - /// await it. Used by the quit handler to ensure pending DB writes + fn serialize_now(&mut self, cx: &mut Context) -> Task<()> { + let state = MultiWorkspaceState { + active_workspace_id: self.workspace().read(cx).database_id(), + project_groups: self + .project_groups + .iter() + .map(|group| { + crate::persistence::model::SerializedProjectGroup::from_group( + &group.key, + group.expanded, + ) + }) + .collect::>(), + sidebar_open: self.sidebar_open, + sidebar_state: self.sidebar.as_ref().and_then(|s| s.serialized_state(cx)), + }; + let window_id = self.window_id; + let kvp = db::kvp::KeyValueStore::global(cx); + cx.background_spawn(async move { + crate::persistence::write_multi_workspace_state(&kvp, window_id, state).await; + }) + } + + /// Used by the quit handler to ensure pending DB writes /// complete before the process exits. - pub fn flush_serialization(&mut self) -> Task<()> { - self._serialize_task.take().unwrap_or(Task::ready(())) + pub fn flush_serialization(&mut self, cx: &mut Context) -> Task<()> { + self._serialize_task.take(); + self.serialize_now(cx) } pub fn flush_pending_serialization( @@ -1514,22 +1491,10 @@ impl MultiWorkspace { })); } tasks.append(&mut self.take_pending_removal_tasks()); - tasks.push(self.flush_serialization()); + tasks.push(self.flush_serialization(cx)); tasks } - fn app_will_quit(&mut self, _cx: &mut Context) -> impl Future + use<> { - let mut tasks: Vec> = Vec::new(); - if let Some(task) = self._serialize_task.take() { - tasks.push(task); - } - tasks.extend(std::mem::take(&mut self.pending_removal_tasks)); - - async move { - futures::future::join_all(tasks).await; - } - } - pub fn focus_active_workspace(&self, window: &mut Window, cx: &mut App) { let focus_handle = self.workspace().read(cx).fallback_focus_handle(window, cx); window.focus(&focus_handle, cx); @@ -1757,7 +1722,7 @@ impl MultiWorkspace { })); } } - self.serialize(cx); + tasks.push(self.flush_serialization(cx)); tasks } diff --git a/crates/workspace/src/persistence.rs b/crates/workspace/src/persistence.rs index eeda4cef67..9c3c33458a 100644 --- a/crates/workspace/src/persistence.rs +++ b/crates/workspace/src/persistence.rs @@ -4765,6 +4765,57 @@ mod tests { ); } + #[gpui::test] + async fn test_pending_serialization_flushed_on_shutdown(cx: &mut gpui::TestAppContext) { + crate::tests::init_test(cx); + + let app_state = cx.update(crate::AppState::test); + cx.update(|cx| crate::init(app_state.clone(), cx)); + + let fs = fs::FakeFs::new(cx.executor()); + let dir = unique_test_dir(&fs, "shutdown-flush").await; + let project = Project::test(fs.clone(), [dir.as_path()], cx).await; + + let (multi_workspace, vcx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + + let workspace = multi_workspace.read_with(vcx, |mw, _| mw.workspace().clone()); + + let db = vcx.update(|_, cx| WorkspaceDb::global(cx)); + let workspace_id = db.next_id().await.unwrap(); + workspace.update(vcx, |ws, _cx| { + ws.set_database_id(workspace_id); + }); + + multi_workspace.update_in(vcx, |multi_workspace, window, cx| { + multi_workspace.workspace().update(cx, |workspace, cx| { + workspace.serialize_workspace(window, cx) + }) + }); + + let serialized_paths = db + .workspace_for_id(workspace_id) + .expect("next_id should have reserved a row") + .paths; + assert_eq!( + serialized_paths.paths().len(), + 0, + "the debounced serialization must not have fired yet" + ); + + cx.update(|cx| cx.shutdown()); + + let serialized_paths = db + .workspace_for_id(workspace_id) + .expect("the workspace row should still exist after shutdown") + .paths; + assert_eq!( + serialized_paths.paths(), + std::slice::from_ref(&dir), + "shutdown should flush the pending workspace serialization" + ); + } + #[gpui::test] async fn test_create_workspace_serialization(cx: &mut gpui::TestAppContext) { crate::tests::init_test(cx); @@ -5092,7 +5143,7 @@ mod tests { // Note: removal_tasks may be empty if the background task already // completed (take_pending_removal_tasks filters out ready tasks). tasks.append(&mut removal_tasks); - tasks.push(mw.flush_serialization()); + tasks.push(mw.flush_serialization(cx)); tasks }); futures::future::join_all(all_tasks).await; diff --git a/crates/workspace/src/welcome.rs b/crates/workspace/src/welcome.rs index 7966a3be16..837418e8a6 100644 --- a/crates/workspace/src/welcome.rs +++ b/crates/workspace/src/welcome.rs @@ -585,7 +585,6 @@ impl crate::SerializableItem for WelcomePage { workspace: &mut Workspace, item_id: crate::ItemId, _closing: bool, - _window: &mut Window, cx: &mut Context, ) -> Option>> { let workspace_id = workspace.database_id()?; diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 82ee3f6e7d..aac63cb1c9 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -60,12 +60,13 @@ use futures::{ future::{Shared, try_join_all}, }; use gpui::{ - Action, AnyEntity, AnyView, AnyWeakView, App, AsyncApp, AsyncWindowContext, Axis, Bounds, - Context, CursorStyle, Decorations, DragMoveEvent, Entity, EntityId, EventEmitter, FocusHandle, - Focusable, Global, HitboxBehavior, Hsla, KeyContext, Keystroke, ManagedView, MouseButton, - PathPromptOptions, Point, PromptLevel, Render, ResizeEdge, Size, Stateful, Subscription, - SystemWindowTabController, Task, TaskExt, Tiling, WeakEntity, WindowBounds, WindowHandle, - WindowId, WindowOptions, actions, canvas, point, relative, size, transparent_black, + Action, AnyEntity, AnyView, AnyWeakView, App, AppContext, AsyncApp, AsyncWindowContext, Axis, + Bounds, Context, CursorStyle, Decorations, DragMoveEvent, Entity, EntityId, EventEmitter, + FocusHandle, Focusable, Global, HitboxBehavior, Hsla, KeyContext, Keystroke, ManagedView, + MouseButton, PathPromptOptions, Point, PromptLevel, Render, ResizeEdge, Size, Stateful, + Subscription, SystemWindowTabController, Task, TaskExt, Tiling, WeakEntity, WindowBounds, + WindowHandle, WindowId, WindowOptions, actions, canvas, point, relative, size, + transparent_black, }; pub use history_manager::*; pub use item::{ @@ -578,7 +579,7 @@ actions!( ] ); -#[derive(PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum CloseIntent { /// Quit the program entirely. Quit, @@ -788,6 +789,8 @@ pub fn init(app_state: Arc, cx: &mut App) { toast_layer::init(cx); history_manager::init(app_state.fs.clone(), cx); + cx.on_app_quit(flush_windows_serialization_on_quit).detach(); + cx.on_action(|_: &CloseWindow, cx| Workspace::close_global(cx)) .on_action(|_: &Reload, cx| reload(cx)) .on_action(|action: &Open, cx: &mut App| { @@ -3637,11 +3640,11 @@ impl Workspace { let mut serialize_tasks = Vec::new(); let mut remaining_dirty_items = Vec::new(); if allow_hot_exit_serialization { - workspace.update_in(cx, |workspace, window, cx| { + workspace.update(cx, |workspace, cx| { for (pane, item) in dirty_items { if let Some(task) = item .to_serializable_item_handle(cx) - .and_then(|handle| handle.serialize(workspace, true, window, cx)) + .and_then(|handle| handle.serialize(workspace, true, cx)) { serialize_tasks.push((pane, item, task)); } else { @@ -7114,20 +7117,42 @@ impl Workspace { }) } - /// Bypass the 200ms serialization throttle and write workspace state to - /// the DB immediately. Returns a task the caller can await to ensure the - /// write completes. Used by the quit handler so the most recent state - /// isn't lost to a pending throttle timer when the process exits. + /// Bypass the serialization throttles and write workspace and item state + /// to the DB immediately. Returns a task the caller can await to ensure the + /// writes complete before the process exits. pub fn flush_serialization(&mut self, window: &mut Window, cx: &mut App) -> Task<()> { self._schedule_serialize_workspace.take(); self._serialize_workspace_task.take(); self.bounds_save_task_queued.take(); + let serializable_items = self + .panes + .iter() + .flat_map(|pane| pane.read(cx).items()) + .filter_map(|item| item.to_serializable_item_handle(cx)) + .fold(HashMap::default(), |mut items, item| { + items.entry(item.item_id()).or_insert(item); + items + }); + let item_tasks = serializable_items + .into_values() + .filter_map(|item| { + let item_id = item.item_id(); + let task = item.serialize(self, false, cx)?; + Some(async move { + task.await + .with_context(|| format!("flushing serialization of item {item_id:?}")) + }) + }) + .collect::>(); let bounds_task = self.save_window_bounds(window, cx); let serialize_task = self.serialize_workspace_internal(window, cx); - cx.spawn(async move |_| { + cx.background_spawn(async move { bounds_task.await; serialize_task.await; + for result in futures::future::join_all(item_tasks).await { + result.log_err(); + } }) } @@ -7285,7 +7310,7 @@ impl Workspace { }; let db = WorkspaceDb::global(cx); - window.spawn(cx, async move |_| { + cx.background_spawn(async move { db.save_workspace(serialized_workspace).await; }) } @@ -7296,7 +7321,7 @@ impl Workspace { let docks = build_serialized_docks(self, window, cx); let db = WorkspaceDb::global(cx); let kvp = db::kvp::KeyValueStore::global(cx); - window.spawn(cx, async move |_| { + cx.background_spawn(async move { let open_status_write = db.set_window_open_status( database_id, window_bounds, @@ -7316,7 +7341,7 @@ impl Workspace { // Save dock state for empty non-local workspaces let docks = build_serialized_docks(self, window, cx); let kvp = db::kvp::KeyValueStore::global(cx); - window.spawn(cx, async move |_| { + cx.background_spawn(async move { persistence::write_default_dock_state(&kvp, docks) .await .log_err(); @@ -7380,9 +7405,9 @@ impl Workspace { // We use into_iter() here so that the references to the items are moved into // the tasks and not kept alive while we're sleeping. for (_, item) in unique_items.into_iter() { - if let Ok(Some(task)) = this.update_in(cx, |workspace, window, cx| { - item.serialize(workspace, false, window, cx) - }) { + if let Ok(Some(task)) = + this.update(cx, |workspace, cx| item.serialize(workspace, false, cx)) + { cx.background_spawn(async move { task.await.log_err() }) .detach(); } @@ -11213,17 +11238,8 @@ pub fn reload(cx: &mut App) { } } - // If the user cancels any save prompt, then keep the app open. - for window in workspace_windows { - if let Ok(should_close) = window.update(cx, |multi_workspace, window, cx| { - let workspace = multi_workspace.workspace().clone(); - workspace.update(cx, |workspace, cx| { - workspace.prepare_to_close(CloseIntent::Quit, window, cx) - }) - }) && !should_close.await? - { - return anyhow::Ok(()); - } + if !prepare_windows_to_quit(&workspace_windows, cx).await { + return anyhow::Ok(()); } cx.update(|cx| cx.restart()); anyhow::Ok(()) @@ -11231,6 +11247,147 @@ pub fn reload(cx: &mut App) { .detach_and_log_err(cx); } +pub async fn prepare_windows_to_quit( + workspace_windows: &[WindowHandle], + cx: &mut AsyncApp, +) -> bool { + // If the user cancels any save prompt, then keep the app open. + let mut prepared_windows = Vec::new(); + let mut cancelled = false; + for window in workspace_windows { + match prepare_window_to_close(*window, CloseIntent::Quit, cx).await { + Ok(true) => prepared_windows.push(*window), + Ok(false) => { + cancelled = true; + break; + } + Err(error) => { + log::error!( + "failed to prepare window {:?} to close before quitting: {error:#}", + window.window_id() + ); + cancelled = true; + break; + } + } + } + + if cancelled { + flush_windows_serialization(&prepared_windows, cx).await; + for window in prepared_windows { + window + .update(cx, |_, window, _cx| { + window.remove_window(); + }) + .log_err(); + } + return false; + } + + // Flush all pending workspace serialization before quitting so that + // session_id/window_id are up-to-date in the database. + flush_windows_serialization(workspace_windows, cx).await; + + true +} + +pub(crate) async fn prepare_window_to_close( + window: WindowHandle, + close_intent: CloseIntent, + cx: &mut AsyncApp, +) -> Result { + let active_and_workspaces = window + .update(cx, |multi_workspace, window, _cx| { + if close_intent == CloseIntent::Quit { + window.activate_window(); + } + ( + multi_workspace.workspace().clone(), + multi_workspace.workspaces().cloned().collect::>(), + ) + }) + .log_err(); + + let Some((originally_active, workspaces)) = active_and_workspaces else { + return Ok(true); + }; + + let mut prepared = anyhow::Ok(true); + for workspace in workspaces { + prepared = match window.update(cx, |_, window, cx| { + workspace.update(cx, |workspace, cx| { + workspace.prepare_to_close(close_intent, window, cx) + }) + }) { + Ok(task) => task.await, + Err(error) => Err(error), + } + .with_context(|| format!("preparing workspace {:?} to close", workspace.entity_id())); + if !matches!(prepared, Ok(true)) { + break; + } + } + + // Re-activate the workspace the user actually had focused so it is the + // one serialized (and restored on next launch) as active, rather than + // whichever happened to be last. + window + .update(cx, |multi_workspace, window, cx| { + if !matches!(prepared, Ok(true)) { + for workspace in multi_workspace.workspaces() { + workspace.update(cx, |workspace, _| { + workspace.removing = false; + }); + } + } + multi_workspace.activate(originally_active, None, window, cx); + }) + .log_err(); + + prepared +} + +pub async fn flush_windows_serialization( + workspace_windows: &[WindowHandle], + cx: &mut AsyncApp, +) { + let flush_tasks = collect_flush_tasks(workspace_windows, cx); + futures::future::join_all(flush_tasks).await; +} + +fn flush_windows_serialization_on_quit(cx: &mut App) -> impl Future + use<> { + let workspace_windows = cx + .windows() + .into_iter() + .filter_map(|window| window.downcast::()) + .collect::>(); + let flush_tasks = collect_flush_tasks(&workspace_windows, cx); + async move { + futures::future::join_all(flush_tasks).await; + } +} + +fn collect_flush_tasks( + workspace_windows: &[WindowHandle], + cx: &mut impl AppContext, +) -> Vec> { + let mut flush_tasks = Vec::new(); + for window in workspace_windows { + window + .update(cx, |multi_workspace, window, cx| { + flush_tasks.extend(multi_workspace.flush_pending_serialization(window, cx)); + }) + .with_context(|| { + format!( + "flushing pending serialization for window {:?}", + window.window_id() + ) + }) + .log_err(); + } + flush_tasks +} + fn parse_pixel_position_env_var(value: &str) -> Option> { let mut parts = value.split(','); let x: usize = parts.next()?.parse().ok()?; diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index b460196dd4..bbb11dc176 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -102,9 +102,7 @@ use workspace::{ WorkspaceSettings, create_and_open_local_file, notifications::simple_message_notification::MessageNotification, open_new, }; -use workspace::{ - CloseIntent, CloseProject, CloseWindow, RestoreBanner, with_active_or_new_workspace, -}; +use workspace::{CloseProject, CloseWindow, RestoreBanner, with_active_or_new_workspace}; use workspace::{Pane, notifications::DetachAndPromptErr}; use zed_actions::{ About, GetMerch, OpenAccountSettings, OpenBrowser, OpenDocs, OpenProjectTasks, @@ -1807,74 +1805,9 @@ fn quit(_: &Quit, cx: &mut App) { } } - // If the user cancels any save prompt, then keep the app open. - for window in &workspace_windows { - let window = *window; - let active_and_workspaces = window - .update(cx, |multi_workspace, _, _cx| { - ( - multi_workspace.workspace().clone(), - multi_workspace.workspaces().cloned().collect::>(), - ) - }) - .log_err(); - - let Some((originally_active, workspaces)) = active_and_workspaces else { - continue; - }; - - for workspace in workspaces { - if let Some(should_close) = window - .update(cx, |multi_workspace, window, cx| { - multi_workspace.activate(workspace.clone(), None, window, cx); - window.activate_window(); - workspace.update(cx, |workspace, cx| { - workspace.prepare_to_close(CloseIntent::Quit, window, cx) - }) - }) - .log_err() - { - if !should_close.await? { - // Activating each workspace above to surface its save - // prompts changed which workspace is active. Restore the - // user's focused workspace before bailing so the window - // is left as they had it. - window - .update(cx, |multi_workspace, window, cx| { - multi_workspace.activate( - originally_active.clone(), - None, - window, - cx, - ); - }) - .log_err(); - return Ok(()); - } - } - } - - // The loop above activated each workspace in turn, overwriting the - // persisted active workspace. Re-activate the workspace the user - // actually had focused so it is the one serialized (and restored on - // next launch) as active, rather than whichever happened to be last. - window - .update(cx, |multi_workspace, window, cx| { - multi_workspace.activate(originally_active, None, window, cx); - }) - .log_err(); + if !workspace::prepare_windows_to_quit(&workspace_windows, cx).await { + return Ok(()); } - // Flush all pending workspace serialization before quitting so that - // session_id/window_id are up-to-date in the database. - let mut flush_tasks = Vec::new(); - for window in &workspace_windows { - window - .update(cx, |multi_workspace, window, cx| { - flush_tasks.extend(multi_workspace.flush_pending_serialization(window, cx)); - }) - .log_err(); - } - futures::future::join_all(flush_tasks).await; cx.update(|cx| cx.quit()); anyhow::Ok(()) @@ -2971,7 +2904,7 @@ mod tests { }) }) .collect::>(); - tasks.push(multi_workspace.flush_serialization()); + tasks.push(multi_workspace.flush_serialization(cx)); tasks }) .unwrap(); @@ -6995,6 +6928,11 @@ mod tests { }) .unwrap(); + window1 + .update(cx, |_, window, _| window.activate_window()) + .unwrap(); + cx.run_until_parked(); + cx.dispatch_action(*window1, Quit); cx.run_until_parked(); @@ -7061,6 +6999,11 @@ mod tests { }) .unwrap(); + window1 + .update(cx, |_, window, _| window.activate_window()) + .unwrap(); + cx.run_until_parked(); + cx.dispatch_action(*window1, Quit); cx.run_until_parked(); @@ -7169,9 +7112,154 @@ mod tests { assert_eq!( cx.windows().len(), - 2, - "Case 3: Windows should still exist after cancelling quit" + 1, + "Case 3: The clean window should close when quit is cancelled in another window" ); + assert_eq!( + window1.update(cx, |_, _, _| ()).is_ok(), + false, + "Case 3: The clean window should have been closed" + ); + assert_eq!( + window2.update(cx, |_, _, _| ()).is_ok(), + true, + "Case 3: The window with the cancelled prompt should stay open" + ); + } + + #[gpui::test] + async fn test_reload_checks_all_workspaces_for_dirty_items(cx: &mut TestAppContext) { + let app_state = init_test(cx); + cx.update(init); + + app_state + .fs + .as_fake() + .insert_tree( + path!("/"), + json!({ + "dir1": { + "a.txt": "content a" + }, + "dir2": { + "b.txt": "content b" + } + }), + ) + .await; + + let project1 = Project::test(app_state.fs.clone(), [path!("/dir1").as_ref()], cx).await; + let window = cx.add_window({ + let project = project1.clone(); + |window, cx| MultiWorkspace::test_new(project, window, cx) + }); + window + .update(cx, |multi_workspace, _, cx| { + multi_workspace.open_sidebar(cx); + }) + .unwrap(); + cx.run_until_parked(); + + let project2 = Project::test(app_state.fs.clone(), [path!("/dir2").as_ref()], cx).await; + let workspace1 = window + .read_with(cx, |multi_workspace, _| multi_workspace.workspace().clone()) + .unwrap(); + let workspace2 = window + .update(cx, |multi_workspace, window, cx| { + multi_workspace.test_add_workspace(project2.clone(), window, cx) + }) + .unwrap(); + + window + .update(cx, |multi_workspace, window, cx| { + multi_workspace.activate(workspace2.clone(), None, window, cx); + multi_workspace.activate(workspace1.clone(), None, window, cx); + }) + .unwrap(); + cx.run_until_parked(); + + let worktree2_id = project2.update(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }); + + let editor2 = window + .update(cx, |_, window, cx| { + workspace2.update(cx, |workspace, cx| { + workspace.open_path((worktree2_id, rel_path("b.txt")), None, true, window, cx) + }) + }) + .unwrap() + .await + .unwrap() + .downcast::() + .unwrap(); + + window + .update(cx, |_, window, cx| { + editor2.update(cx, |editor, cx| { + editor.insert("dirty in non-active workspace", window, cx); + }); + }) + .unwrap(); + cx.run_until_parked(); + + window + .read_with(cx, |multi_workspace, _| { + assert_eq!(multi_workspace.workspace(), &workspace1); + }) + .unwrap(); + + let mut will_restart = cx.expect_restart(); + + cx.dispatch_action(*window, workspace::Reload); + cx.run_until_parked(); + + window + .read_with(cx, |multi_workspace, _| { + assert_eq!( + multi_workspace.workspace(), + &workspace2, + "Non-active workspace should be activated when it has a dirty item" + ); + }) + .unwrap(); + + assert!( + cx.has_pending_prompt(), + "Reload should prompt to save the dirty item in the non-active workspace" + ); + + cx.simulate_prompt_answer("Cancel"); + cx.run_until_parked(); + + assert_eq!( + will_restart.try_recv(), + Ok(None), + "Cancelling the save prompt should abort the restart" + ); + window + .read_with(cx, |multi_workspace, _| { + assert_eq!( + multi_workspace.workspace(), + &workspace1, + "Cancelling reload should restore the originally focused workspace" + ); + }) + .unwrap(); + + cx.dispatch_action(*window, workspace::Reload); + cx.run_until_parked(); + + assert!( + cx.has_pending_prompt(), + "Reload should prompt again for the dirty item" + ); + cx.simulate_prompt_answer("Don't Save"); + cx.run_until_parked(); + + will_restart + .await + .expect("reload should restart the app after the dirty item is resolved"); } #[gpui::test] @@ -7466,14 +7554,11 @@ mod tests { .unwrap(); // Quit. With no dirty items there are no save prompts, so the quit flow - // runs the prepare_to_close loop (which activates every workspace in - // turn to surface prompts) and then flushes serialization. cx.quit() is + // runs the prepare_to_close loop and then flushes serialization. cx.quit() is // a no-op in tests, so the window stays around for inspection. cx.dispatch_action(*window, Quit); cx.run_until_parked(); - // The fix re-activates the originally-focused workspace after the loop, - // so the window must still be focused on dir1, not dir2. window .read_with(cx, |mw, cx| { let active = mw.workspace().read(cx).root_paths(cx); @@ -7531,6 +7616,139 @@ mod tests { .unwrap(); } + #[gpui::test] + async fn test_reload_restores_project_windows_and_tabs(cx: &mut TestAppContext) { + use session::Session; + + let app_state = init_test(cx); + cx.update(init); + + let first_dir = format!("reload-restore-{}", uuid::Uuid::new_v4()); + let second_dir = format!("reload-restore-{}", uuid::Uuid::new_v4()); + app_state + .fs + .as_fake() + .insert_tree( + path!("/"), + json!({ + first_dir.clone(): { + "a.txt": "a", + "b.txt": "b" + }, + second_dir.clone(): { + "c.txt": "c", + "d.txt": "d" + } + }), + ) + .await; + let mut first_dir = PathBuf::from(path!("/")).join(first_dir); + let mut second_dir = PathBuf::from(path!("/")).join(second_dir); + if second_dir < first_dir { + std::mem::swap(&mut first_dir, &mut second_dir); + } + + let session_id = cx.read(|cx| app_state.session.read(cx).id().to_owned()); + let first_window = open_test_project_window_with_tabs( + &app_state, + &first_dir, + &[rel_path("a.txt"), rel_path("b.txt")], + cx, + ) + .await; + let second_window = open_test_project_window_with_tabs( + &app_state, + &second_dir, + &[rel_path("c.txt"), rel_path("d.txt")], + cx, + ) + .await; + + let restart = cx.expect_restart(); + cx.update(workspace::reload); + let (restart_path, restart_arguments) = restart.await.expect("restart was not requested"); + assert_eq!(restart_path, None); + assert!(restart_arguments.is_empty()); + + let database = cx.update(|cx| workspace::WorkspaceDb::global(cx)); + let locations = workspace::last_session_workspace_locations( + &database, + &session_id, + None, + app_state.fs.as_ref(), + ) + .await + .expect("failed to read session workspaces"); + assert_eq!(locations.len(), 2); + + for window in [first_window, second_window] { + window + .update(cx, |_, window, _| window.remove_window()) + .expect("workspace window was closed"); + } + cx.run_until_parked(); + + cx.update(|cx| { + app_state.session.update(cx, |app_session, _cx| { + app_session + .replace_session_for_test(Session::test_with_old_session(session_id.clone())); + }); + }); + + let mut async_cx = cx.to_async(); + crate::restore_or_create_workspace(app_state.clone(), &mut async_cx) + .await + .expect("failed to restore workspaces"); + cx.run_until_parked(); + + let mut restored_tabs = cx.read(|cx| { + cx.windows() + .into_iter() + .filter_map(|window| window.downcast::()) + .map(|window| { + window + .read_with(cx, |multi_workspace, cx| { + let workspace = multi_workspace.workspace().read(cx); + let root_path = workspace + .root_paths(cx) + .into_iter() + .next() + .expect("restored project should have a root path") + .as_ref() + .to_path_buf(); + let tab_paths = workspace + .active_pane() + .read(cx) + .items() + .map(|item| { + item.project_path(cx) + .expect("restored tab should have a project path") + .path + }) + .collect::>(); + (root_path, tab_paths) + }) + .expect("restored workspace window was closed") + }) + .collect::>() + }); + restored_tabs.sort_by(|left, right| left.0.cmp(&right.0)); + + assert_eq!( + restored_tabs, + vec![ + ( + first_dir, + vec![rel_path("a.txt").into(), rel_path("b.txt").into()] + ), + ( + second_dir, + vec![rel_path("c.txt").into(), rel_path("d.txt").into()] + ), + ] + ); + } + #[gpui::test] async fn test_restored_project_groups_survive_workspace_key_change(cx: &mut TestAppContext) { use session::Session; @@ -7826,4 +8044,54 @@ mod tests { "active workspace should contain the remaining project, not be empty: {active_paths:?}" ); } + + async fn open_test_project_window_with_tabs( + app_state: &Arc, + root_path: &Path, + tab_paths: &[&RelPath], + cx: &mut TestAppContext, + ) -> WindowHandle { + let workspace::OpenResult { window, .. } = cx + .update(|cx| { + Workspace::new_local( + vec![root_path.into()], + app_state.clone(), + None, + None, + None, + workspace::OpenMode::Activate, + cx, + ) + }) + .await + .expect("failed to open project workspace"); + + let workspace = window + .read_with(cx, |multi_workspace, _| multi_workspace.workspace().clone()) + .expect("workspace window was closed"); + let worktree_id = workspace.read_with(cx, |workspace, cx| { + workspace + .project() + .read(cx) + .worktrees(cx) + .next() + .expect("project should have a worktree") + .read(cx) + .id() + }); + + for tab_path in tab_paths { + window + .update(cx, |_, window, cx| { + workspace.update(cx, |workspace, cx| { + workspace.open_path((worktree_id, *tab_path), None, true, window, cx) + }) + }) + .expect("workspace window was closed") + .await + .expect("failed to open project tab"); + } + + window + } } diff --git a/crates/zed/src/zed/move_to_applications.rs b/crates/zed/src/zed/move_to_applications.rs index 329e49fb81..79ef27ee38 100644 --- a/crates/zed/src/zed/move_to_applications.rs +++ b/crates/zed/src/zed/move_to_applications.rs @@ -214,7 +214,7 @@ fn should_offer_to_move(app_path: &Path) -> bool { async fn move_to_applications(app_path: &Path, cx: &mut AsyncWindowContext) -> Result<()> { let destination_path = install_destination(app_path).await?; - restart_into(destination_path, cx) + restart_into(destination_path, cx).await } async fn install_destination(app_path: &Path) -> Result { @@ -305,7 +305,18 @@ async fn copy_app_bundle(source: &Path, destination: &Path) -> Result<()> { Ok(()) } -fn restart_into(app_path: PathBuf, cx: &mut AsyncWindowContext) -> Result<()> { +async fn restart_into(app_path: PathBuf, cx: &mut AsyncWindowContext) -> Result<()> { + let workspace_windows = cx + .update(|_window, cx| { + cx.windows() + .into_iter() + .filter_map(|window| window.downcast::()) + .collect::>() + }) + .context("collecting workspace windows before restarting")?; + if !workspace::prepare_windows_to_quit(&workspace_windows, cx).await { + return Ok(()); + } cx.update(|_window, cx| { cx.set_restart_path(app_path); cx.restart();