From 1a491707e355773fddff76ec1a249a8858d045bd Mon Sep 17 00:00:00 2001 From: Oliver Azevedo Barnes Date: Fri, 13 Feb 2026 00:35:18 +0000 Subject: [PATCH] devcontainer: Fix OpenDevContainer action panic due to double workspace entity lease (#49058) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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::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 | 129 +++++++++++++++++- crates/recent_projects/src/remote_servers.rs | 61 +++++---- 2 files changed, 164 insertions(+), 26 deletions(-) diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 0a0b2c4b79f465ed4331410186e35965613d498b..e69bde6335c84d0e7b1332a1b6f14abcd5fafdab 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/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::().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::(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::().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::(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 { cx.update(|cx| { let state = AppState::test(cx); diff --git a/crates/recent_projects/src/remote_servers.rs b/crates/recent_projects/src/remote_servers.rs index b49d30dc23212c2925fa0cf4b5700890c32f5dba..6e2d9ce226c8f15552963bf457e622141f87cec7 100644 --- a/crates/recent_projects/src/remote_servers.rs +++ b/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, + configs: Vec, + app_state: Arc, + dev_container_context: Option, window: &mut Window, workspace: WeakEntity, cx: &mut Context, ) -> 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 { + } 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)) + }) + .ok() + .flatten() + { let config = configs.into_iter().next(); - self.open_dev_container(config, window, cx); + 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"); } } fn open_dev_container( &self, config: Option, + app_state: Arc, + context: DevContainerContext, window: &mut Window, cx: &mut Context, ) { - 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() - .flatten() - else { - log::error!("No active project directory for Dev Container"); - return; - }; - let replace_window = window.window_handle().downcast::(); cx.spawn_in(window, async move |entity, cx| {