Remove excessive mutex synchronization (#3637)

Remove excessive mutex synchronization (#3637)

### Motivation

With the improvements to the position scanners (#3612 and #3583), we're now seeing some cases of invalid location being raised, despite no reports of any issues regarding the state of the documents.

I think we're making a mistake by trying to apply document operations with higher priority than feature requests. For example, if we receive a request for completion and the user immediately edits the file, the server might process the edit before finishing the completion request and there are no guarantees that the new state of the document can satisfy that original request.

I want to propose pushing text synchronization operations to the queue, so that they are processed in order and without locking, which I believe will improve the situation.

### Implementation

I start pushing text synchronization operations to the work queue and removed many of the mutex locks that we originally had.

Since requests are processed in order, there should not be a chance of a feature request being processed with an incorrect document state.

Co-authored-by: vinistock <[email protected]>

Author: vinistock

lib/ruby_lsp/base_server.rb

@@ -83,8 +83,7 @@ def start
         # The following requests need to be executed in the main thread directly to avoid concurrency issues. Everything
         # else is pushed into the incoming queue
         case method
-        when "initialize", "initialized", "textDocument/didOpen", "textDocument/didClose", "textDocument/didChange",
-          "rubyLsp/diagnoseState"
+        when "initialize", "initialized", "rubyLsp/diagnoseState"
           process_message(message)
         when "shutdown"
           @global_state.synchronize do
@@ -94,7 +93,7 @@ def start
             @writer.write(Result.new(id: message[:id], response: nil).to_hash)
           end
         when "exit"
-          @global_state.synchronize { exit(@incoming_queue.closed? ? 0 : 1) }
+          exit(@incoming_queue.closed? ? 0 : 1)
         else
           @incoming_queue << message
         end

lib/ruby_lsp/document.rb

@@ -139,12 +139,10 @@ def past_expensive_limit?
 
     #: (Hash[Symbol, untyped] start_pos, ?Hash[Symbol, untyped]? end_pos) -> [Integer, Integer?]
     def find_index_by_position(start_pos, end_pos = nil)
-      @global_state.synchronize do
-        scanner = create_scanner
-        start_index = scanner.find_char_position(start_pos)
-        end_index = scanner.find_char_position(end_pos) if end_pos
-        [start_index, end_index]
-      end
+      scanner = create_scanner
+      start_index = scanner.find_char_position(start_pos)
+      end_index = scanner.find_char_position(end_pos) if end_pos
+      [start_index, end_index]
     end
 
     private

lib/ruby_lsp/server.rb

@@ -367,62 +367,56 @@ def run_initialized
 
     #: (Hash[Symbol, untyped] message) -> void
     def text_document_did_open(message)
-      @global_state.synchronize do
-        text_document = message.dig(:params, :textDocument)
-        language_id = case text_document[:languageId]
-        when "erb", "eruby"
-          :erb
-        when "rbs"
-          :rbs
-        else
-          :ruby
-        end
+      text_document = message.dig(:params, :textDocument)
+      language_id = case text_document[:languageId]
+      when "erb", "eruby"
+        :erb
+      when "rbs"
+        :rbs
+      else
+        :ruby
+      end
 
-        document = @store.set(
-          uri: text_document[:uri],
-          source: text_document[:text],
-          version: text_document[:version],
-          language_id: language_id,
-        )
+      document = @store.set(
+        uri: text_document[:uri],
+        source: text_document[:text],
+        version: text_document[:version],
+        language_id: language_id,
+      )
 
-        if document.past_expensive_limit? && text_document[:uri].scheme == "file"
-          log_message = <<~MESSAGE
-            The file #{text_document[:uri].path} is too long. For performance reasons, semantic highlighting and
-            diagnostics will be disabled.
-          MESSAGE
+      if document.past_expensive_limit? && text_document[:uri].scheme == "file"
+        log_message = <<~MESSAGE
+          The file #{text_document[:uri].path} is too long. For performance reasons, semantic highlighting and
+          diagnostics will be disabled.
+        MESSAGE
 
-          send_message(
-            Notification.new(
-              method: "window/logMessage",
-              params: Interface::LogMessageParams.new(
-                type: Constant::MessageType::WARNING,
-                message: log_message,
-              ),
+        send_message(
+          Notification.new(
+            method: "window/logMessage",
+            params: Interface::LogMessageParams.new(
+              type: Constant::MessageType::WARNING,
+              message: log_message,
             ),
-          )
-        end
+          ),
+        )
       end
     end
 
     #: (Hash[Symbol, untyped] message) -> void
     def text_document_did_close(message)
-      @global_state.synchronize do
-        uri = message.dig(:params, :textDocument, :uri)
-        @store.delete(uri)
+      uri = message.dig(:params, :textDocument, :uri)
+      @store.delete(uri)
 
-        # Clear diagnostics for the closed file, so that they no longer appear in the problems tab
-        send_message(Notification.publish_diagnostics(uri.to_s, []))
-      end
+      # Clear diagnostics for the closed file, so that they no longer appear in the problems tab
+      send_message(Notification.publish_diagnostics(uri.to_s, []))
     end
 
     #: (Hash[Symbol, untyped] message) -> void
     def text_document_did_change(message)
       params = message[:params]
       text_document = params[:textDocument]
 
-      @global_state.synchronize do
-        @store.push_edits(uri: text_document[:uri], edits: params[:contentChanges], version: text_document[:version])
-      end
+      @store.push_edits(uri: text_document[:uri], edits: params[:contentChanges], version: text_document[:version])
     end
 
     #: (Hash[Symbol, untyped] message) -> void
@@ -1105,11 +1099,7 @@ def handle_rubocop_config_change(uri)
       @global_state.register_formatter("rubocop_internal", Requests::Support::RuboCopFormatter.new)
 
       # Clear all document caches for pull diagnostics
-      @global_state.synchronize do
-        @store.each do |_uri, document|
-          document.clear_cache("textDocument/diagnostic")
-        end
-      end
+      @store.each { |_uri, document| document.clear_cache("textDocument/diagnostic") }
 
       # Request a pull diagnostic refresh from the editor
       if @global_state.client_capabilities.supports_diagnostic_refresh