Skip to content

Commit

Permalink
fix: run compiler in task (#95)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mhanberg committed Jul 6, 2023
1 parent 32f8313 commit 96bfc76
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 13 deletions.
4 changes: 4 additions & 0 deletions lib/next_ls.ex
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ defmodule NextLS do
Keyword.split(args, [
:cache,
:task_supervisor,
:runtime_task_supervisor,
:dynamic_supervisor,
:extensions,
:extension_registry,
Expand All @@ -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])
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
2 changes: 2 additions & 0 deletions lib/next_ls/lsp_supervisor.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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")},
Expand All @@ -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}
]
Expand Down
46 changes: 37 additions & 9 deletions lib/next_ls/runtime.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand All @@ -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)}
Expand Down
26 changes: 23 additions & 3 deletions test/next_ls/runtime_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -70,10 +83,17 @@ defmodule NextLs.RuntimeTest do
test "compiles the code and returns diagnostics", %{logger: logger, cwd: cwd} do

Check failure on line 83 in test/next_ls/runtime_test.exs

View workflow job for this annotation

GitHub Actions / Test (1.14.x/26.x)

test compiles the code and returns diagnostics (NextLs.RuntimeTest)
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)
Expand Down
4 changes: 3 additions & 1 deletion test/next_ls_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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,
Expand Down

0 comments on commit 96bfc76

Please sign in to comment.