Fix invalid value set in write_manifest_pid (#430)

* properly set pid to write_manifest_pid

previously it was {:ok, pid} | nil, now correctly pid | nil

* reduce test flakyness

* unlink process before killing

* Apply suggestions from code review

Co-authored-by: Jason Axelson <axelson@users.noreply.github.com>

Author: lukaszsamson

apps/language_server/lib/language_server/dialyzer.ex

@@ -125,8 +125,11 @@ defmodule ElixirLS.LanguageServer.Dialyzer do
       if not is_nil(state.build_ref) do
         do_analyze(state)
       else
-        if state.write_manifest_pid, do: Process.exit(state.write_manifest_pid, :kill)
-        pid = Manifest.write(state.root_path, active_plt, mod_deps, md5, warnings, timestamp)
+        maybe_cancel_write_manifest(state)
+
+        {:ok, pid} =
+          Manifest.write(state.root_path, active_plt, mod_deps, md5, warnings, timestamp)
+
         %{state | write_manifest_pid: pid}
       end
 
@@ -187,9 +190,9 @@ defmodule ElixirLS.LanguageServer.Dialyzer do
 
   ## Helpers
 
-  defp do_analyze(%{write_manifest_pid: write_manifest_pid} = state) do
+  defp do_analyze(state) do
     # Cancel writing to the manifest, since we'll end up overwriting it anyway
-    if is_pid(write_manifest_pid), do: Process.exit(write_manifest_pid, :cancelled)
+    maybe_cancel_write_manifest(state)
 
     parent = self()
     analysis_pid = spawn_link(fn -> compile(parent, state) end)
@@ -534,4 +537,11 @@ defmodule ElixirLS.LanguageServer.Dialyzer do
     seconds = :calendar.datetime_to_gregorian_seconds(:calendar.universal_time())
     :calendar.gregorian_seconds_to_datetime(seconds - 1)
   end
+
+  defp maybe_cancel_write_manifest(%{write_manifest_pid: nil}), do: :ok
+
+  defp maybe_cancel_write_manifest(%{write_manifest_pid: pid}) do
+    Process.unlink(pid)
+    Process.exit(pid, :kill)
+  end
 end

apps/language_server/lib/language_server/dialyzer/manifest.ex

@@ -19,7 +19,7 @@ defmodule ElixirLS.LanguageServer.Dialyzer.Manifest do
   end
 
   def write(_, _, _, _, _, nil) do
-    nil
+    {:ok, nil}
   end
 
   def write(root_path, active_plt, mod_deps, md5, warnings, timestamp) do

apps/language_server/test/dialyzer_test.exs

@@ -73,6 +73,7 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
 
         b_uri = SourceFile.path_to_uri("lib/b.ex")
         Server.receive_packet(server, did_open(b_uri, "elixir", 1, b_text))
+        Process.sleep(1500)
         File.write!("lib/b.ex", b_text)
 
         Server.receive_packet(server, did_save(b_uri))
@@ -81,7 +82,8 @@ defmodule ElixirLS.LanguageServer.DialyzerTest do
 
         assert_receive notification("window/logMessage", %{
                          "message" => "[ElixirLS Dialyzer] Analyzing 2 modules: [A, B]"
-                       })
+                       }),
+                       40000
 
         # Stop while we're still capturing logs to avoid log leakage
         GenServer.stop(server)