assistant panel: Fix panic when opening panel with zed.dev provider (#15538)

Thorsten Ball and Bennet created 1 year ago

There was/is some race condition that gets triggered only with the
zed.dev provider when opening the provider that would cause a
double-borrow on workspace.

This PR fixes the issue by cloning the workspace weakly. Turns out we
can go very far with just the weak reference.

We're still a bit unsure why exactly the race condition happened, since
it's hard to reproduce, but we're working on configuration
view/management in #15490 anyway.

Release Notes:

- N/A

Co-authored-by: Bennet <[e-mail]>

Change summary

crates/assistant/src/assistant_panel.rs     | 32 +++++++---------------
crates/language_model/src/provider/cloud.rs |  6 ++-
2 files changed, 14 insertions(+), 24 deletions(-)

Detailed changes

crates/assistant/src/assistant_panel.rs

@@ -545,7 +545,7 @@ impl AssistantPanel {
             let mut editor = ContextEditor::for_context(
                 context,
                 self.fs.clone(),
-                workspace.clone(),
+                self.workspace.clone(),
                 self.project.clone(),
                 lsp_adapter_delegate,
                 assistant_panel,
@@ -789,12 +789,9 @@ impl AssistantPanel {
                 let context = task.await?;
 
                 this.update(&mut cx, |this, cx| {
-                    let Some(workspace) = this.workspace.upgrade() else {
-                        return Ok(());
-                    };
-                    let lsp_adapter_delegate = workspace.update(cx, |workspace, cx| {
-                        make_lsp_adapter_delegate(workspace.project(), cx).log_err()
-                    });
+                    let workspace = this.workspace.clone();
+                    let project = this.project.clone();
+                    let lsp_adapter_delegate = make_lsp_adapter_delegate(&project, cx).log_err();
 
                     let fs = this.fs.clone();
                     let project = this.project.clone();
@@ -804,7 +801,7 @@ impl AssistantPanel {
                         let mut editor = ContextEditor::for_context(
                             context,
                             fs,
-                            workspace.clone(),
+                            workspace,
                             project,
                             lsp_adapter_delegate,
                             weak_assistant_panel,
@@ -826,17 +823,14 @@ impl AssistantPanel {
             None
         } else {
             let context = self.context_store.update(cx, |store, cx| store.create(cx));
-            let workspace = self.workspace.upgrade()?;
-            let lsp_adapter_delegate = workspace.update(cx, |workspace, cx| {
-                make_lsp_adapter_delegate(workspace.project(), cx).log_err()
-            });
+            let lsp_adapter_delegate = make_lsp_adapter_delegate(&self.project, cx).log_err();
 
             let assistant_panel = cx.view().downgrade();
             let editor = cx.new_view(|cx| {
                 let mut editor = ContextEditor::for_context(
                     context,
                     self.fs.clone(),
-                    workspace.clone(),
+                    self.workspace.clone(),
                     self.project.clone(),
                     lsp_adapter_delegate,
                     assistant_panel,
@@ -1004,9 +998,6 @@ impl AssistantPanel {
             let context = context.await?;
             let assistant_panel = this.clone();
             this.update(&mut cx, |this, cx| {
-                let workspace = workspace
-                    .upgrade()
-                    .ok_or_else(|| anyhow!("workspace dropped"))?;
                 let editor = cx.new_view(|cx| {
                     ContextEditor::for_context(
                         context,
@@ -1060,9 +1051,6 @@ impl AssistantPanel {
             let context = context.await?;
             let assistant_panel = this.clone();
             this.update(&mut cx, |this, cx| {
-                let workspace = workspace
-                    .upgrade()
-                    .ok_or_else(|| anyhow!("workspace dropped"))?;
                 let editor = cx.new_view(|cx| {
                     ContextEditor::for_context(
                         context,
@@ -1276,7 +1264,7 @@ impl ContextEditor {
     fn for_context(
         context: Model<Context>,
         fs: Arc<dyn Fs>,
-        workspace: View<Workspace>,
+        workspace: WeakView<Workspace>,
         project: Model<Project>,
         lsp_adapter_delegate: Option<Arc<dyn LspAdapterDelegate>>,
         assistant_panel: WeakView<AssistantPanel>,
@@ -1284,7 +1272,7 @@ impl ContextEditor {
     ) -> Self {
         let completion_provider = SlashCommandCompletionProvider::new(
             Some(cx.view().downgrade()),
-            Some(workspace.downgrade()),
+            Some(workspace.clone()),
         );
 
         let editor = cx.new_view(|cx| {
@@ -1318,7 +1306,7 @@ impl ContextEditor {
             scroll_position: None,
             remote_id: None,
             fs,
-            workspace: workspace.downgrade(),
+            workspace,
             project,
             pending_slash_command_creases: HashMap::default(),
             pending_slash_command_blocks: HashMap::default(),

crates/language_model/src/provider/cloud.rs

@@ -84,8 +84,10 @@ impl CloudLanguageModelProvider {
             while let Some(status) = status_rx.next().await {
                 if let Some(this) = state_ref.upgrade() {
                     _ = this.update(&mut cx, |this, cx| {
-                        this.status = status;
-                        cx.notify();
+                        if this.status != status {
+                            this.status = status;
+                            cx.notify();
+                        }
                     });
                 } else {
                     break;