Fix panic when immediately closing a window while opening paths (#3092)

Max Brunsfeld created

Fixes this panic that I've been seeing in Slack:


[example](https://zed-industries.slack.com/archives/C04S6T1T7TQ/p1696530575535779)


```
thread 'main' panicked at 'assertion failed: opened_items.len() == project_paths_to_open.len()'
crates/workspace/src/workspace.rs:3628
<backtrace::capture::Backtrace>::create
<backtrace::capture::Backtrace>::new
Zed::init_panic_hook::{closure#0}
std::panicking::rust_panic_with_hook
std::panicking::begin_panic_handler::{{closure}}
std::sys_common::backtrace::__rust_end_short_backtrace
_rust_begin_unwind
core::panicking::panic_fmt
core::panicking::panic
<workspace::Workspace>::new_local::{closure#0}::{closure#0}
```

I believe it was caused by a window being closed immediately, while it
was still loading some paths. There was a mismatch in expectation
between the `workspace::open_items` function (which contains this
assertion), and the `Workspace::load_workspace` method. That later
method can return an empty vector if the workspace handle is dropped
while it is executing.

Release Notes:

- Fixed a crash when closing a Zed window immediately after opening it

Change summary

crates/workspace/src/workspace.rs | 230 +++++++++++++++-----------------
1 file changed, 109 insertions(+), 121 deletions(-)

Detailed changes

crates/workspace/src/workspace.rs 🔗

@@ -79,7 +79,7 @@ use status_bar::StatusBar;
 pub use status_bar::StatusItemView;
 use theme::{Theme, ThemeSettings};
 pub use toolbar::{ToolbarItemLocation, ToolbarItemView};
-use util::{async_iife, ResultExt};
+use util::ResultExt;
 pub use workspace_settings::{AutosaveSetting, GitGutterSetting, WorkspaceSettings};
 
 lazy_static! {
@@ -936,7 +936,8 @@ impl Workspace {
                 app_state,
                 cx,
             )
-            .await;
+            .await
+            .unwrap_or_default();
 
             (workspace, opened_items)
         })
@@ -3400,140 +3401,124 @@ impl Workspace {
         serialized_workspace: SerializedWorkspace,
         paths_to_open: Vec<Option<ProjectPath>>,
         cx: &mut AppContext,
-    ) -> Task<Vec<Option<Result<Box<dyn ItemHandle>, anyhow::Error>>>> {
+    ) -> Task<Result<Vec<Option<Box<dyn ItemHandle>>>>> {
         cx.spawn(|mut cx| async move {
-            let result = async_iife! {{
-                let (project, old_center_pane) =
-                workspace.read_with(&cx, |workspace, _| {
-                    (
-                        workspace.project().clone(),
-                        workspace.last_active_center_pane.clone(),
-                    )
-                })?;
+            let (project, old_center_pane) = workspace.read_with(&cx, |workspace, _| {
+                (
+                    workspace.project().clone(),
+                    workspace.last_active_center_pane.clone(),
+                )
+            })?;
 
-                let mut center_items = None;
-                let mut center_group = None;
-                // Traverse the splits tree and add to things
-                if let Some((group, active_pane, items)) = serialized_workspace
-                        .center_group
-                        .deserialize(&project, serialized_workspace.id, &workspace, &mut cx)
-                        .await {
-                    center_items = Some(items);
-                    center_group = Some((group, active_pane))
-                }
+            let mut center_group = None;
+            let mut center_items = None;
+            // Traverse the splits tree and add to things
+            if let Some((group, active_pane, items)) = serialized_workspace
+                .center_group
+                .deserialize(&project, serialized_workspace.id, &workspace, &mut cx)
+                .await
+            {
+                center_items = Some(items);
+                center_group = Some((group, active_pane))
+            }
 
-                let resulting_list = cx.read(|cx| {
-                    let mut opened_items = center_items
-                        .unwrap_or_default()
-                        .into_iter()
-                        .filter_map(|item| {
-                            let item = item?;
-                            let project_path = item.project_path(cx)?;
-                            Some((project_path, item))
-                        })
-                        .collect::<HashMap<_, _>>();
+            let mut items_by_project_path = cx.read(|cx| {
+                center_items
+                    .unwrap_or_default()
+                    .into_iter()
+                    .filter_map(|item| {
+                        let item = item?;
+                        let project_path = item.project_path(cx)?;
+                        Some((project_path, item))
+                    })
+                    .collect::<HashMap<_, _>>()
+            });
 
-                    paths_to_open
-                        .into_iter()
-                        .map(|path_to_open| {
-                            path_to_open.map(|path_to_open| {
-                                Ok(opened_items.remove(&path_to_open))
-                            })
-                            .transpose()
-                            .map(|item| item.flatten())
-                            .transpose()
-                        })
-                        .collect::<Vec<_>>()
-                });
+            let opened_items = paths_to_open
+                .into_iter()
+                .map(|path_to_open| {
+                    path_to_open
+                        .and_then(|path_to_open| items_by_project_path.remove(&path_to_open))
+                })
+                .collect::<Vec<_>>();
 
-                // Remove old panes from workspace panes list
-                workspace.update(&mut cx, |workspace, cx| {
-                    if let Some((center_group, active_pane)) = center_group {
-                        workspace.remove_panes(workspace.center.root.clone(), cx);
+            // Remove old panes from workspace panes list
+            workspace.update(&mut cx, |workspace, cx| {
+                if let Some((center_group, active_pane)) = center_group {
+                    workspace.remove_panes(workspace.center.root.clone(), cx);
 
-                        // Swap workspace center group
-                        workspace.center = PaneGroup::with_root(center_group);
+                    // Swap workspace center group
+                    workspace.center = PaneGroup::with_root(center_group);
 
-                        // Change the focus to the workspace first so that we retrigger focus in on the pane.
-                        cx.focus_self();
+                    // Change the focus to the workspace first so that we retrigger focus in on the pane.
+                    cx.focus_self();
 
-                        if let Some(active_pane) = active_pane {
-                            cx.focus(&active_pane);
-                        } else {
-                            cx.focus(workspace.panes.last().unwrap());
-                        }
+                    if let Some(active_pane) = active_pane {
+                        cx.focus(&active_pane);
                     } else {
-                        let old_center_handle = old_center_pane.and_then(|weak| weak.upgrade(cx));
-                        if let Some(old_center_handle) = old_center_handle {
-                            cx.focus(&old_center_handle)
-                        } else {
-                            cx.focus_self()
-                        }
+                        cx.focus(workspace.panes.last().unwrap());
                     }
+                } else {
+                    let old_center_handle = old_center_pane.and_then(|weak| weak.upgrade(cx));
+                    if let Some(old_center_handle) = old_center_handle {
+                        cx.focus(&old_center_handle)
+                    } else {
+                        cx.focus_self()
+                    }
+                }
 
-                    let docks = serialized_workspace.docks;
-                    workspace.left_dock.update(cx, |dock, cx| {
-                        dock.set_open(docks.left.visible, cx);
-                        if let Some(active_panel) = docks.left.active_panel {
-                            if let Some(ix) = dock.panel_index_for_ui_name(&active_panel, cx) {
-                                dock.activate_panel(ix, cx);
-                            }
-                        }
-                                dock.active_panel()
-                                    .map(|panel| {
-                                        panel.set_zoomed(docks.left.zoom, cx)
-                                    });
-                                if docks.left.visible && docks.left.zoom {
-                                    cx.focus_self()
-                                }
-                    });
-                    // TODO: I think the bug is that setting zoom or active undoes the bottom zoom or something
-                    workspace.right_dock.update(cx, |dock, cx| {
-                        dock.set_open(docks.right.visible, cx);
-                        if let Some(active_panel) = docks.right.active_panel {
-                            if let Some(ix) = dock.panel_index_for_ui_name(&active_panel, cx) {
-                                dock.activate_panel(ix, cx);
-
-                            }
+                let docks = serialized_workspace.docks;
+                workspace.left_dock.update(cx, |dock, cx| {
+                    dock.set_open(docks.left.visible, cx);
+                    if let Some(active_panel) = docks.left.active_panel {
+                        if let Some(ix) = dock.panel_index_for_ui_name(&active_panel, cx) {
+                            dock.activate_panel(ix, cx);
                         }
-                                dock.active_panel()
-                                    .map(|panel| {
-                                        panel.set_zoomed(docks.right.zoom, cx)
-                                    });
-
-                                if docks.right.visible && docks.right.zoom {
-                                    cx.focus_self()
-                                }
-                    });
-                    workspace.bottom_dock.update(cx, |dock, cx| {
-                        dock.set_open(docks.bottom.visible, cx);
-                        if let Some(active_panel) = docks.bottom.active_panel {
-                            if let Some(ix) = dock.panel_index_for_ui_name(&active_panel, cx) {
-                                dock.activate_panel(ix, cx);
-                            }
+                    }
+                    dock.active_panel()
+                        .map(|panel| panel.set_zoomed(docks.left.zoom, cx));
+                    if docks.left.visible && docks.left.zoom {
+                        cx.focus_self()
+                    }
+                });
+                // TODO: I think the bug is that setting zoom or active undoes the bottom zoom or something
+                workspace.right_dock.update(cx, |dock, cx| {
+                    dock.set_open(docks.right.visible, cx);
+                    if let Some(active_panel) = docks.right.active_panel {
+                        if let Some(ix) = dock.panel_index_for_ui_name(&active_panel, cx) {
+                            dock.activate_panel(ix, cx);
                         }
+                    }
+                    dock.active_panel()
+                        .map(|panel| panel.set_zoomed(docks.right.zoom, cx));
 
-                        dock.active_panel()
-                            .map(|panel| {
-                                panel.set_zoomed(docks.bottom.zoom, cx)
-                            });
-
-                        if docks.bottom.visible && docks.bottom.zoom {
-                            cx.focus_self()
+                    if docks.right.visible && docks.right.zoom {
+                        cx.focus_self()
+                    }
+                });
+                workspace.bottom_dock.update(cx, |dock, cx| {
+                    dock.set_open(docks.bottom.visible, cx);
+                    if let Some(active_panel) = docks.bottom.active_panel {
+                        if let Some(ix) = dock.panel_index_for_ui_name(&active_panel, cx) {
+                            dock.activate_panel(ix, cx);
                         }
-                    });
+                    }
 
+                    dock.active_panel()
+                        .map(|panel| panel.set_zoomed(docks.bottom.zoom, cx));
 
-                    cx.notify();
-                })?;
+                    if docks.bottom.visible && docks.bottom.zoom {
+                        cx.focus_self()
+                    }
+                });
 
-                // Serialize ourself to make sure our timestamps and any pane / item changes are replicated
-                workspace.read_with(&cx, |workspace, cx| workspace.serialize_workspace(cx))?;
+                cx.notify();
+            })?;
 
-                Ok::<_, anyhow::Error>(resulting_list)
-            }};
+            // Serialize ourself to make sure our timestamps and any pane / item changes are replicated
+            workspace.read_with(&cx, |workspace, cx| workspace.serialize_workspace(cx))?;
 
-            result.await.unwrap_or_default()
+            Ok(opened_items)
         })
     }
 
@@ -3607,7 +3592,7 @@ async fn open_items(
     mut project_paths_to_open: Vec<(PathBuf, Option<ProjectPath>)>,
     app_state: Arc<AppState>,
     mut cx: AsyncAppContext,
-) -> Vec<Option<anyhow::Result<Box<dyn ItemHandle>>>> {
+) -> Result<Vec<Option<Result<Box<dyn ItemHandle>>>>> {
     let mut opened_items = Vec::with_capacity(project_paths_to_open.len());
 
     if let Some(serialized_workspace) = serialized_workspace {
@@ -3625,16 +3610,19 @@ async fn open_items(
                     cx,
                 )
             })
-            .await;
+            .await?;
 
         let restored_project_paths = cx.read(|cx| {
             restored_items
                 .iter()
-                .filter_map(|item| item.as_ref()?.as_ref().ok()?.project_path(cx))
+                .filter_map(|item| item.as_ref()?.project_path(cx))
                 .collect::<HashSet<_>>()
         });
 
-        opened_items = restored_items;
+        for restored_item in restored_items {
+            opened_items.push(restored_item.map(Ok));
+        }
+
         project_paths_to_open
             .iter_mut()
             .for_each(|(_, project_path)| {
@@ -3687,7 +3675,7 @@ async fn open_items(
         }
     }
 
-    opened_items
+    Ok(opened_items)
 }
 
 fn notify_of_new_dock(workspace: &WeakViewHandle<Workspace>, cx: &mut AsyncAppContext) {