Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lsp: opening multiple-files in a short period causes a deadlock #10603

Closed
kitsonk opened this issue May 12, 2021 · 1 comment
Closed

lsp: opening multiple-files in a short period causes a deadlock #10603

kitsonk opened this issue May 12, 2021 · 1 comment
Assignees
Labels
bug Something isn't working correctly high priority lsp related to the language server

Comments

@kitsonk
Copy link
Contributor

kitsonk commented May 12, 2021

When the lsp receives several textDocument/didOpen messages in quick succession it can cause a dead-lock, as the lsp is querying the client for configuration information. This can occur if a user is doing a find/replace across multiple files. This only affects Deno 1.10.0.

@kitsonk kitsonk added bug Something isn't working correctly high priority lsp related to the language server labels May 12, 2021
@kitsonk kitsonk self-assigned this May 12, 2021
@kitsonk
Copy link
Contributor Author

kitsonk commented May 12, 2021

@ry this diff works the bug but disables the workspace folder config:

diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs
index bc41fd02a..441b2cc99 100644
--- a/cli/lsp/config.rs
+++ b/cli/lsp/config.rs
@@ -148,6 +148,7 @@ pub struct Config {
 }
 
 impl Config {
+  #[allow(unused)]
   pub fn contains(&self, specifier: &ModuleSpecifier) -> bool {
     self.specifier_settings.contains_key(specifier)
   }
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index 5ed6f693c..2f090938c 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -594,25 +594,26 @@ impl Inner {
     let specifier = self.url_map.normalize_url(&params.text_document.uri);
 
     // we only query the individual resource file if the client supports it
-    if self.config.client_capabilities.workspace_configuration
-      && !self.config.contains(&specifier)
-    {
-      if let Ok(value) = self
-        .client
-        .configuration(vec![ConfigurationItem {
-          scope_uri: Some(params.text_document.uri.clone()),
-          section: Some(SETTINGS_SECTION.to_string()),
-        }])
-        .await
-      {
-        if let Err(err) = self
-          .config
-          .update_specifier(specifier.clone(), value[0].clone())
-        {
-          warn!("Error updating specifier configuration: {}", err);
-        }
-      }
-    }
+    // TODO(@kitsonk) workaround https://github.com/denoland/deno/issues/10603
+    // if self.config.client_capabilities.workspace_configuration
+    //   && !self.config.contains(&specifier)
+    // {
+    //   if let Ok(value) = self
+    //     .client
+    //     .configuration(vec![ConfigurationItem {
+    //       scope_uri: Some(params.text_document.uri.clone()),
+    //       section: Some(SETTINGS_SECTION.to_string()),
+    //     }])
+    //     .await
+    //   {
+    //     if let Err(err) = self
+    //       .config
+    //       .update_specifier(specifier.clone(), value[0].clone())
+    //     {
+    //       warn!("Error updating specifier configuration: {}", err);
+    //     }
+    //   }
+    // }
 
     if params.text_document.uri.scheme() == "deno" {
       // we can ignore virtual text documents opening, as they don't need to

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly high priority lsp related to the language server
Projects
None yet
Development

No branches or pull requests

1 participant