Synchronize store operations in server instead

Trying to synchronize inside the store is a mistake
because it may lead to deadlocks. We need the server
to control this aspect

lib/ruby_lsp/document.rb

@@ -97,33 +97,31 @@ def cache_get(request_name)
 
     sig { params(edits: T::Array[T::Hash[Symbol, T.untyped]], version: Integer).void }
     def push_edits(edits, version:)
-      @global_state.synchronize do
-        edits.each do |edit|
-          range = edit[:range]
-          scanner = create_scanner
+      edits.each do |edit|
+        range = edit[:range]
+        scanner = create_scanner
 
-          start_position = scanner.find_char_position(range[:start])
-          end_position = scanner.find_char_position(range[:end])
+        start_position = scanner.find_char_position(range[:start])
+        end_position = scanner.find_char_position(range[:end])
 
-          @source[start_position...end_position] = edit[:text]
-        end
+        @source[start_position...end_position] = edit[:text]
+      end
 
-        @version = version
-        @needs_parsing = true
-        @cache.clear
+      @version = version
+      @needs_parsing = true
+      @cache.clear
 
-        last_edit = edits.last
-        return unless last_edit
+      last_edit = edits.last
+      return unless last_edit
 
-        last_edit_range = last_edit[:range]
+      last_edit_range = last_edit[:range]
 
-        @last_edit = if last_edit_range[:start] == last_edit_range[:end]
-          Insert.new(last_edit_range)
-        elsif last_edit[:text].empty?
-          Delete.new(last_edit_range)
-        else
-          Replace.new(last_edit_range)
-        end
+      @last_edit = if last_edit_range[:start] == last_edit_range[:end]
+        Insert.new(last_edit_range)
+      elsif last_edit[:text].empty?
+        Delete.new(last_edit_range)
+      else
+        Replace.new(last_edit_range)
       end
     end
 

lib/ruby_lsp/server.rb

@@ -359,56 +359,62 @@ def run_initialized
 
     sig { params(message: T::Hash[Symbol, T.untyped]).void }
     def text_document_did_open(message)
-      text_document = message.dig(:params, :textDocument)
-      language_id = case text_document[:languageId]
-      when "erb", "eruby"
-        Document::LanguageId::ERB
-      when "rbs"
-        Document::LanguageId::RBS
-      else
-        Document::LanguageId::Ruby
-      end
+      @global_state.synchronize do
+        text_document = message.dig(:params, :textDocument)
+        language_id = case text_document[:languageId]
+        when "erb", "eruby"
+          Document::LanguageId::ERB
+        when "rbs"
+          Document::LanguageId::RBS
+        else
+          Document::LanguageId::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
 
     sig { params(message: T::Hash[Symbol, T.untyped]).void }
     def text_document_did_close(message)
-      uri = message.dig(:params, :textDocument, :uri)
-      @store.delete(uri)
+      @global_state.synchronize do
+        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, []))
+        # 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
     end
 
     sig { params(message: T::Hash[Symbol, T.untyped]).void }
     def text_document_did_change(message)
       params = message[:params]
       text_document = params[:textDocument]
 
-      @store.push_edits(uri: text_document[:uri], edits: params[:contentChanges], version: text_document[:version])
+      @global_state.synchronize do
+        @store.push_edits(uri: text_document[:uri], edits: params[:contentChanges], version: text_document[:version])
+      end
     end
 
     sig { params(message: T::Hash[Symbol, T.untyped]).void }

lib/ruby_lsp/store.rb

@@ -65,15 +65,13 @@ def get(uri)
       ).returns(Document[T.untyped])
     end
     def set(uri:, source:, version:, language_id:)
-      @global_state.synchronize do
-        @state[uri.to_s] = case language_id
-        when Document::LanguageId::ERB
-          ERBDocument.new(source: source, version: version, uri: uri, global_state: @global_state)
-        when Document::LanguageId::RBS
-          RBSDocument.new(source: source, version: version, uri: uri, global_state: @global_state)
-        else
-          RubyDocument.new(source: source, version: version, uri: uri, global_state: @global_state)
-        end
+      @state[uri.to_s] = case language_id
+      when Document::LanguageId::ERB
+        ERBDocument.new(source: source, version: version, uri: uri, global_state: @global_state)
+      when Document::LanguageId::RBS
+        RBSDocument.new(source: source, version: version, uri: uri, global_state: @global_state)
+      else
+        RubyDocument.new(source: source, version: version, uri: uri, global_state: @global_state)
       end
     end
 
@@ -94,9 +92,7 @@ def empty?
 
     sig { params(uri: URI::Generic).void }
     def delete(uri)
-      @global_state.synchronize do
-        @state.delete(uri.to_s)
-      end
+      @state.delete(uri.to_s)
     end
 
     sig { params(uri: URI::Generic).returns(T::Boolean) }

vscode/src/test/suite/client.test.ts

@@ -1034,4 +1034,24 @@ suite("Client", () => {
       ),
     );
   }).timeout(20000);
+
+  test("requests for documents that were not opened by the client", async () => {
+    const uri = vscode.Uri.joinPath(
+      workspaceUri,
+      "lib",
+      "ruby_lsp",
+      "server.rb",
+    );
+
+    const response: vscode.DocumentSymbol[] = await client.sendRequest(
+      "textDocument/documentSymbol",
+      {
+        textDocument: {
+          uri: uri.toString(),
+        },
+      },
+    );
+
+    assert.ok(response.length > 0);
+  }).timeout(20000);
 });