fix: shutdown runtimes explicitly when LSP shutsdown

This should hopefully help with the tests, as they often spit out "blah blah overlapping partitions blah blah" with regard to the clustered nodes that are started. In test, we start many many nodes all connected to the main node, which is the test application. Copied some code from @zachallaun from #165 Co-authored-by: Zach Allaun <[email protected]>
elixir-tools · Aug 12, 2023 · ca04380 · ca04380
1 parent ebe2ea3
commit ca04380
Show file tree

Hide file tree

Showing 10 changed files with 138 additions and 65 deletions.
diff --git a/.formatter.exs b/.formatter.exs
@@ -12,10 +12,7 @@
   plugins: [Styler],
   inputs: [
     ".formatter.exs",
-    "{config,lib,}/**/*.{ex,exs}",
-    "test/next_ls_test.exs",
-    "test/test_helper.exs",
-    "test/next_ls/**/*.{ex,exs}",
+    "{config,lib,test}/**/*.{ex,exs}",
     "priv/**/*.ex"
   ]
 ]
diff --git a/lib/next_ls.ex b/lib/next_ls.ex
@@ -317,6 +317,10 @@ defmodule NextLS do
   end
 
   def handle_request(%Shutdown{}, lsp) do
+    dispatch(lsp.assigns.registry, :runtimes, fn entries ->
+      for {pid, _} <- entries, do: :ok = Runtime.shutdown(pid)
+    end)
+
     {:reply, nil, assign(lsp, exit_code: 0)}
   end
 

diff --git a/lib/next_ls/runtime.ex b/lib/next_ls/runtime.ex
@@ -9,27 +9,12 @@ defmodule NextLS.Runtime do
   @type mod_fun_arg :: {atom(), atom(), list()}
 
   @spec call(pid(), mod_fun_arg()) :: any()
-  def call(server, mfa), do: GenServer.call(server, {:call, mfa})
+  def call(server, mfa), do: GenServer.call(server, {:call, mfa}, :infinity)
 
-  @spec ready?(pid()) :: boolean()
-  def ready?(server), do: GenServer.call(server, :ready?)
-
-  @spec await(pid(), non_neg_integer()) :: :ok | :timeout
-  def await(server, count \\ 50)
-
-  def await(_server, 0) do
-    :timeout
-  end
-
-  def await(server, count) do
-    if ready?(server) do
-      :ok
-    else
-      Process.sleep(500)
-      await(server, count - 1)
-    end
-  end
+  @spec shutdown(pid()) :: :ok
+  def shutdown(server), do: GenServer.call(server, :shutdown)
 
+  @spec compile(pid()) :: list()
   def compile(server) do
     GenServer.call(server, :compile, :infinity)
   end
@@ -156,6 +141,7 @@ defmodule NextLS.Runtime do
       {:ok,
        %{
          name: name,
+         node: nil,
          compiler_ref: nil,
          port: port,
          task_supervisor: task_supervisor,
@@ -172,15 +158,15 @@ defmodule NextLS.Runtime do
   end
 
   @impl GenServer
-  def handle_call(:ready?, _from, %{node: _node} = state) do
+  def handle_call(:ready?, _from, %{node: nil} = state) do
     {:reply, true, state}
   end
 
   def handle_call(:ready?, _from, state) do
     {:reply, false, state}
   end
 
-  def handle_call({:call, {m, f, a}}, _from, %{node: node} = state) do
+  def handle_call({:call, {m, f, a}}, _from, %{node: node} = state) when not is_nil(node) do
     reply = :rpc.call(node, m, f, a)
     {:reply, {:ok, reply}, state}
   end
@@ -189,18 +175,36 @@ defmodule NextLS.Runtime do
     {:reply, {:error, :not_ready}, state}
   end
 
+  def handle_call(:shutdown, _from, state) do
+    with {:ok, node} <- Map.fetch(state, :node) do
+      Node.disconnect(node)
+    end
+
+    with {:ok, port} <- Map.fetch(state, :port) do
+      Port.close(port)
+    end
+
+    {:reply, :ok, %{state | node: nil, port: nil}}
+  end
+
   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, [])
+        case :rpc.call(node, :_next_ls_private_compiler, :compile, []) do
+          {:badrpc, reason} ->
+            NextLS.Logger.warning(state.logger, "Bad RPC call to node #{node}: #{inspect(reason)}")
 
-        Registry.dispatch(state.registry, :extensions, fn entries ->
-          for {pid, _} <- entries do
-            send(pid, {:compiler, errors})
-          end
-        end)
+            []
 
-        errors
+          {_, errors} ->
+            Registry.dispatch(state.registry, :extensions, fn entries ->
+              for {pid, _} <- entries do
+                send(pid, {:compiler, errors})
+              end
+            end)
+
+            errors
+        end
       end)
 
     {:noreply, %{state | compiler_ref: %{task.ref => from}}}
@@ -236,6 +240,23 @@ defmodule NextLS.Runtime do
     {:noreply, state}
   end
 
+  def handle_info({port, other}, %{port: port} = state) do
+    NextLS.Logger.log(state.logger, other)
+    {:noreply, state}
+  end
+
+  def handle_info(msg, state) do
+    NextLS.Logger.log(state.logger, """
+    Runtime process for #{state.name} received message:
+
+    #{inspect(msg)}.
+
+    You can safely ignore this log message.
+    """)
+
+    {:noreply, state}
+  end
+
   defp connect(_node, _port, 0) do
     false
   end

diff --git a/test/next_ls/definition_test.exs b/test/next_ls/definition_test.exs
@@ -89,6 +89,8 @@ defmodule NextLS.DefinitionTest do
         },
         "uri" => ^uri
       }
+
+      shutdown(client)
     end
 
     test "go to imported function definition", %{client: client, bar: bar, imported: imported} = context do
@@ -125,6 +127,8 @@ defmodule NextLS.DefinitionTest do
         },
         "uri" => ^uri
       }
+
+      shutdown(client)
     end
 
     test "go to remote function definition", %{client: client, bar: bar, remote: remote} = context do
@@ -161,6 +165,8 @@ defmodule NextLS.DefinitionTest do
         },
         "uri" => ^uri
       }
+
+      shutdown(client)
     end
   end
 
@@ -255,6 +261,8 @@ defmodule NextLS.DefinitionTest do
         },
         "uri" => ^uri
       }
+
+      shutdown(client)
     end
 
     test "go to imported macro definition", %{client: client, bar: bar, imported: imported} = context do
@@ -291,6 +299,8 @@ defmodule NextLS.DefinitionTest do
         },
         "uri" => ^uri
       }
+
+      shutdown(client)
     end
 
     test "go to remote macro definition", %{client: client, bar: bar, remote: remote} = context do
@@ -327,6 +337,8 @@ defmodule NextLS.DefinitionTest do
         },
         "uri" => ^uri
       }
+
+      shutdown(client)
     end
   end
 
@@ -398,8 +410,9 @@ defmodule NextLS.DefinitionTest do
                         }
                       },
                       "uri" => ^uri
-                    },
-                    500
+                    }
+
+      shutdown(client)
     end
   end
 end
diff --git a/test/next_ls/diagnostics_test.exs b/test/next_ls/diagnostics_test.exs
@@ -112,6 +112,8 @@ defmodule NextLS.DiagnosticsTest do
           }
         ]
       }
+
+      shutdown(client)
     end
   end
 end
diff --git a/test/next_ls/references_test.exs b/test/next_ls/references_test.exs
@@ -102,5 +102,7 @@ defmodule NextLS.ReferencesTest do
                       }
                     }
                   ]
+
+    shutdown(client)
   end
 end
diff --git a/test/next_ls/runtime_test.exs b/test/next_ls/runtime_test.exs
@@ -48,7 +48,7 @@ defmodule NextLs.RuntimeTest do
     tvisor = start_supervised!(Task.Supervisor)
 
     pid =
-      start_supervised!(
+      start_link_supervised!(
         {Runtime,
          name: "my_proj",
          on_initialized: on_init,
@@ -61,8 +61,6 @@ defmodule NextLs.RuntimeTest do
          registry: RuntimeTest.Registry}
       )
 
-    Process.link(pid)
-
     assert wait_for_ready()
 
     assert {:ok, "\"hi\""} = Runtime.call(pid, {Kernel, :inspect, ["hi"]})
@@ -74,7 +72,7 @@ defmodule NextLs.RuntimeTest do
     tvisor = start_supervised!(Task.Supervisor)
 
     pid =
-      start_supervised!(
+      start_link_supervised!(
         {Runtime,
          task_supervisor: tvisor,
          name: "my_proj",
@@ -87,8 +85,6 @@ defmodule NextLs.RuntimeTest do
          registry: RuntimeTest.Registry}
       )
 
-    Process.link(pid)
-
     assert {:error, :not_ready} = Runtime.call(pid, {IO, :puts, ["hi"]})
   end
 
@@ -146,6 +142,38 @@ defmodule NextLs.RuntimeTest do
     end) =~ "Connected to node"
   end
 
+  test "shuts down the node and port", %{logger: logger, cwd: cwd, on_init: on_init} do
+    start_supervised!({Registry, keys: :duplicate, name: RuntimeTest.Registry})
+
+    tvisor = start_supervised!(Task.Supervisor)
+
+    pid =
+      start_link_supervised!(
+        {Runtime,
+         name: "my_proj",
+         on_initialized: on_init,
+         task_supervisor: tvisor,
+         working_dir: cwd,
+         uri: "file://#{cwd}",
+         parent: self(),
+         logger: logger,
+         db: :some_db,
+         registry: RuntimeTest.Registry}
+      )
+
+    assert wait_for_ready()
+
+    state = :sys.get_state(pid)
+
+    assert state.node in Node.list()
+    assert Port.info(state.port)
+
+    assert :ok == Runtime.shutdown(pid)
+
+    assert state.node not in Node.list()
+    refute Port.info(state.port)
+  end
+
   defp wait_for_ready do
     receive do
       :ready -> true

diff --git a/test/next_ls/workspaces_test.exs b/test/next_ls/workspaces_test.exs
@@ -65,6 +65,7 @@ defmodule NextLS.WorkspacesTest do
 
     assert_is_ready(context, "proj_two")
     assert_notification "window/logMessage", %{"message" => "[NextLS] Compiled!"}
+    shutdown(client)
   end
 
   @tag root_paths: ["proj_one", "proj_two"]
@@ -94,6 +95,8 @@ defmodule NextLS.WorkspacesTest do
     assert_notification "window/logMessage", %{
       "message" => ^message
     }
+
+    shutdown(client)
   end
 
   @tag root_paths: ["proj_one"]
@@ -125,6 +128,7 @@ defmodule NextLS.WorkspacesTest do
 
     assert_is_ready(context, "proj_one")
     assert_notification "window/logMessage", %{"message" => "[NextLS] Compiled!"}
+    shutdown(client)
   end
 
   @tag root_paths: ["proj_one"]
@@ -141,5 +145,7 @@ defmodule NextLS.WorkspacesTest do
       jsonrpc: "2.0",
       params: %{changes: [%{type: 3, uri: "file://#{Path.join(cwd, "proj_one/lib/peace.ex")}"}]}
     })
+
+    shutdown(client)
   end
 end
-Original file line number
+Diff line change
@@ Expand Up / @@ -112,6 +112,8 @@ defmodule NextLS.DiagnosticsTest do @@
               }
             ]
           }
+          shutdown(client)
         end
       end
     end