fix(lsp): fix deadlocks, use one big mutex (#9271)

The LSP code had numerous places where competing threads could take out
out locks in different orders, making it very prone to deadlocks.
This commit sidesteps the entire issue by switching to a single lock.

The above is a little white lie: the Sources struct still uses a mutex
internally to avoid having to boil the ocean (because being honest about
what it does involves changing all its methods to `&mut self` but that
ripples out extensively...) I'll save that one for another day.

cli/lsp/diagnostics.rs

@@ -9,7 +9,6 @@ use super::tsc;
 use crate::diagnostics;
 use crate::media_type::MediaType;
 
-use deno_core::error::custom_error;
 use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_core::ModuleSpecifier;
@@ -265,17 +264,13 @@ pub async fn generate_ts_diagnostics(
 }
 
 pub async fn generate_dependency_diagnostics(
-  state_snapshot: StateSnapshot,
+  mut state_snapshot: StateSnapshot,
   diagnostic_collection: DiagnosticCollection,
 ) -> Result<DiagnosticVec, AnyError> {
   tokio::task::spawn_blocking(move || {
     let mut diagnostics = Vec::new();
 
-    let mut sources = if let Ok(sources) = state_snapshot.sources.lock() {
-      sources
-    } else {
-      return Err(custom_error("Deadlock", "deadlock locking sources"));
-    };
+    let sources = &mut state_snapshot.sources;
     for specifier in state_snapshot.documents.open_specifiers() {
       let version = state_snapshot.documents.version(specifier);
       let current_version = diagnostic_collection.get_version(specifier);

cli/lsp/language_server.rs

@@ -18,7 +18,6 @@ use std::collections::HashMap;
 use std::env;
 use std::path::PathBuf;
 use std::sync::Arc;
-use std::sync::Mutex;
 use tokio::fs;
 
 use crate::deno_dir;
@@ -42,35 +41,44 @@ use super::tsc::TsServer;
 use super::utils;
 
 #[derive(Debug, Clone)]
-pub struct LanguageServer {
-  assets: Arc<Mutex<HashMap<ModuleSpecifier, Option<AssetDocument>>>>,
-  client: Client,
-  ts_server: TsServer,
-  config: Arc<Mutex<Config>>,
-  documents: Arc<Mutex<DocumentCache>>,
-  sources: Arc<Mutex<Sources>>,
-  diagnostics: Arc<Mutex<DiagnosticCollection>>,
-  maybe_config_uri: Arc<Mutex<Option<Url>>>,
-  maybe_import_map: Arc<Mutex<Option<ImportMap>>>,
-  maybe_import_map_uri: Arc<Mutex<Option<Url>>>,
-}
+pub struct LanguageServer(Arc<tokio::sync::Mutex<Inner>>);
 
 #[derive(Debug, Clone, Default)]
 pub struct StateSnapshot {
-  pub assets: Arc<Mutex<HashMap<ModuleSpecifier, Option<AssetDocument>>>>,
+  pub assets: HashMap<ModuleSpecifier, Option<AssetDocument>>,
   pub documents: DocumentCache,
-  pub sources: Arc<Mutex<Sources>>,
+  pub sources: Sources,
+}
+
+#[derive(Debug)]
+struct Inner {
+  assets: HashMap<ModuleSpecifier, Option<AssetDocument>>,
+  client: Client,
+  ts_server: TsServer,
+  config: Config,
+  documents: DocumentCache,
+  sources: Sources,
+  diagnostics: DiagnosticCollection,
+  maybe_config_uri: Option<Url>,
+  maybe_import_map: Option<ImportMap>,
+  maybe_import_map_uri: Option<Url>,
 }
 
 impl LanguageServer {
   pub fn new(client: Client) -> Self {
+    Self(Arc::new(tokio::sync::Mutex::new(Inner::new(client))))
+  }
+}
+
+impl Inner {
+  fn new(client: Client) -> Self {
     let maybe_custom_root = env::var("DENO_DIR").map(String::into).ok();
     let dir = deno_dir::DenoDir::new(maybe_custom_root)
       .expect("could not access DENO_DIR");
     let location = dir.root.join("deps");
-    let sources = Arc::new(Mutex::new(Sources::new(&location)));
+    let sources = Sources::new(&location);
 
-    LanguageServer {
+    Self {
       assets: Default::default(),
       client,
       ts_server: TsServer::new(),
@@ -85,20 +93,18 @@ impl LanguageServer {
   }
 
   fn enabled(&self) -> bool {
-    let config = self.config.lock().unwrap();
-    config.settings.enable
+    self.config.settings.enable
   }
 
   /// Searches assets, open documents and external sources for a line_index,
   /// which might be performed asynchronously, hydrating in memory caches for
   /// subsequent requests.
-  pub async fn get_line_index(
+  async fn get_line_index(
     &self,
     specifier: ModuleSpecifier,
   ) -> Result<LineIndex, AnyError> {
     if specifier.as_url().scheme() == "asset" {
-      let maybe_asset =
-        { self.assets.lock().unwrap().get(&specifier).cloned() };
+      let maybe_asset = self.assets.get(&specifier).cloned();
       if let Some(maybe_asset) = maybe_asset {
         if let Some(asset) = maybe_asset {
           Ok(asset.line_index)
@@ -106,22 +112,19 @@ impl LanguageServer {
           Err(anyhow!("asset is missing: {}", specifier))
         }
       } else {
-        let state_snapshot = self.snapshot();
+        let mut state_snapshot = self.snapshot();
         if let Some(asset) =
-          tsc::get_asset(&specifier, &self.ts_server, &state_snapshot).await?
+          tsc::get_asset(&specifier, &self.ts_server, &mut state_snapshot)
+            .await?
         {
           Ok(asset.line_index)
         } else {
           Err(anyhow!("asset is missing: {}", specifier))
         }
       }
-    } else if let Some(line_index) =
-      self.documents.lock().unwrap().line_index(&specifier)
-    {
+    } else if let Some(line_index) = self.documents.line_index(&specifier) {
       Ok(line_index)
-    } else if let Some(line_index) =
-      self.sources.lock().unwrap().get_line_index(&specifier)
-    {
+    } else if let Some(line_index) = self.sources.get_line_index(&specifier) {
       Ok(line_index)
     } else {
       Err(anyhow!("Unable to find line index for: {}", specifier))
@@ -130,124 +133,129 @@ impl LanguageServer {
 
   /// Only searches already cached assets and documents for a line index.  If
   /// the line index cannot be found, `None` is returned.
-  pub fn get_line_index_sync(
+  fn get_line_index_sync(
     &self,
     specifier: &ModuleSpecifier,
   ) -> Option<LineIndex> {
     if specifier.as_url().scheme() == "asset" {
-      if let Some(Some(asset)) = self.assets.lock().unwrap().get(specifier) {
+      if let Some(Some(asset)) = self.assets.get(specifier) {
         Some(asset.line_index.clone())
       } else {
         None
       }
     } else {
-      let documents = self.documents.lock().unwrap();
+      let documents = &self.documents;
       if documents.contains(specifier) {
         documents.line_index(specifier)
       } else {
-        self.sources.lock().unwrap().get_line_index(specifier)
+        self.sources.get_line_index(specifier)
       }
     }
   }
 
-  async fn prepare_diagnostics(&self) -> Result<(), AnyError> {
+  async fn prepare_diagnostics(&mut self) -> Result<(), AnyError> {
     let (enabled, lint_enabled) = {
-      let config = self.config.lock().unwrap();
+      let config = &self.config;
       (config.settings.enable, config.settings.lint)
     };
 
     let lint = async {
-      let mut disturbed = false;
+      let mut diagnostics = None;
       if lint_enabled {
-        let diagnostic_collection = self.diagnostics.lock().unwrap().clone();
-        let diagnostics = diagnostics::generate_lint_diagnostics(
-          self.snapshot(),
-          diagnostic_collection,
-        )
-        .await;
-        disturbed = !diagnostics.is_empty();
-        {
-          let mut diagnostics_collection = self.diagnostics.lock().unwrap();
-          for (specifier, version, diagnostics) in diagnostics {
-            diagnostics_collection.set(
-              specifier,
-              DiagnosticSource::Lint,
-              version,
-              diagnostics,
-            );
-          }
-        }
+        diagnostics = Some(
+          diagnostics::generate_lint_diagnostics(
+            self.snapshot(),
+            self.diagnostics.clone(),
+          )
+          .await,
+        );
       };
-      Ok::<bool, AnyError>(disturbed)
+      Ok::<_, AnyError>(diagnostics)
     };
 
     let ts = async {
-      let mut disturbed = false;
+      let mut diagnostics = None;
       if enabled {
-        let diagnostics_collection = self.diagnostics.lock().unwrap().clone();
-        let diagnostics = diagnostics::generate_ts_diagnostics(
-          self.snapshot(),
-          diagnostics_collection,
-          &self.ts_server,
-        )
-        .await?;
-        disturbed = !diagnostics.is_empty();
-        {
-          let mut diagnostics_collection = self.diagnostics.lock().unwrap();
-          for (specifier, version, diagnostics) in diagnostics {
-            diagnostics_collection.set(
-              specifier,
-              DiagnosticSource::TypeScript,
-              version,
-              diagnostics,
-            );
-          }
-        }
+        diagnostics = Some(
+          diagnostics::generate_ts_diagnostics(
+            self.snapshot(),
+            self.diagnostics.clone(),
+            &self.ts_server,
+          )
+          .await?,
+        );
       };
-      Ok::<bool, AnyError>(disturbed)
+      Ok::<_, AnyError>(diagnostics)
     };
 
     let deps = async {
-      let mut disturbed = false;
+      let mut diagnostics = None;
       if enabled {
-        let diagnostics_collection = self.diagnostics.lock().unwrap().clone();
-        let diagnostics = diagnostics::generate_dependency_diagnostics(
-          self.snapshot(),
-          diagnostics_collection,
-        )
-        .await?;
-        disturbed = !diagnostics.is_empty();
-        {
-          let mut diagnostics_collection = self.diagnostics.lock().unwrap();
-          for (specifier, version, diagnostics) in diagnostics {
-            diagnostics_collection.set(
-              specifier,
-              DiagnosticSource::Deno,
-              version,
-              diagnostics,
-            );
-          }
-        }
+        diagnostics = Some(
+          diagnostics::generate_dependency_diagnostics(
+            self.snapshot(),
+            self.diagnostics.clone(),
+          )
+          .await?,
+        );
       };
-      Ok::<bool, AnyError>(disturbed)
+      Ok::<_, AnyError>(diagnostics)
     };
 
     let (lint_res, ts_res, deps_res) = tokio::join!(lint, ts, deps);
-    if lint_res? || ts_res? || deps_res? {
+    let mut disturbed = false;
+
+    if let Some(diagnostics) = lint_res? {
+      for (specifier, version, diagnostics) in diagnostics {
+        self.diagnostics.set(
+          specifier,
+          DiagnosticSource::Lint,
+          version,
+          diagnostics,
+        );
+        disturbed = true;
+      }
+    }
+
+    if let Some(diagnostics) = ts_res? {
+      for (specifier, version, diagnostics) in diagnostics {
+        self.diagnostics.set(
+          specifier,
+          DiagnosticSource::TypeScript,
+          version,
+          diagnostics,
+        );
+        disturbed = true;
+      }
+    }
+
+    if let Some(diagnostics) = deps_res? {
+      for (specifier, version, diagnostics) in diagnostics {
+        self.diagnostics.set(
+          specifier,
+          DiagnosticSource::Deno,
+          version,
+          diagnostics,
+        );
+        disturbed = true;
+      }
+    }
+
+    if disturbed {
       self.publish_diagnostics().await?;
     }
 
     Ok(())
   }
 
-  async fn publish_diagnostics(&self) -> Result<(), AnyError> {
+  async fn publish_diagnostics(&mut self) -> Result<(), AnyError> {
     let (maybe_changes, diagnostics_collection) = {
-      let mut diagnostics_collection = self.diagnostics.lock().unwrap();
+      let diagnostics_collection = &mut self.diagnostics;
       let maybe_changes = diagnostics_collection.take_changes();
       (maybe_changes, diagnostics_collection.clone())
     };
     if let Some(diagnostic_changes) = maybe_changes {
-      let settings = self.config.lock().unwrap().settings.clone();
+      let settings = self.config.settings.clone();
       for specifier in diagnostic_changes {
         // TODO(@kitsonk) not totally happy with the way we collect and store
         // different types of diagnostics and offer them up to the client, we
@@ -275,7 +283,7 @@ impl LanguageServer {
           );
         }
         let uri = specifier.as_url().clone();
-        let version = self.documents.lock().unwrap().version(&specifier);
+        let version = self.documents.version(&specifier);
         self
           .client
           .publish_diagnostics(uri, diagnostics, version)
@@ -286,17 +294,17 @@ impl LanguageServer {
     Ok(())
   }
 
-  pub fn snapshot(&self) -> StateSnapshot {
+  fn snapshot(&self) -> StateSnapshot {
     StateSnapshot {
       assets: self.assets.clone(),
-      documents: self.documents.lock().unwrap().clone(),
+      documents: self.documents.clone(),
       sources: self.sources.clone(),
     }
   }
 
-  pub async fn update_import_map(&self) -> Result<(), AnyError> {
+  async fn update_import_map(&mut self) -> Result<(), AnyError> {
     let (maybe_import_map, maybe_root_uri) = {
-      let config = self.config.lock().unwrap();
+      let config = &self.config;
       (config.settings.import_map.clone(), config.root_uri.clone())
     };
     if let Some(import_map_str) = &maybe_import_map {
@@ -331,15 +339,15 @@ impl LanguageServer {
         })?;
       let import_map =
         ImportMap::from_json(&import_map_url.to_string(), &import_map_json)?;
-      *self.maybe_import_map_uri.lock().unwrap() = Some(import_map_url);
-      *self.maybe_import_map.lock().unwrap() = Some(import_map);
+      self.maybe_import_map_uri = Some(import_map_url);
+      self.maybe_import_map = Some(import_map);
     } else {
-      *self.maybe_import_map.lock().unwrap() = None;
+      self.maybe_import_map = None;
     }
     Ok(())
   }
 
-  async fn update_tsconfig(&self) -> Result<(), AnyError> {
+  async fn update_tsconfig(&mut self) -> Result<(), AnyError> {
     let mut tsconfig = TsConfig::new(json!({
       "allowJs": true,
       "experimentalDecorators": true,
@@ -351,7 +359,7 @@ impl LanguageServer {
       "target": "esnext",
     }));
     let (maybe_config, maybe_root_uri) = {
-      let config = self.config.lock().unwrap();
+      let config = &self.config;
       if config.settings.unstable {
         let unstable_libs = json!({
           "lib": ["deno.ns", "deno.window", "deno.unstable"]
@@ -394,7 +402,7 @@ impl LanguageServer {
       let (value, maybe_ignored_options) =
         parse_config(&config_text, &config_path)?;
       tsconfig.merge(&value);
-      *self.maybe_config_uri.lock().unwrap() = Some(config_url);
+      self.maybe_config_uri = Some(config_url);
       if let Some(ignored_options) = maybe_ignored_options {
         // TODO(@kitsonk) turn these into diagnostics that can be sent to the
         // client
@@ -409,10 +417,10 @@ impl LanguageServer {
   }
 }
 
-#[lspower::async_trait]
-impl lspower::LanguageServer for LanguageServer {
+// lspower::LanguageServer methods. This file's LanguageServer delegates to us.
+impl Inner {
   async fn initialize(
-    &self,
+    &mut self,
     params: InitializeParams,
   ) -> LspResult<InitializeResult> {
     info!("Starting Deno language server...");
@@ -441,7 +449,7 @@ impl lspower::LanguageServer for LanguageServer {
     }
 
     {
-      let mut config = self.config.lock().unwrap();
+      let config = &mut self.config;
       config.root_uri = params.root_uri;
       if let Some(value) = params.initialization_options {
         config.update(value)?;
@@ -459,7 +467,7 @@ impl lspower::LanguageServer for LanguageServer {
     })
   }
 
-  async fn initialized(&self, _: InitializedParams) {
+  async fn initialized(&mut self, _: InitializedParams) {
     // Check to see if we need to setup the import map
     if let Err(err) = self.update_import_map().await {
       self
@@ -470,8 +478,6 @@ impl lspower::LanguageServer for LanguageServer {
 
     if self
       .config
-      .lock()
-      .unwrap()
       .client_capabilities
       .workspace_did_change_watched_files
     {
@@ -506,7 +512,7 @@ impl lspower::LanguageServer for LanguageServer {
     Ok(())
   }
 
-  async fn did_open(&self, params: DidOpenTextDocumentParams) {
+  async fn did_open(&mut self, params: DidOpenTextDocumentParams) {
     if params.text_document.uri.scheme() == "deno" {
       // we can ignore virtual text documents opening, as they don't need to
       // be tracked in memory, as they are static assets that won't change
@@ -514,16 +520,14 @@ impl lspower::LanguageServer for LanguageServer {
       return;
     }
     let specifier = utils::normalize_url(params.text_document.uri);
-    self.documents.lock().unwrap().open(
+    self.documents.open(
       specifier.clone(),
       params.text_document.version,
       params.text_document.text,
     );
     if let Err(err) = self
       .documents
-      .lock()
-      .unwrap()
-      .analyze_dependencies(&specifier, &self.maybe_import_map.lock().unwrap())
+      .analyze_dependencies(&specifier, &self.maybe_import_map)
     {
       error!("{}", err);
     }
@@ -534,9 +538,9 @@ impl lspower::LanguageServer for LanguageServer {
     }
   }
 
-  async fn did_change(&self, params: DidChangeTextDocumentParams) {
+  async fn did_change(&mut self, params: DidChangeTextDocumentParams) {
     let specifier = utils::normalize_url(params.text_document.uri);
-    if let Err(err) = self.documents.lock().unwrap().change(
+    if let Err(err) = self.documents.change(
       &specifier,
       params.text_document.version,
       params.content_changes,
@@ -545,9 +549,7 @@ impl lspower::LanguageServer for LanguageServer {
     }
     if let Err(err) = self
       .documents
-      .lock()
-      .unwrap()
-      .analyze_dependencies(&specifier, &self.maybe_import_map.lock().unwrap())
+      .analyze_dependencies(&specifier, &self.maybe_import_map)
     {
       error!("{}", err);
     }
@@ -558,7 +560,7 @@ impl lspower::LanguageServer for LanguageServer {
     }
   }
 
-  async fn did_close(&self, params: DidCloseTextDocumentParams) {
+  async fn did_close(&mut self, params: DidCloseTextDocumentParams) {
     if params.text_document.uri.scheme() == "deno" {
       // we can ignore virtual text documents opening, as they don't need to
       // be tracked in memory, as they are static assets that won't change
@@ -566,7 +568,7 @@ impl lspower::LanguageServer for LanguageServer {
       return;
     }
     let specifier = utils::normalize_url(params.text_document.uri);
-    self.documents.lock().unwrap().close(&specifier);
+    self.documents.close(&specifier);
 
     // TODO(@kitsonk): how to better lazily do this?
     if let Err(err) = self.prepare_diagnostics().await {
@@ -579,16 +581,10 @@ impl lspower::LanguageServer for LanguageServer {
   }
 
   async fn did_change_configuration(
-    &self,
+    &mut self,
     params: DidChangeConfigurationParams,
   ) {
-    let config = if self
-      .config
-      .lock()
-      .unwrap()
-      .client_capabilities
-      .workspace_configuration
-    {
+    let config = if self.config.client_capabilities.workspace_configuration {
       self
         .client
         .configuration(vec![ConfigurationItem {
@@ -611,7 +607,7 @@ impl lspower::LanguageServer for LanguageServer {
     };
 
     if let Some(config) = config {
-      if let Err(err) = self.config.lock().unwrap().update(config) {
+      if let Err(err) = self.config.update(config) {
         error!("failed to update settings: {}", err);
       }
       if let Err(err) = self.update_import_map().await {
@@ -632,14 +628,12 @@ impl lspower::LanguageServer for LanguageServer {
   }
 
   async fn did_change_watched_files(
-    &self,
+    &mut self,
     params: DidChangeWatchedFilesParams,
   ) {
     // if the current import map has changed, we need to reload it
-    let maybe_import_map_uri =
-      self.maybe_import_map_uri.lock().unwrap().clone();
-    if let Some(import_map_uri) = maybe_import_map_uri {
-      if params.changes.iter().any(|fe| import_map_uri == fe.uri) {
+    if let Some(import_map_uri) = &self.maybe_import_map_uri {
+      if params.changes.iter().any(|fe| *import_map_uri == fe.uri) {
         if let Err(err) = self.update_import_map().await {
           self
             .client
@@ -649,9 +643,8 @@ impl lspower::LanguageServer for LanguageServer {
       }
     }
     // if the current tsconfig has changed, we need to reload it
-    let maybe_config_uri = self.maybe_config_uri.lock().unwrap().clone();
-    if let Some(config_uri) = maybe_config_uri {
-      if params.changes.iter().any(|fe| config_uri == fe.uri) {
+    if let Some(config_uri) = &self.maybe_config_uri {
+      if params.changes.iter().any(|fe| *config_uri == fe.uri) {
         if let Err(err) = self.update_tsconfig().await {
           self
             .client
@@ -669,8 +662,6 @@ impl lspower::LanguageServer for LanguageServer {
     let specifier = utils::normalize_url(params.text_document.uri.clone());
     let file_text = self
       .documents
-      .lock()
-      .unwrap()
       .content(&specifier)
       .map_err(|_| {
         LspError::invalid_params(
@@ -793,7 +784,7 @@ impl lspower::LanguageServer for LanguageServer {
   }
 
   async fn references(
-    &self,
+    &mut self,
     params: ReferenceParams,
   ) -> LspResult<Option<Vec<Location>>> {
     if !self.enabled() {
@@ -842,7 +833,7 @@ impl lspower::LanguageServer for LanguageServer {
   }
 
   async fn goto_definition(
-    &self,
+    &mut self,
     params: GotoDefinitionParams,
   ) -> LspResult<Option<GotoDefinitionResponse>> {
     if !self.enabled() {
@@ -1038,7 +1029,7 @@ impl lspower::LanguageServer for LanguageServer {
         .into_workspace_edit(
           &params.new_name,
           |s| self.get_line_index(s),
-          |s| self.documents.lock().unwrap().version(&s),
+          |s| self.documents.version(&s),
         )
         .await
         .map_err(|err| {
@@ -1052,7 +1043,7 @@ impl lspower::LanguageServer for LanguageServer {
   }
 
   async fn request_else(
-    &self,
+    &mut self,
     method: &str,
     params: Option<Value>,
   ) -> LspResult<Option<Value>> {
@@ -1089,30 +1080,139 @@ impl lspower::LanguageServer for LanguageServer {
   }
 }
 
-#[derive(Debug, Deserialize, Serialize)]
-#[serde(rename_all = "camelCase")]
-pub struct CacheParams {
-  pub text_document: TextDocumentIdentifier,
+#[lspower::async_trait]
+impl lspower::LanguageServer for LanguageServer {
+  async fn initialize(
+    &self,
+    params: InitializeParams,
+  ) -> LspResult<InitializeResult> {
+    self.0.lock().await.initialize(params).await
+  }
+
+  async fn initialized(&self, params: InitializedParams) {
+    self.0.lock().await.initialized(params).await
+  }
+
+  async fn shutdown(&self) -> LspResult<()> {
+    self.0.lock().await.shutdown().await
+  }
+
+  async fn did_open(&self, params: DidOpenTextDocumentParams) {
+    self.0.lock().await.did_open(params).await
+  }
+
+  async fn did_change(&self, params: DidChangeTextDocumentParams) {
+    self.0.lock().await.did_change(params).await
+  }
+
+  async fn did_close(&self, params: DidCloseTextDocumentParams) {
+    self.0.lock().await.did_close(params).await
+  }
+
+  async fn did_save(&self, params: DidSaveTextDocumentParams) {
+    self.0.lock().await.did_save(params).await
+  }
+
+  async fn did_change_configuration(
+    &self,
+    params: DidChangeConfigurationParams,
+  ) {
+    self.0.lock().await.did_change_configuration(params).await
+  }
+
+  async fn did_change_watched_files(
+    &self,
+    params: DidChangeWatchedFilesParams,
+  ) {
+    self.0.lock().await.did_change_watched_files(params).await
+  }
+
+  async fn formatting(
+    &self,
+    params: DocumentFormattingParams,
+  ) -> LspResult<Option<Vec<TextEdit>>> {
+    self.0.lock().await.formatting(params).await
+  }
+
+  async fn hover(&self, params: HoverParams) -> LspResult<Option<Hover>> {
+    self.0.lock().await.hover(params).await
+  }
+
+  async fn document_highlight(
+    &self,
+    params: DocumentHighlightParams,
+  ) -> LspResult<Option<Vec<DocumentHighlight>>> {
+    self.0.lock().await.document_highlight(params).await
+  }
+
+  async fn references(
+    &self,
+    params: ReferenceParams,
+  ) -> LspResult<Option<Vec<Location>>> {
+    self.0.lock().await.references(params).await
+  }
+
+  async fn goto_definition(
+    &self,
+    params: GotoDefinitionParams,
+  ) -> LspResult<Option<GotoDefinitionResponse>> {
+    self.0.lock().await.goto_definition(params).await
+  }
+
+  async fn completion(
+    &self,
+    params: CompletionParams,
+  ) -> LspResult<Option<CompletionResponse>> {
+    self.0.lock().await.completion(params).await
+  }
+
+  async fn goto_implementation(
+    &self,
+    params: GotoImplementationParams,
+  ) -> LspResult<Option<GotoImplementationResponse>> {
+    self.0.lock().await.goto_implementation(params).await
+  }
+
+  async fn rename(
+    &self,
+    params: RenameParams,
+  ) -> LspResult<Option<WorkspaceEdit>> {
+    self.0.lock().await.rename(params).await
+  }
+
+  async fn request_else(
+    &self,
+    method: &str,
+    params: Option<Value>,
+  ) -> LspResult<Option<Value>> {
+    self.0.lock().await.request_else(method, params).await
+  }
 }
 
 #[derive(Debug, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
-pub struct VirtualTextDocumentParams {
-  pub text_document: TextDocumentIdentifier,
+struct CacheParams {
+  text_document: TextDocumentIdentifier,
 }
 
-impl LanguageServer {
-  async fn cache(&self, params: CacheParams) -> LspResult<bool> {
+#[derive(Debug, Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
+struct VirtualTextDocumentParams {
+  text_document: TextDocumentIdentifier,
+}
+
+impl Inner {
+  async fn cache(&mut self, params: CacheParams) -> LspResult<bool> {
     let specifier = utils::normalize_url(params.text_document.uri);
-    let maybe_import_map = self.maybe_import_map.lock().unwrap().clone();
+    let maybe_import_map = self.maybe_import_map.clone();
     sources::cache(specifier.clone(), maybe_import_map)
       .await
       .map_err(|err| {
         error!("{}", err);
         LspError::internal_error()
       })?;
-    if self.documents.lock().unwrap().contains(&specifier) {
-      self.diagnostics.lock().unwrap().invalidate(&specifier);
+    if self.documents.contains(&specifier) {
+      self.diagnostics.invalidate(&specifier);
     }
     self.prepare_diagnostics().await.map_err(|err| {
       error!("{}", err);
@@ -1128,20 +1228,18 @@ impl LanguageServer {
     let specifier = utils::normalize_url(params.text_document.uri);
     let url = specifier.as_url();
     let contents = if url.as_str() == "deno:/status.md" {
-      let documents = self.documents.lock().unwrap();
       Some(format!(
         r#"# Deno Language Server Status
 
   - Documents in memory: {}
 
   "#,
-        documents.len()
+        self.documents.len()
       ))
     } else {
       match url.scheme() {
         "asset" => {
-          let maybe_asset =
-            { self.assets.lock().unwrap().get(&specifier).cloned() };
+          let maybe_asset = self.assets.get(&specifier).cloned();
           if let Some(maybe_asset) = maybe_asset {
             if let Some(asset) = maybe_asset {
               Some(asset.text)
@@ -1149,9 +1247,9 @@ impl LanguageServer {
               None
             }
           } else {
-            let state_snapshot = self.snapshot();
+            let mut state_snapshot = self.snapshot();
             if let Some(asset) =
-              tsc::get_asset(&specifier, &self.ts_server, &state_snapshot)
+              tsc::get_asset(&specifier, &self.ts_server, &mut state_snapshot)
                 .await
                 .map_err(|_| LspError::internal_error())?
             {
@@ -1163,8 +1261,7 @@ impl LanguageServer {
           }
         }
         _ => {
-          let mut sources = self.sources.lock().unwrap();
-          if let Some(text) = sources.get_text(&specifier) {
+          if let Some(text) = self.sources.get_text(&specifier) {
             Some(text)
           } else {
             error!("The cached sources was not found: {}", specifier);

cli/lsp/sources.rs

@@ -51,7 +51,10 @@ struct Metadata {
 }
 
 #[derive(Debug, Clone, Default)]
-pub struct Sources {
+pub struct Sources(Arc<Mutex<Inner>>);
+
+#[derive(Debug, Default)]
+struct Inner {
   http_cache: HttpCache,
   maybe_import_map: Option<ImportMap>,
   metadata: HashMap<ModuleSpecifier, Metadata>,
@@ -61,13 +64,64 @@ pub struct Sources {
 
 impl Sources {
   pub fn new(location: &Path) -> Self {
+    Self(Arc::new(Mutex::new(Inner::new(location))))
+  }
+
+  pub fn contains(&self, specifier: &ModuleSpecifier) -> bool {
+    self.0.lock().unwrap().contains(specifier)
+  }
+
+  /// Provides the length of the source content, calculated in a way that should
+  /// match the behavior of JavaScript, where strings are stored effectively as
+  /// `&[u16]` and when counting "chars" we need to represent the string as a
+  /// UTF-16 string in Rust.
+  pub fn get_length_utf16(&self, specifier: &ModuleSpecifier) -> Option<usize> {
+    self.0.lock().unwrap().get_length_utf16(specifier)
+  }
+
+  pub fn get_line_index(
+    &self,
+    specifier: &ModuleSpecifier,
+  ) -> Option<LineIndex> {
+    self.0.lock().unwrap().get_line_index(specifier)
+  }
+
+  pub fn get_media_type(
+    &self,
+    specifier: &ModuleSpecifier,
+  ) -> Option<MediaType> {
+    self.0.lock().unwrap().get_media_type(specifier)
+  }
+
+  pub fn get_script_version(
+    &self,
+    specifier: &ModuleSpecifier,
+  ) -> Option<String> {
+    self.0.lock().unwrap().get_script_version(specifier)
+  }
+
+  pub fn get_text(&self, specifier: &ModuleSpecifier) -> Option<String> {
+    self.0.lock().unwrap().get_text(specifier)
+  }
+
+  pub fn resolve_import(
+    &self,
+    specifier: &str,
+    referrer: &ModuleSpecifier,
+  ) -> Option<(ModuleSpecifier, MediaType)> {
+    self.0.lock().unwrap().resolve_import(specifier, referrer)
+  }
+}
+
+impl Inner {
+  fn new(location: &Path) -> Self {
     Self {
       http_cache: HttpCache::new(location),
       ..Default::default()
     }
   }
 
-  pub fn contains(&mut self, specifier: &ModuleSpecifier) -> bool {
+  fn contains(&mut self, specifier: &ModuleSpecifier) -> bool {
     if let Some(specifier) = self.resolve_specifier(specifier) {
       if self.get_metadata(&specifier).is_some() {
         return true;
@@ -76,20 +130,13 @@ impl Sources {
     false
   }
 
-  /// Provides the length of the source content, calculated in a way that should
-  /// match the behavior of JavaScript, where strings are stored effectively as
-  /// `&[u16]` and when counting "chars" we need to represent the string as a
-  /// UTF-16 string in Rust.
-  pub fn get_length_utf16(
-    &mut self,
-    specifier: &ModuleSpecifier,
-  ) -> Option<usize> {
+  fn get_length_utf16(&mut self, specifier: &ModuleSpecifier) -> Option<usize> {
     let specifier = self.resolve_specifier(specifier)?;
     let metadata = self.get_metadata(&specifier)?;
     Some(metadata.source.encode_utf16().count())
   }
 
-  pub fn get_line_index(
+  fn get_line_index(
     &mut self,
     specifier: &ModuleSpecifier,
   ) -> Option<LineIndex> {
@@ -98,7 +145,7 @@ impl Sources {
     Some(metadata.line_index)
   }
 
-  pub fn get_media_type(
+  fn get_media_type(
     &mut self,
     specifier: &ModuleSpecifier,
   ) -> Option<MediaType> {
@@ -236,7 +283,7 @@ impl Sources {
     None
   }
 
-  pub fn get_script_version(
+  fn get_script_version(
     &mut self,
     specifier: &ModuleSpecifier,
   ) -> Option<String> {
@@ -257,7 +304,7 @@ impl Sources {
     None
   }
 
-  pub fn get_text(&mut self, specifier: &ModuleSpecifier) -> Option<String> {
+  fn get_text(&mut self, specifier: &ModuleSpecifier) -> Option<String> {
     let specifier = self.resolve_specifier(specifier)?;
     let metadata = self.get_metadata(&specifier)?;
     Some(metadata.source)
@@ -277,7 +324,7 @@ impl Sources {
     Some((resolved_specifier, media_type))
   }
 
-  pub fn resolve_import(
+  fn resolve_import(
     &mut self,
     specifier: &str,
     referrer: &ModuleSpecifier,
@@ -373,7 +420,7 @@ mod tests {
 
   #[test]
   fn test_sources_get_script_version() {
-    let (mut sources, _) = setup();
+    let (sources, _) = setup();
     let c = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap());
     let tests = c.join("tests");
     let specifier = ModuleSpecifier::resolve_path(
@@ -386,7 +433,7 @@ mod tests {
 
   #[test]
   fn test_sources_get_text() {
-    let (mut sources, _) = setup();
+    let (sources, _) = setup();
     let c = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap());
     let tests = c.join("tests");
     let specifier = ModuleSpecifier::resolve_path(
@@ -401,7 +448,7 @@ mod tests {
 
   #[test]
   fn test_sources_get_length_utf16() {
-    let (mut sources, _) = setup();
+    let (sources, _) = setup();
     let c = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap());
     let tests = c.join("tests");
     let specifier = ModuleSpecifier::resolve_path(
@@ -416,10 +463,10 @@ mod tests {
 
   #[test]
   fn test_sources_resolve_specifier_non_supported_schema() {
-    let (mut sources, _) = setup();
+    let (sources, _) = setup();
     let specifier = ModuleSpecifier::resolve_url("foo://a/b/c.ts")
       .expect("could not create specifier");
-    let actual = sources.resolve_specifier(&specifier);
+    let actual = sources.0.lock().unwrap().resolve_specifier(&specifier);
     assert!(actual.is_none());
   }
 }

cli/lsp/tsc.rs

@@ -96,7 +96,7 @@ pub struct AssetDocument {
 pub async fn get_asset(
   specifier: &ModuleSpecifier,
   ts_server: &TsServer,
-  state_snapshot: &StateSnapshot,
+  state_snapshot: &mut StateSnapshot,
 ) -> Result<Option<AssetDocument>, AnyError> {
   let specifier_str = specifier.to_string().replace("asset:///", "");
   if let Some(text) = tsc::get_asset(&specifier_str) {
@@ -106,8 +106,6 @@ pub async fn get_asset(
     });
     state_snapshot
       .assets
-      .lock()
-      .unwrap()
       .insert(specifier.clone(), maybe_asset.clone());
     Ok(maybe_asset)
   } else {
@@ -128,8 +126,6 @@ pub async fn get_asset(
     };
     state_snapshot
       .assets
-      .lock()
-      .unwrap()
       .insert(specifier.clone(), maybe_asset.clone());
     Ok(maybe_asset)
   }
@@ -915,7 +911,7 @@ fn get_length(state: &mut State, args: Value) -> Result<Value, AnyError> {
       .unwrap();
     Ok(json!(content.encode_utf16().count()))
   } else {
-    let mut sources = state.state_snapshot.sources.lock().unwrap();
+    let sources = &state.state_snapshot.sources;
     Ok(json!(sources.get_length_utf16(&specifier).unwrap()))
   }
 }
@@ -940,7 +936,7 @@ fn get_text(state: &mut State, args: Value) -> Result<Value, AnyError> {
       .unwrap()
       .clone()
   } else {
-    let mut sources = state.state_snapshot.sources.lock().unwrap();
+    let sources = &state.state_snapshot.sources;
     sources.get_text(&specifier).unwrap()
   };
   Ok(json!(text::slice(&content, v.start..v.end)))
@@ -950,11 +946,7 @@ fn resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
   let v: ResolveArgs = serde_json::from_value(args)?;
   let mut resolved = Vec::<Option<(String, String)>>::new();
   let referrer = ModuleSpecifier::resolve_url(&v.base)?;
-  let mut sources = if let Ok(sources) = state.state_snapshot.sources.lock() {
-    sources
-  } else {
-    return Err(custom_error("Deadlock", "deadlock locking sources"));
-  };
+  let sources = &state.state_snapshot.sources;
 
   if state.state_snapshot.documents.contains(&referrer) {
     if let Some(dependencies) =
@@ -1048,7 +1040,7 @@ fn script_version(state: &mut State, args: Value) -> Result<Value, AnyError> {
   if let Some(version) = state.state_snapshot.documents.version(&specifier) {
     return Ok(json!(version.to_string()));
   } else {
-    let mut sources = state.state_snapshot.sources.lock().unwrap();
+    let sources = &state.state_snapshot.sources;
     if let Some(version) = sources.get_script_version(&specifier) {
       return Ok(json!(version));
     }