Optimize lock usage and documentation, reduce the risk of deadlocks, and improve file handling logic.

Author: CppCXY

crates/emmylua_ls/src/context/mod.rs

@@ -23,6 +23,81 @@ pub use workspace_manager::*;
 
 use crate::context::snapshot::ServerContextInner;
 
+// ============================================================================
+// LOCK ORDERING GUIDELINES (CRITICAL - Must Follow to Avoid Deadlocks)
+// ============================================================================
+//
+// This module uses multiple locks (RwLock and Mutex) for concurrent access to shared state.
+// To prevent deadlocks, **ALL code must acquire locks in the following order**:
+//
+// ## Global Lock Order (Low to High Priority):
+// 1. **diagnostic_tokens** (Mutex) - File diagnostic task tokens
+// 2. **workspace_diagnostic_token** (Mutex) - Workspace diagnostic task token
+// 3. **update_token** (Mutex) - Reindex/config update token
+// 4. **analysis** (RwLock - READ) - Read-only access to EmmyLuaAnalysis
+// 5. **workspace_manager** (RwLock - READ) - Read-only access to WorkspaceManager
+// 6. **workspace_manager** (RwLock - WRITE) - Exclusive access to WorkspaceManager
+// 7. **analysis** (RwLock - WRITE) - Exclusive access to EmmyLuaAnalysis
+//
+// ## Lock Ordering Rules:
+// - **NEVER acquire a lower-priority lock while holding a higher-priority lock**
+// - **ALWAYS release locks in reverse order (LIFO) or use explicit scope blocks**
+// - **NEVER upgrade a read lock to a write lock (release read, then acquire write)**
+// - **Minimize lock scope**: only hold locks for the minimum necessary time
+// - **Avoid holding locks across `.await` points when possible**
+// - **NEVER call async methods that might acquire locks while holding a lock**
+//
+// ## Examples:
+//
+// ### ✅ CORRECT - Proper lock ordering:
+// ```rust
+// // Acquire workspace_manager read lock first, then release before analysis write
+// let should_process = {
+//     let workspace_manager = context.workspace_manager().read().await;
+//     workspace_manager.is_workspace_file(&uri)
+// };
+// if should_process {
+//     let mut analysis = context.analysis().write().await;
+//     analysis.update_file(&uri, text);
+// }
+// ```
+//
+// ### ❌ WRONG - ABBA deadlock risk:
+// ```rust
+// let mut analysis = context.analysis().write().await;  // Lock A
+// // ... operations ...
+// let workspace = context.workspace_manager().write().await;  // Lock B (while holding A!)
+// // DEADLOCK RISK: Another thread might hold B and wait for A
+// ```
+//
+// ### ✅ CORRECT - Release before calling async methods:
+// ```rust
+// let data = {
+//     let workspace = context.workspace_manager().read().await;
+//     workspace.get_config().clone()  // Clone data
+// }; // Lock released
+// init_analysis(data).await;  // Safe to call async method
+// ```
+//
+// ### ❌ WRONG - Holding lock while calling async method:
+// ```rust
+// let workspace = context.workspace_manager().write().await;
+// workspace.reload_workspace().await;  // May acquire analysis lock internally!
+// ```
+//
+// ## Atomic Operations (Lock-Free):
+// The following atomics can be accessed without lock ordering concerns:
+// - `workspace_initialized` (AtomicBool)
+// - `workspace_diagnostic_level` (AtomicU8)
+// - `workspace_version` (AtomicI64)
+//
+// ## Notes:
+// - Use `drop(lock_guard)` explicitly to release locks early when needed
+// - Use scope blocks `{ ... }` to limit lock lifetime
+// - When in doubt, release all locks before performing complex operations
+// - If you need to modify this ordering, update this documentation AND review all call sites
+// ============================================================================
+
 pub struct ServerContext {
     #[allow(unused)]
     conn: Connection,

crates/emmylua_ls/src/context/workspace_manager.rs

@@ -192,16 +192,21 @@ impl WorkspaceManager {
                 return;
             }
 
-            let mut analysis = analysis.write().await;
-
-            // 在重新索引之前清理不存在的文件
-            analysis.cleanup_nonexistent_files();
+            // Perform reindex with minimal lock holding time
+            {
+                let mut analysis = analysis.write().await;
+                // 在重新索引之前清理不存在的文件
+                analysis.cleanup_nonexistent_files();
+                analysis.reindex();
+                // Release lock immediately after reindex
+            }
 
-            analysis.reindex();
+            // Cancel diagnostics and update status without holding analysis lock
             file_diagnostic.cancel_workspace_diagnostic().await;
             workspace_diagnostic_status
                 .store(WorkspaceDiagnosticLevel::Fast.to_u8(), Ordering::Release);
 
+            // Trigger diagnostics refresh
             if lsp_features.supports_workspace_diagnostic() {
                 client.refresh_workspace_diagnostics();
             } else {

crates/emmylua_ls/src/handlers/completion/providers/equality_provider.rs

@@ -1,7 +1,5 @@
 use emmylua_code_analysis::{InferGuard, LuaType, infer_table_field_value_should_be};
-use emmylua_parser::{
-    BinaryOperator, LuaAst, LuaAstNode, LuaBlock, LuaLiteralExpr,
-};
+use emmylua_parser::{BinaryOperator, LuaAst, LuaAstNode, LuaBlock, LuaLiteralExpr};
 
 use crate::handlers::completion::{
     completion_builder::CompletionBuilder, providers::function_provider::dispatch_type,

crates/emmylua_ls/src/handlers/configuration/mod.rs

@@ -11,27 +11,36 @@ pub async fn on_did_change_configuration(
     let pretty_json = serde_json::to_string_pretty(&params).ok()?;
     log::info!("on_did_change_configuration: {}", pretty_json);
 
-    let workspace_manager = context.workspace_manager().read().await;
-    if !workspace_manager.is_workspace_initialized() {
+    // Check initialization status and get client config
+    let (is_initialized, client_id, supports_config_request) = {
+        let workspace_manager = context.workspace_manager().read().await;
+        let is_initialized = workspace_manager.is_workspace_initialized();
+        let client_id = workspace_manager.client_config.client_id;
+        let supports_config_request = context.lsp_features().supports_config_request();
+        (is_initialized, client_id, supports_config_request)
+    };
+
+    if !is_initialized {
         return Some(());
     }
 
-    let client_id = workspace_manager.client_config.client_id;
     if client_id.is_vscode() {
         return Some(());
     }
 
-    drop(workspace_manager);
-
-    let supports_config_request = context.lsp_features().supports_config_request();
-
     log::info!("change config client_id: {:?}", client_id);
+
+    // Get new config without holding any locks
     let new_client_config = get_client_config(&context, client_id, supports_config_request).await;
-    let mut workspace_manager = context.workspace_manager().write().await;
-    workspace_manager.client_config = new_client_config;
 
-    log::info!("reloading workspace folders");
-    workspace_manager.reload_workspace().await;
+    // Update config and reload - acquire write lock only when necessary
+    {
+        let mut workspace_manager = context.workspace_manager().write().await;
+        workspace_manager.client_config = new_client_config;
+        log::info!("reloading workspace folders");
+        workspace_manager.reload_workspace().await;
+    }
+
     Some(())
 }
 

crates/emmylua_ls/src/handlers/text_document/text_document_handler.rs

@@ -11,22 +11,39 @@ pub async fn on_did_open_text_document(
     context: ServerContextSnapshot,
     params: DidOpenTextDocumentParams,
 ) -> Option<()> {
-    let mut analysis = context.analysis().write().await;
     let uri = params.text_document.uri;
     let text = params.text_document.text;
-    let old_file_id = analysis.get_file_id(&uri);
-    // check is filter file
-    if old_file_id.is_none() {
-        let workspace_manager = context.workspace_manager().read().await;
-        if !workspace_manager.is_workspace_file(&uri) {
-            return None;
+
+    // Check if file should be filtered before acquiring locks
+    // Follow lock order: workspace_manager (read) -> analysis (write)
+    let should_process = {
+        let analysis = context.analysis().read().await;
+        let old_file_id = analysis.get_file_id(&uri);
+        if old_file_id.is_some() {
+            true
+        } else {
+            drop(analysis);
+            let workspace_manager = context.workspace_manager().read().await;
+            workspace_manager.is_workspace_file(&uri)
         }
+    };
+
+    if !should_process {
+        return None;
     }
 
-    let file_id = analysis.update_file_by_uri(&uri, Some(text));
-    if !context.lsp_features().supports_pull_diagnostic() {
+    // Update file and get diagnostic settings
+    let (file_id, supports_pull, interval) = {
+        let mut analysis = context.analysis().write().await;
+        let file_id = analysis.update_file_by_uri(&uri, Some(text));
         let emmyrc = analysis.get_emmyrc();
         let interval = emmyrc.diagnostics.diagnostic_interval.unwrap_or(500);
+        let supports_pull = context.lsp_features().supports_pull_diagnostic();
+        (file_id, supports_pull, interval)
+    };
+
+    // Schedule diagnostic task without holding any locks
+    if !supports_pull {
         if let Some(file_id) = file_id {
             context
                 .file_diagnostic()
@@ -35,9 +52,11 @@ pub async fn on_did_open_text_document(
         }
     }
 
-    let mut workspace = context.workspace_manager().write().await;
-    workspace.current_open_files.insert(uri);
-    drop(workspace);
+    // Update open files list
+    {
+        let mut workspace = context.workspace_manager().write().await;
+        workspace.current_open_files.insert(uri);
+    }
 
     Some(())
 }
@@ -76,36 +95,54 @@ pub async fn on_did_change_text_document(
     context: ServerContextSnapshot,
     params: DidChangeTextDocumentParams,
 ) -> Option<()> {
-    let mut analysis = context.analysis().write().await;
     let uri = params.text_document.uri;
     let text = params.content_changes.first()?.text.clone();
-    let old_file_id = analysis.get_file_id(&uri);
-    // check is filter file
-    if old_file_id.is_none() {
-        let workspace_manager = context.workspace_manager().read().await;
-        if !workspace_manager.is_workspace_file(&uri) {
-            return None;
+
+    // Check if file should be filtered before acquiring locks
+    // Follow lock order: workspace_manager (read) -> analysis (write)
+    let should_process = {
+        let analysis = context.analysis().read().await;
+        let old_file_id = analysis.get_file_id(&uri);
+        if old_file_id.is_some() {
+            true
+        } else {
+            drop(analysis);
+            let workspace_manager = context.workspace_manager().read().await;
+            workspace_manager.is_workspace_file(&uri)
         }
+    };
+
+    if !should_process {
+        return None;
     }
 
-    let file_id = analysis.update_file_by_uri(&uri, Some(text));
-    let emmyrc = analysis.get_emmyrc();
+    // Update file and get settings
+    let (file_id, emmyrc, supports_pull) = {
+        let mut analysis = context.analysis().write().await;
+        let file_id = analysis.update_file_by_uri(&uri, Some(text));
+        let emmyrc = analysis.get_emmyrc();
+        let supports_pull = context.lsp_features().supports_pull_diagnostic();
+        (file_id, emmyrc, supports_pull)
+    };
+
     let interval = emmyrc.diagnostics.diagnostic_interval.unwrap_or(500);
-    drop(analysis);
 
+    // Handle reindex without holding locks
     if emmyrc.workspace.enable_reindex {
         let workspace = context.workspace_manager().read().await;
         workspace.extend_reindex_delay().await;
-        drop(workspace);
     }
-    if !context.lsp_features().supports_pull_diagnostic() {
+
+    // Schedule diagnostic task
+    if !supports_pull {
         if let Some(file_id) = file_id {
             context
                 .file_diagnostic()
                 .add_diagnostic_task(file_id, interval)
                 .await;
         }
     }
+
     Some(())
 }