devcontainer: Fix OpenDevContainer action panic due to double workspace entity lease (#49058)

Closes #49055

**Heads up**: This might be a naïve solution. I ran into the issue after
merging latest main into
https://github.com/zed-industries/zed/pull/48896, and confirming that it
was unrelated to that PR and incoming from upstream.

Agent one-shot the fix, it works and tests pass. But I'm still wrapping
my head around the changes that led to the bug. I figured the breakage
is bad enough (I couldn't open devcontainers at all) to submit a
possibly naïve fix.

## Fix 

Hoists the `find_devcontainer_configs` call out of `new_dev_container`
and into the call site, where we already have a direct `&mut Workspace`
reference that doesn't go through the entity map. The computed configs
are passed into `new_dev_container` as an argument.

## What was happening

After #48800 ("Re-add MultiWorkspace"), `with_active_or_new_workspace`
nests a `Workspace` entity lease inside a `MultiWorkspace` entity lease.
The `OpenDevContainer` handler was also changed from async to sync in
the same PR, so `RemoteServerProjects::new_dev_container` now runs while
`Workspace` is leased. Inside `new_dev_container`, a
`WeakEntity<Workspace>::read_with` call tries to read `Workspace`
through the entity map, finds it already leased, and panics.

Release Notes:

- Fixed a panic when opening the dev container modal via the
`OpenDevContainer` action.

crates/recent_projects/src/recent_projects.rs

@@ -34,6 +34,7 @@ pub use remote_connections::RemoteSettings;
 pub use remote_servers::RemoteServerProjects;
 use settings::{Settings, WorktreeId};
 
+use dev_container::{DevContainerContext, find_devcontainer_configs};
 use ui::{
     ContextMenu, Divider, KeyBinding, ListItem, ListItemSpacing, ListSubHeader, PopoverMenu,
     PopoverMenuHandle, TintColor, Tooltip, prelude::*,
@@ -352,9 +353,20 @@ pub fn init(cx: &mut App) {
             }
 
             let fs = workspace.project().read(cx).fs().clone();
+            let configs = find_devcontainer_configs(workspace, cx);
+            let app_state = workspace.app_state().clone();
+            let dev_container_context = DevContainerContext::from_workspace(workspace, cx);
             let handle = cx.entity().downgrade();
             workspace.toggle_modal(window, cx, |window, cx| {
-                RemoteServerProjects::new_dev_container(fs, window, handle, cx)
+                RemoteServerProjects::new_dev_container(
+                    fs,
+                    configs,
+                    app_state,
+                    dev_container_context,
+                    window,
+                    handle,
+                    cx,
+                )
             });
         });
     });
@@ -1621,6 +1633,121 @@ mod tests {
             .unwrap()
     }
 
+    #[gpui::test]
+    async fn test_open_dev_container_action_with_single_config(cx: &mut TestAppContext) {
+        let app_state = init_test(cx);
+
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                path!("/project"),
+                json!({
+                    ".devcontainer": {
+                        "devcontainer.json": "{}"
+                    },
+                    "src": {
+                        "main.rs": "fn main() {}"
+                    }
+                }),
+            )
+            .await;
+
+        cx.update(|cx| {
+            open_paths(
+                &[PathBuf::from(path!("/project"))],
+                app_state,
+                workspace::OpenOptions::default(),
+                cx,
+            )
+        })
+        .await
+        .unwrap();
+
+        assert_eq!(cx.update(|cx| cx.windows().len()), 1);
+        let multi_workspace = cx.update(|cx| cx.windows()[0].downcast::<MultiWorkspace>().unwrap());
+
+        cx.run_until_parked();
+
+        // This dispatch triggers with_active_or_new_workspace -> MultiWorkspace::update
+        // -> Workspace::update -> toggle_modal -> new_dev_container.
+        // Before the fix, this panicked with "cannot read workspace::Workspace while
+        // it is already being updated" because new_dev_container and open_dev_container
+        // tried to read the Workspace entity through a WeakEntity handle while it was
+        // already leased by the outer update.
+        cx.dispatch_action(*multi_workspace, OpenDevContainer);
+
+        multi_workspace
+            .update(cx, |multi_workspace, _, cx| {
+                let modal = multi_workspace
+                    .workspace()
+                    .read(cx)
+                    .active_modal::<RemoteServerProjects>(cx);
+                assert!(
+                    modal.is_some(),
+                    "Dev container modal should be open after dispatching OpenDevContainer"
+                );
+            })
+            .unwrap();
+    }
+
+    #[gpui::test]
+    async fn test_open_dev_container_action_with_multiple_configs(cx: &mut TestAppContext) {
+        let app_state = init_test(cx);
+
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                path!("/project"),
+                json!({
+                    ".devcontainer": {
+                        "rust": {
+                            "devcontainer.json": "{}"
+                        },
+                        "python": {
+                            "devcontainer.json": "{}"
+                        }
+                    },
+                    "src": {
+                        "main.rs": "fn main() {}"
+                    }
+                }),
+            )
+            .await;
+
+        cx.update(|cx| {
+            open_paths(
+                &[PathBuf::from(path!("/project"))],
+                app_state,
+                workspace::OpenOptions::default(),
+                cx,
+            )
+        })
+        .await
+        .unwrap();
+
+        assert_eq!(cx.update(|cx| cx.windows().len()), 1);
+        let multi_workspace = cx.update(|cx| cx.windows()[0].downcast::<MultiWorkspace>().unwrap());
+
+        cx.run_until_parked();
+
+        cx.dispatch_action(*multi_workspace, OpenDevContainer);
+
+        multi_workspace
+            .update(cx, |multi_workspace, _, cx| {
+                let modal = multi_workspace
+                    .workspace()
+                    .read(cx)
+                    .active_modal::<RemoteServerProjects>(cx);
+                assert!(
+                    modal.is_some(),
+                    "Dev container modal should be open after dispatching OpenDevContainer with multiple configs"
+                );
+            })
+            .unwrap();
+    }
+
     fn init_test(cx: &mut TestAppContext) -> Arc<AppState> {
         cx.update(|cx| {
             let state = AppState::test(cx);

crates/recent_projects/src/remote_servers.rs

@@ -53,7 +53,7 @@ use util::{
     rel_path::RelPath,
 };
 use workspace::{
-    ModalView, MultiWorkspace, OpenLog, OpenOptions, Toast, Workspace,
+    AppState, ModalView, MultiWorkspace, OpenLog, OpenOptions, Toast, Workspace,
     notifications::{DetachAndPromptErr, NotificationId},
     open_remote_project_with_existing_connection,
 };
@@ -258,9 +258,20 @@ impl PickerDelegate for DevContainerPickerDelegate {
             .update(cx, move |modal, cx| {
                 if secondary {
                     modal.edit_in_dev_container_json(selected_config.clone(), window, cx);
-                } else {
-                    modal.open_dev_container(selected_config, window, cx);
+                } else if let Some((app_state, context)) = modal
+                    .workspace
+                    .read_with(cx, |workspace, cx| {
+                        let app_state = workspace.app_state().clone();
+                        let context = DevContainerContext::from_workspace(workspace, cx)?;
+                        Some((app_state, context))
+                    })
+                    .ok()
+                    .flatten()
+                {
+                    modal.open_dev_container(selected_config, app_state, context, window, cx);
                     modal.view_in_progress_dev_container(window, cx);
+                } else {
+                    log::error!("No active project directory for Dev Container");
                 }
             })
             .ok();
@@ -807,14 +818,13 @@ impl RemoteServerProjects {
     /// Used when suggesting dev container connection from toast notification.
     pub fn new_dev_container(
         fs: Arc<dyn Fs>,
+        configs: Vec<DevContainerConfig>,
+        app_state: Arc<AppState>,
+        dev_container_context: Option<DevContainerContext>,
         window: &mut Window,
         workspace: WeakEntity<Workspace>,
         cx: &mut Context<Self>,
     ) -> Self {
-        let configs = workspace
-            .read_with(cx, |workspace, cx| find_devcontainer_configs(workspace, cx))
-            .unwrap_or_default();
-
         let initial_mode = if configs.len() > 1 {
             DevContainerCreationProgress::SelectingConfig
         } else {
@@ -834,10 +844,12 @@ impl RemoteServerProjects {
             let delegate = DevContainerPickerDelegate::new(configs, cx.weak_entity());
             this.dev_container_picker =
                 Some(cx.new(|cx| Picker::uniform_list(delegate, window, cx).modal(false)));
-        } else {
+        } else if let Some(context) = dev_container_context {
             let config = configs.into_iter().next();
-            this.open_dev_container(config, window, cx);
+            this.open_dev_container(config, app_state, context, window, cx);
             this.view_in_progress_dev_container(window, cx);
+        } else {
+            log::error!("No active project directory for Dev Container");
         }
 
         this
@@ -1809,33 +1821,32 @@ impl RemoteServerProjects {
                 CreateRemoteDevContainer::new(DevContainerCreationProgress::SelectingConfig, cx);
             self.mode = Mode::CreateRemoteDevContainer(state);
             cx.notify();
-        } else {
-            let config = configs.into_iter().next();
-            self.open_dev_container(config, window, cx);
-            self.view_in_progress_dev_container(window, cx);
-        }
-    }
-
-    fn open_dev_container(
-        &self,
-        config: Option<DevContainerConfig>,
-        window: &mut Window,
-        cx: &mut Context<Self>,
-    ) {
-        let Some((app_state, context)) = self
+        } else if let Some((app_state, context)) = self
             .workspace
             .read_with(cx, |workspace, cx| {
                 let app_state = workspace.app_state().clone();
                 let context = DevContainerContext::from_workspace(workspace, cx)?;
                 Some((app_state, context))
             })
-            .log_err()
+            .ok()
             .flatten()
-        else {
+        {
+            let config = configs.into_iter().next();
+            self.open_dev_container(config, app_state, context, window, cx);
+            self.view_in_progress_dev_container(window, cx);
+        } else {
             log::error!("No active project directory for Dev Container");
-            return;
-        };
+        }
+    }
 
+    fn open_dev_container(
+        &self,
+        config: Option<DevContainerConfig>,
+        app_state: Arc<AppState>,
+        context: DevContainerContext,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
         let replace_window = window.window_handle().downcast::<MultiWorkspace>();
 
         cx.spawn_in(window, async move |entity, cx| {