Fix runner client race condition (#270)

* Update ruby-lsp requirement

* Fix race condition when booting the server

Author: vinistock

Gemfile.lock

@@ -5,7 +5,7 @@ PATH
       actionpack (>= 6.0)
       activerecord (>= 6.0)
       railties (>= 6.0)
-      ruby-lsp (>= 0.14.0, < 0.15.0)
+      ruby-lsp (>= 0.14.2, < 0.15.0)
       sorbet-runtime (>= 0.5.9897)
 
 GEM
@@ -221,7 +221,7 @@ GEM
       rubocop (~> 1.51)
     rubocop-sorbet (0.7.7)
       rubocop (>= 0.90.0)
-    ruby-lsp (0.14.1)
+    ruby-lsp (0.14.2)
       language_server-protocol (~> 3.17.0)
       prism (>= 0.22.0, < 0.25)
       sorbet-runtime (>= 0.5.10782)

lib/ruby_lsp/ruby_lsp_rails/addon.rb

@@ -14,21 +14,24 @@ module Rails
     class Addon < ::RubyLsp::Addon
       extend T::Sig
 
-      sig { returns(RunnerClient) }
-      def client
-        @client ||= T.let(RunnerClient.create_client, T.nilable(RunnerClient))
+      sig { void }
+      def initialize
+        super
+
+        # We first initialize the client as a NullClient, so that we can start the server in a background thread. Until
+        # the real client is initialized, features that depend on it will not be blocked by using the NullClient
+        @client = T.let(NullClient.new, RunnerClient)
       end
 
       sig { override.params(message_queue: Thread::Queue).void }
       def activate(message_queue)
-        # Eagerly initialize the client in a thread. This allows the indexing from the Ruby LSP to continue running even
-        # while we boot large Rails applications in the background
-        Thread.new { client }
+        # Start booting the real client in a background thread. Until this completes, the client will be a NullClient
+        Thread.new { @client = RunnerClient.create_client }
       end
 
       sig { override.void }
       def deactivate
-        client.shutdown
+        @client.shutdown
       end
 
       # Creates a new CodeLens listener. This method is invoked on every CodeLens request
@@ -52,7 +55,7 @@ def create_code_lens_listener(response_builder, uri, dispatcher)
         ).void
       end
       def create_hover_listener(response_builder, nesting, index, dispatcher)
-        Hover.new(client, response_builder, nesting, index, dispatcher)
+        Hover.new(@client, response_builder, nesting, index, dispatcher)
       end
 
       sig do

ruby-lsp-rails.gemspec

@@ -24,6 +24,6 @@ Gem::Specification.new do |spec|
   spec.add_dependency("actionpack", ">= 6.0")
   spec.add_dependency("activerecord", ">= 6.0")
   spec.add_dependency("railties", ">= 6.0")
-  spec.add_dependency("ruby-lsp", ">= 0.14.0", "< 0.15.0")
+  spec.add_dependency("ruby-lsp", ">= 0.14.2", "< 0.15.0")
   spec.add_dependency("sorbet-runtime", ">= 0.5.9897")
 end

test/ruby_lsp_rails/code_lens_test.rb

@@ -207,6 +207,13 @@ def generate_code_lens_for_source(source)
         store = RubyLsp::Store.new
         store.set(uri: uri, source: source, version: 1)
 
+        capture_subprocess_io do
+          RubyLsp::Executor.new(store, @message_queue).execute({
+            method: "initialized",
+            params: {},
+          })
+        end
+
         response = RubyLsp::Executor.new(store, @message_queue).execute({
           method: "textDocument/codeLens",
           params: { textDocument: { uri: uri }, position: { line: 0, character: 0 } },

test/ruby_lsp_rails/hover_test.rb

@@ -195,21 +195,25 @@ def hover_on_source(source, position)
           RubyIndexer::IndexablePath.new(nil, T.must(uri.to_standardized_path)), source
         )
 
-        response = T.let(nil, T.nilable(RubyLsp::Result))
-        capture_io do
-          response = executor.execute(
-            {
-              method: "textDocument/hover",
-              params: {
-                textDocument: { uri: uri },
-                position: position,
-              },
-            },
-          )
+        capture_subprocess_io do
+          RubyLsp::Executor.new(store, @message_queue).execute({
+            method: "initialized",
+            params: {},
+          })
         end
 
-        assert_nil(T.must(response).error)
-        T.must(response).response
+        response = executor.execute(
+          {
+            method: "textDocument/hover",
+            params: {
+              textDocument: { uri: uri },
+              position: position,
+            },
+          },
+        )
+
+        assert_nil(response.error)
+        response.response
       end
 
       def dummy_root