From 2ea3b7905a18ed4eb052c474e6e27cbb016befe0 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Sat, 26 Oct 2024 14:19:29 -0400 Subject: [PATCH] Try timeout --- exe/ruby-lsp-launcher | 22 ++++++---- .../scripts/compose_bundle_windows.rb | 1 - test/integration_test.rb | 41 ++++++++++--------- 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/exe/ruby-lsp-launcher b/exe/ruby-lsp-launcher index 2cdf97dbc..8447143f8 100755 --- a/exe/ruby-lsp-launcher +++ b/exe/ruby-lsp-launcher @@ -22,9 +22,13 @@ locked_bundler_version_file = File.join(".ruby-lsp", "locked_bundler_version") # Compose the Ruby LSP bundle in a forked process so that we can require gems without polluting the main process # `$LOAD_PATH` and `Gem.loaded_specs`. Windows doesn't support forking, so we need a separate path to support it pid = if Gem.win_platform? + load_path = $LOAD_PATH.flat_map do |path| + ["-I", File.expand_path(path)] + end + Process.spawn( - ENV.to_hash, Gem.ruby, + *load_path, File.expand_path("../lib/ruby_lsp/scripts/compose_bundle_windows.rb", __dir__), raw_initialize, ) @@ -35,8 +39,12 @@ else end end -# Wait until the composed Bundle is finished -Process.wait(pid) +begin + # Wait until the composed Bundle is finished + Process.wait(pid) +rescue Errno::ECHILD + # In theory, the child process can finish before we even get to the wait call, but that is not an error +end begin # We can't require `bundler/setup` because that file prematurely exits the process if setup fails. However, we can't @@ -61,11 +69,11 @@ rescue StandardError => e # features in a degraded mode. We simply save the error so that we can report to the user that certain gems might be # missing, but we respect the LSP life cycle setup_error = e + $stderr.puts("Failed to set up composed Bundle\n#{e.full_message}") - # If we failed to set up the bundle, the minimum we need is to have our own dependencies activated so that we can - # require the gems the LSP depends on. If even that fails, then there's no way we can continue to run the language - # server - Gem::Specification.find_by_name("ruby-lsp").activate + # If Bundler.setup fails, we need to restore the original $LOAD_PATH so that we can still require the Ruby LSP server + # in degraded mode + $LOAD_PATH.unshift(File.expand_path("../lib", __dir__)) end error_path = File.join(".ruby-lsp", "install_error") diff --git a/lib/ruby_lsp/scripts/compose_bundle_windows.rb b/lib/ruby_lsp/scripts/compose_bundle_windows.rb index 3be82ad01..3bdc94115 100644 --- a/lib/ruby_lsp/scripts/compose_bundle_windows.rb +++ b/lib/ruby_lsp/scripts/compose_bundle_windows.rb @@ -1,7 +1,6 @@ # typed: strict # frozen_string_literal: true -$LOAD_PATH.unshift(File.expand_path("../../../lib", __dir__)) require_relative "compose_bundle" # When this is invoked on Windows, we pass the raw initialize as an argument to this script. On other platforms, we diff --git a/test/integration_test.rb b/test/integration_test.rb index 83fcd2562..f443cb481 100644 --- a/test/integration_test.rb +++ b/test/integration_test.rb @@ -140,9 +140,6 @@ def test_launch_mode_with_no_gemfile_and_bundle_path Bundler.with_unbundled_env do system("bundle config --local path #{File.join("vendor", "bundle")}") assert_path_exists(File.join(dir, ".bundle", "config")) - end - - Bundler.with_unbundled_env do launch(dir) end end @@ -151,16 +148,17 @@ def test_launch_mode_with_no_gemfile_and_bundle_path private def launch(workspace_path) - load_path = $-I.map do |p| - "-I#{File.expand_path(p)}" - end.join(" ") - - stdin, stdout, stderr, wait_thr = T.unsafe(Open3).popen3( - ENV.to_hash, - Gem.ruby, - load_path, - File.join(@root, "exe", "ruby-lsp-launcher"), - ) + specification = Gem::Specification.find_by_name("ruby-lsp") + paths = [specification.full_gem_path] + paths.concat(specification.dependencies.map { |dep| dep.to_spec.full_gem_path }) + + load_path = $LOAD_PATH.filter_map do |path| + next unless paths.any? { |gem_path| path.start_with?(gem_path) } || !path.start_with?(Bundler.bundle_path.to_s) + + ["-I", File.expand_path(path)] + end.uniq.flatten + + stdin, stdout, stderr, wait_thr = Open3.popen3(Gem.ruby, *load_path, File.join(@root, "exe", "ruby-lsp-launcher")) stdin.sync = true stdin.binmode stdout.sync = true @@ -180,14 +178,19 @@ def launch(workspace_path) send_message(stdin, { id: 2, method: "shutdown" }) send_message(stdin, { method: "exit" }) - begin - Process.wait(wait_thr.pid) - rescue Errno::ECHILD - nil + # Wait until the process exits + wait_thr.join + + # If the child process failed, it is really difficult to diagnose what's happening unless we read what was printed + # to stderr + unless T.unsafe(wait_thr.value).success? + require "timeout" + + Timeout.timeout(5) do + flunk("Process failed\n#{stderr.read}") + end end - wait_thr.join - refute_predicate(wait_thr, :alive?) assert_path_exists(File.join(workspace_path, ".ruby-lsp", "bundle_gemfile")) assert_path_exists(File.join(workspace_path, ".ruby-lsp", "Gemfile")) assert_path_exists(File.join(workspace_path, ".ruby-lsp", "Gemfile.lock"))