From 96bfc76250a568ea313693586adffd2de5249692 Mon Sep 17 00:00:00 2001 From: Mitchell Hanberg Date: Wed, 5 Jul 2023 22:46:38 -0400 Subject: [PATCH] fix: run compiler in task (#95) This lets the compiler stdout messgaes to be logged to the client while the RPC is still happening. Previously, the port would message back to the process that was synchronously waiting on the RPC to finish, so it didn't send any log messages to the LSP process until the compiler was done. This made it look like the initial compiler was hanging. Fixes #60 --- lib/next_ls.ex | 4 +++ lib/next_ls/lsp_supervisor.ex | 2 ++ lib/next_ls/runtime.ex | 46 ++++++++++++++++++++++++++++------- test/next_ls/runtime_test.exs | 26 +++++++++++++++++--- test/next_ls_test.exs | 4 ++- 5 files changed, 69 insertions(+), 13 deletions(-) diff --git a/lib/next_ls.ex b/lib/next_ls.ex index f070b5d5..e79d1720 100644 --- a/lib/next_ls.ex +++ b/lib/next_ls.ex @@ -53,6 +53,7 @@ defmodule NextLS do Keyword.split(args, [ :cache, :task_supervisor, + :runtime_task_supervisor, :dynamic_supervisor, :extensions, :extension_registry, @@ -65,6 +66,7 @@ defmodule NextLS do @impl true def init(lsp, args) do task_supervisor = Keyword.fetch!(args, :task_supervisor) + runtime_task_supervisor = Keyword.fetch!(args, :runtime_task_supervisor) dynamic_supervisor = Keyword.fetch!(args, :dynamic_supervisor) extension_registry = Keyword.fetch!(args, :extension_registry) extensions = Keyword.get(args, :extensions, [NextLS.ElixirExtension]) @@ -81,6 +83,7 @@ defmodule NextLS do logger: logger, symbol_table: symbol_table, task_supervisor: task_supervisor, + runtime_task_supervisor: runtime_task_supervisor, dynamic_supervisor: dynamic_supervisor, extension_registry: extension_registry, extensions: extensions, @@ -272,6 +275,7 @@ defmodule NextLS do DynamicSupervisor.start_child( lsp.assigns.dynamic_supervisor, {NextLS.Runtime, + task_supervisor: lsp.assigns.runtime_task_supervisor, extension_registry: lsp.assigns.extension_registry, working_dir: working_dir, parent: self(), diff --git a/lib/next_ls/lsp_supervisor.ex b/lib/next_ls/lsp_supervisor.ex index 56f371f2..59bd7e66 100644 --- a/lib/next_ls/lsp_supervisor.ex +++ b/lib/next_ls/lsp_supervisor.ex @@ -35,6 +35,7 @@ defmodule NextLS.LSPSupervisor do children = [ {DynamicSupervisor, name: NextLS.DynamicSupervisor}, {Task.Supervisor, name: NextLS.TaskSupervisor}, + {Task.Supervisor, name: :runtime_task_supervisor}, {GenLSP.Buffer, buffer_opts}, {NextLS.DiagnosticCache, name: :diagnostic_cache}, {NextLS.SymbolTable, name: :symbol_table, path: Path.expand(".elixir-tools")}, @@ -43,6 +44,7 @@ defmodule NextLS.LSPSupervisor do cache: :diagnostic_cache, symbol_table: :symbol_table, task_supervisor: NextLS.TaskSupervisor, + runtime_task_supervisor: :runtime_task_supervisor, dynamic_supervisor: NextLS.DynamicSupervisor, extension_registry: NextLS.ExtensionRegistry} ] diff --git a/lib/next_ls/runtime.ex b/lib/next_ls/runtime.ex index 499be534..8c303bba 100644 --- a/lib/next_ls/runtime.ex +++ b/lib/next_ls/runtime.ex @@ -44,6 +44,7 @@ defmodule NextLS.Runtime do working_dir = Keyword.fetch!(opts, :working_dir) parent = Keyword.fetch!(opts, :parent) logger = Keyword.fetch!(opts, :logger) + task_supervisor = Keyword.fetch!(opts, :task_supervisor) extension_registry = Keyword.fetch!(opts, :extension_registry) port = @@ -104,7 +105,16 @@ defmodule NextLS.Runtime do end end) - {:ok, %{port: port, logger: logger, parent: parent, errors: nil, extension_registry: extension_registry}} + {:ok, + %{ + compiler_ref: nil, + port: port, + task_supervisor: task_supervisor, + logger: logger, + parent: parent, + errors: nil, + extension_registry: extension_registry + }} end @impl GenServer @@ -125,19 +135,37 @@ defmodule NextLS.Runtime do {:reply, {:error, :not_ready}, state} end - def handle_call(:compile, _, %{node: node} = state) do - {_, errors} = :rpc.call(node, :_next_ls_private_compiler, :compile, []) + def handle_call(:compile, from, %{node: node} = state) do + task = + Task.Supervisor.async_nolink(state.task_supervisor, fn -> + {_, errors} = :rpc.call(node, :_next_ls_private_compiler, :compile, []) - Registry.dispatch(state.extension_registry, :extension, fn entries -> - for {pid, _} <- entries do - send(pid, {:compiler, errors}) - end - end) + Registry.dispatch(state.extension_registry, :extension, fn entries -> + for {pid, _} <- entries do + send(pid, {:compiler, errors}) + end + end) - {:reply, errors, %{state | errors: errors}} + errors + end) + + {:noreply, %{state | compiler_ref: %{task.ref => from}}} end @impl GenServer + def handle_info({ref, errors}, %{compiler_ref: compiler_ref} = state) when is_map_key(compiler_ref, ref) do + Process.demonitor(ref, [:flush]) + + GenServer.reply(compiler_ref[ref], errors) + + {:noreply, %{state | compiler_ref: nil}} + end + + def handle_info({:DOWN, ref, :process, _pid, _reason}, %{compiler_ref: compiler_ref} = state) + when is_map_key(compiler_ref, ref) do + {:noreply, %{state | compiler_ref: nil}} + end + def handle_info({:node, node}, state) do Node.monitor(node, true) {:noreply, Map.put(state, :node, node)} diff --git a/test/next_ls/runtime_test.exs b/test/next_ls/runtime_test.exs index f8911aeb..bdc6538d 100644 --- a/test/next_ls/runtime_test.exs +++ b/test/next_ls/runtime_test.exs @@ -41,10 +41,16 @@ defmodule NextLs.RuntimeTest do test "returns the response in an ok tuple", %{logger: logger, cwd: cwd} do start_supervised!({Registry, keys: :unique, name: RuntimeTestRegistry}) + tvisor = start_supervised!(Task.Supervisor) pid = start_supervised!( - {Runtime, working_dir: cwd, parent: self(), logger: logger, extension_registry: RuntimeTestRegistry} + {Runtime, + task_supervisor: tvisor, + working_dir: cwd, + parent: self(), + logger: logger, + extension_registry: RuntimeTestRegistry} ) Process.link(pid) @@ -57,9 +63,16 @@ defmodule NextLs.RuntimeTest do test "call returns an error when the runtime is node ready", %{logger: logger, cwd: cwd} do start_supervised!({Registry, keys: :unique, name: RuntimeTestRegistry}) + tvisor = start_supervised!(Task.Supervisor) + pid = start_supervised!( - {Runtime, working_dir: cwd, parent: self(), logger: logger, extension_registry: RuntimeTestRegistry} + {Runtime, + task_supervisor: tvisor, + working_dir: cwd, + parent: self(), + logger: logger, + extension_registry: RuntimeTestRegistry} ) Process.link(pid) @@ -70,10 +83,17 @@ defmodule NextLs.RuntimeTest do test "compiles the code and returns diagnostics", %{logger: logger, cwd: cwd} do start_supervised!({Registry, keys: :unique, name: RuntimeTestRegistry}) + tvisor = start_supervised!(Task.Supervisor) + capture_log(fn -> pid = start_supervised!( - {Runtime, working_dir: cwd, parent: self(), logger: logger, extension_registry: RuntimeTestRegistry} + {Runtime, + task_supervisor: tvisor, + working_dir: cwd, + parent: self(), + logger: logger, + extension_registry: RuntimeTestRegistry} ) Process.link(pid) diff --git a/test/next_ls_test.exs b/test/next_ls_test.exs index 08ab3f3f..fcf16755 100644 --- a/test/next_ls_test.exs +++ b/test/next_ls_test.exs @@ -869,7 +869,8 @@ defmodule NextLSTest do defp with_lsp(%{tmp_dir: tmp_dir}) do root_path = Path.absname(tmp_dir) - tvisor = start_supervised!(Task.Supervisor) + tvisor = start_supervised!(Supervisor.child_spec(Task.Supervisor, id: :one)) + r_tvisor = start_supervised!(Supervisor.child_spec(Task.Supervisor, id: :two)) rvisor = start_supervised!({DynamicSupervisor, [strategy: :one_for_one]}) start_supervised!({Registry, [keys: :unique, name: Registry.NextLSTest]}) extensions = [NextLS.ElixirExtension] @@ -879,6 +880,7 @@ defmodule NextLSTest do server = server(NextLS, task_supervisor: tvisor, + runtime_task_supervisor: r_tvisor, dynamic_supervisor: rvisor, extension_registry: Registry.NextLSTest, extensions: extensions,