fix: stop calls to IO and other writes to stdout from crashing Expert.

apps/engine/mix.exs

@@ -51,7 +51,9 @@ defmodule Engine.MixProject do
       {:elixir_sense,
        github: "elixir-lsp/elixir_sense", ref: "da065ae9ccc125d05b901b9eb6981ff559a8f9f1"},
       {:forge, path: "../forge"},
-      {:gen_lsp, "~> 0.11.3"},
+      # Fork carrying the `:device` option that Expert.Stdio isolation needs.
+      {:gen_lsp,
+       github: "Moosieus/gen_lsp", branch: "cam/pluggable-stdio-device", override: true},
       {:logger_backends, "~> 1.0"},
       {:patch, "~> 0.15", only: [:dev, :test], runtime: false},
       {:quokka, "~> 2.12", only: [:dev, :test], runtime: false},

apps/engine/mix.lock

@@ -9,7 +9,7 @@
   "erlex": {:hex, :erlex, "0.2.9", "7debbbaa9f4f368b8cd648983e0f1d7963028508e9c59e9d4ed504e94ef52a55", [:mix], [], "hexpm", "8cfffc0ec7159e6d73de2ab28a588064de80f88b2798d5cbe4482cbbc200178b"},
   "ex_nar": {:hex, :ex_nar, "0.3.0", "3136d019241afc97a5da8e43613a56ac0ae30dad12df5d3bc66547e55e7453e0", [:mix], [], "hexpm", "cbb42d047764feac6c411efddcadc31866e9a998dd6e2bc1eb428cec1c49fdcd"},
   "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"},
-  "gen_lsp": {:hex, :gen_lsp, "0.11.3", "b530024296091531a8968234178f926fbb07a5453b2612ece7ef9b654bf90bc0", [:mix], [{:jason, "~> 1.3", [hex: :jason, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.5 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:schematic, "~> 0.2.1", [hex: :schematic, repo: "hexpm", optional: false]}, {:typed_struct, "~> 0.3.0", [hex: :typed_struct, repo: "hexpm", optional: false]}], "hexpm", "a674de4b06cbc56311d13a0fdf69066837785ba9c90da54984e58f485fd019cb"},
+  "gen_lsp": {:git, "https://github.com/Moosieus/gen_lsp.git", "c95ef3d768d62f1477ae8aa7fcc2bd74d0237232", [branch: "cam/pluggable-stdio-device"]},
   "hpax": {:hex, :hpax, "1.0.4", "777de5d433b0fbdc7c418159c8055910faa8047ffdb3d6b31098d2a46cd7685c", [:mix], [], "hexpm", "afc7cb142ebcc2d01ce7816190b98ce5dd49e799111b24249f3443d730f377ca"},
   "jason": {:hex, :jason, "1.4.5", "2e3a008590b0b8d7388c20293e9dcc9cf3e5d642fd2a114e4cbbb52e595d940a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b0c823996102bcd0239b3c2444eb00409b72f6a140c1950bc8b457d836b30684"},
   "logger_backends": {:hex, :logger_backends, "1.0.0", "09c4fad6202e08cb0fbd37f328282f16539aca380f512523ce9472b28edc6bdf", [:mix], [], "hexpm", "1faceb3e7ec3ef66a8f5746c5afd020e63996df6fd4eb8cdb789e5665ae6c9ce"},
@@ -31,7 +31,7 @@
   "sourceror": {:hex, :sourceror, "1.12.2", "85bfd48159f020c0cbfc72f289f11456fdc05dc43719b6f2589fb969faefa113", [:mix], [], "hexpm", "da37d3da09c5b890528802c7056a8f585a061973820d7656b6e3649c14f0e9cb"},
   "spitfire": {:hex, :spitfire, "0.3.13", "edd207b065eaec57acc5484097d0aa3e97fe4246168c54e67a9af040b8dee4c1", [:mix], [], "hexpm", "3601be88ceed4967b584e96444de3e1d12d6555ae0864a7390b9cd5332d134b4"},
   "stream_data": {:hex, :stream_data, "1.2.0", "58dd3f9e88afe27dc38bef26fce0c84a9e7a96772b2925c7b32cd2435697a52b", [:mix], [], "hexpm", "eb5c546ee3466920314643edf68943a5b14b32d1da9fe01698dc92b73f89a9ed"},
-  "telemetry": {:hex, :telemetry, "1.3.0", "fedebbae410d715cf8e7062c96a1ef32ec22e764197f70cda73d82778d61e7a2", [:rebar3], [], "hexpm", "7015fc8919dbe63764f4b4b87a95b7c0996bd539e0d499be6ec9d7f3875b79e6"},
+  "telemetry": {:hex, :telemetry, "1.4.2", "a0cb522801dffb1c49fe6e30561badffc7b6d0e180db1300df759faa22062855", [:rebar3], [], "hexpm", "928f6495066506077862c0d1646609eed891a4326bee3126ba54b60af61febb1"},
   "toxic2": {:git, "https://github.com/lukaszsamson/toxic2.git", "c47c911dac5aafa860206d222a781c3d71afc843", [ref: "c47c911dac5aafa860206d222a781c3d71afc843"]},
   "typed_struct": {:hex, :typed_struct, "0.3.0", "939789e3c1dca39d7170c87f729127469d1315dcf99fee8e152bb774b17e7ff7", [:mix], [], "hexpm", "c50bd5c3a61fe4e198a8504f939be3d3c85903b382bde4865579bc23111d1b6d"},
   "websock": {:hex, :websock, "0.5.3", "2f69a6ebe810328555b6fe5c831a851f485e303a7c8ce6c5f675abeb20ebdadc", [:mix], [], "hexpm", "6105453d7fac22c712ad66fab1d45abdf049868f253cf719b625151460b8b453"},

apps/expert/lib/expert/application.ex

@@ -87,10 +87,11 @@ defmodule Expert.Application do
     buffer_opts =
       cond do
         opts[:stdio] ->
+          {:ok, protocol_device} = Expert.StdioRedirect.install()
           :ok = Expert.Logging.ProjectLogFile.attach()
           :ok = mute_default_log_handler()
           Logger.info("Expert v#{Expert.vsn()} starting on stdio")
-          []
+          [communication: {GenLSP.Communication.Stdio, [device: protocol_device]}]
 
         is_integer(opts[:port]) ->
           :ok = Expert.Logging.ProjectLogFile.attach()
@@ -120,7 +121,7 @@ defmodule Expert.Application do
     end
 
     children_spec = children(buffer: buffer_opts)
-    opts = [strategy: :one_for_one, name: Expert.Supervisor]
+    opts = [name: Expert.Supervisor, strategy: :one_for_one, auto_shutdown: :any_significant]
 
     Supervisor.start_link(children_spec, opts)
   end
@@ -136,7 +137,11 @@ defmodule Expert.Application do
       {DynamicSupervisor, name: Expert.DynamicSupervisor},
       {GenLSP.Assigns, [name: Expert.Assigns]},
       {Task.Supervisor, name: :expert_task_queue},
-      {GenLSP.Buffer, [name: Expert.Buffer] ++ buffer_opts},
+      Supervisor.child_spec(
+        {GenLSP.Buffer, [name: Expert.Buffer] ++ buffer_opts},
+        restart: :temporary,
+        significant: true
+      ),
       {Expert.Project.Store, []},
       {Expert.EngineBuilds, []},
       hex_cache_child_spec(),

apps/expert/lib/expert/stdio_redirect.ex

@@ -0,0 +1,70 @@
+defmodule Expert.StdioRedirect do
+  @moduledoc """
+  An IO server used to redirect all would-be `:stdio` writes to `:stderr`, allowing GenLSP exclusive access to `:stdio`.
+  """
+
+  @protocol_device {__MODULE__, :protocol_device}
+
+  @doc """
+  Swaps the boot-time `:user` process (the normal I/O device for `:stdio`) with one that writes
+  output to `:standard_error` instead, and returns the PID for the original `:user`.
+  """
+  @spec install() :: {:ok, pid()} | {:error, term()}
+  def install do
+    case protocol_device() do
+      device when is_pid(device) -> {:ok, device}
+      nil -> do_install()
+    end
+  end
+
+  defp do_install do
+    case Process.whereis(:user) do
+      protocol_device when is_pid(protocol_device) ->
+        redirect = start()
+
+        :io.setopts(protocol_device, binary: true, encoding: :latin1)
+
+        swap_user(redirect)
+        redirect_group_leaders(redirect)
+
+        :persistent_term.put(@protocol_device, protocol_device)
+        {:ok, protocol_device}
+
+      _ ->
+        {:error, :no_user_device}
+    end
+  end
+
+  defp swap_user(redirect) do
+    case Process.whereis(:user) do
+      nil -> :ok
+      _pid -> Process.unregister(:user)
+    end
+
+    Process.register(redirect, :user)
+  end
+
+  defp redirect_group_leaders(redirect) do
+    :erlang.group_leader(redirect, self())
+
+    case Process.whereis(:application_controller) do
+      pid when is_pid(pid) -> :erlang.group_leader(redirect, pid)
+      _ -> :ok
+    end
+  end
+
+  defp protocol_device, do: :persistent_term.get(@protocol_device, nil)
+
+  defp start, do: spawn(fn -> loop() end)
+
+  defp loop do
+    receive do
+      {:io_request, _from, _reply_as, _request} = request ->
+        send(:standard_error, request)
+        loop()
+
+      _ignored ->
+        loop()
+    end
+  end
+end

apps/expert/mix.exs

@@ -120,7 +120,9 @@ defmodule Expert.MixProject do
       {:engine, path: "../engine", only: [:test]},
       {:exqlite, "~> 0.36.0"},
       {:forge, path: "../forge"},
-      {:gen_lsp, "~> 0.11.3"},
+      # Fork carrying the `:device` option that Expert.StdioRedirect needs.
+      {:gen_lsp,
+       github: "Moosieus/gen_lsp", branch: "cam/pluggable-stdio-device", override: true},
       {:hex_core, "~> 0.10"},
       {:jason, "~> 1.4"},
       {:patch, "~> 0.15", runtime: false, only: [:dev, :test]},

apps/expert/mix.lock

@@ -14,7 +14,7 @@
   "exqlite": {:hex, :exqlite, "0.36.0", "07b4f95d61cb82b8d52946d0639497fa7d32117e09b2c8d25e24a38723c295cb", [:make, :mix], [{:cc_precompiler, "~> 0.1", [hex: :cc_precompiler, repo: "hexpm", optional: false]}, {:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.8", [hex: :elixir_make, repo: "hexpm", optional: false]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "cbeca3ce781f9ff07cfa9a87486f3ebd512a143ad6a14ed5c9fca21fe0bf3ae7"},
   "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"},
   "finch": {:hex, :finch, "0.20.0", "5330aefb6b010f424dcbbc4615d914e9e3deae40095e73ab0c1bb0968933cadf", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.6.2 or ~> 1.7", [hex: :mint, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.4 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:nimble_pool, "~> 1.1", [hex: :nimble_pool, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "2658131a74d051aabfcba936093c903b8e89da9a1b63e430bee62045fa9b2ee2"},
-  "gen_lsp": {:hex, :gen_lsp, "0.11.3", "b530024296091531a8968234178f926fbb07a5453b2612ece7ef9b654bf90bc0", [:mix], [{:jason, "~> 1.3", [hex: :jason, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.5 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:schematic, "~> 0.2.1", [hex: :schematic, repo: "hexpm", optional: false]}, {:typed_struct, "~> 0.3.0", [hex: :typed_struct, repo: "hexpm", optional: false]}], "hexpm", "a674de4b06cbc56311d13a0fdf69066837785ba9c90da54984e58f485fd019cb"},
+  "gen_lsp": {:git, "https://github.com/Moosieus/gen_lsp.git", "c95ef3d768d62f1477ae8aa7fcc2bd74d0237232", [branch: "cam/pluggable-stdio-device"]},
   "hex_core": {:hex, :hex_core, "0.15.0", "8eadc0ccb08e3742f2313073d04f39eaa7904617329039e9d3c402f5dd227673", [:rebar3], [], "hexpm", "c2093764c7af8ef0818c104fa141eba431e7be93f8374638c45c7037b26a52f8"},
   "hpax": {:hex, :hpax, "1.0.4", "777de5d433b0fbdc7c418159c8055910faa8047ffdb3d6b31098d2a46cd7685c", [:mix], [], "hexpm", "afc7cb142ebcc2d01ce7816190b98ce5dd49e799111b24249f3443d730f377ca"},
   "jason": {:hex, :jason, "1.4.5", "2e3a008590b0b8d7388c20293e9dcc9cf3e5d642fd2a114e4cbbb52e595d940a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b0c823996102bcd0239b3c2444eb00409b72f6a140c1950bc8b457d836b30684"},
@@ -38,7 +38,7 @@
   "schematic": {:hex, :schematic, "0.2.1", "0b091df94146fd15a0a343d1bd179a6c5a58562527746dadd09477311698dbb1", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "0b255d65921e38006138201cd4263fd8bb807d9dfc511074615cd264a571b3b1"},
   "sourceror": {:hex, :sourceror, "1.12.2", "85bfd48159f020c0cbfc72f289f11456fdc05dc43719b6f2589fb969faefa113", [:mix], [], "hexpm", "da37d3da09c5b890528802c7056a8f585a061973820d7656b6e3649c14f0e9cb"},
   "spitfire": {:hex, :spitfire, "0.3.13", "edd207b065eaec57acc5484097d0aa3e97fe4246168c54e67a9af040b8dee4c1", [:mix], [], "hexpm", "3601be88ceed4967b584e96444de3e1d12d6555ae0864a7390b9cd5332d134b4"},
-  "telemetry": {:hex, :telemetry, "1.3.0", "fedebbae410d715cf8e7062c96a1ef32ec22e764197f70cda73d82778d61e7a2", [:rebar3], [], "hexpm", "7015fc8919dbe63764f4b4b87a95b7c0996bd539e0d499be6ec9d7f3875b79e6"},
+  "telemetry": {:hex, :telemetry, "1.4.2", "a0cb522801dffb1c49fe6e30561badffc7b6d0e180db1300df759faa22062855", [:rebar3], [], "hexpm", "928f6495066506077862c0d1646609eed891a4326bee3126ba54b60af61febb1"},
   "toxic2": {:git, "https://github.com/lukaszsamson/toxic2.git", "c47c911dac5aafa860206d222a781c3d71afc843", [ref: "c47c911dac5aafa860206d222a781c3d71afc843"]},
   "typed_struct": {:hex, :typed_struct, "0.3.0", "939789e3c1dca39d7170c87f729127469d1315dcf99fee8e152bb774b17e7ff7", [:mix], [], "hexpm", "c50bd5c3a61fe4e198a8504f939be3d3c85903b382bde4865579bc23111d1b6d"},
   "websock": {:hex, :websock, "0.5.3", "2f69a6ebe810328555b6fe5c831a851f485e303a7c8ce6c5f675abeb20ebdadc", [:mix], [], "hexpm", "6105453d7fac22c712ad66fab1d45abdf049868f253cf719b625151460b8b453"},

apps/expert/test/expert/stdio_redirect_test.exs

@@ -0,0 +1,188 @@
+defmodule Expert.StdioRedirectTest do
+  use ExUnit.Case, async: false
+
+  @protocol "PROTOCOL_SENTINEL_ON_STDOUT"
+
+  describe "install/0" do
+    test "protocol writes reach the device; every rogue write reaches stderr" do
+      # Each entry writes its sentinel through a different rogue path. After isolation, none may
+      # reach the protocol device (stdout); all must reach stderr.
+      sentinels = ~w(
+        ROGUE_io_puts
+        ROGUE_io_write
+        ROGUE_io_inspect
+        ROGUE_io_puts_user
+        ROGUE_io_puts_stdio
+        ROGUE_io_puts_standard_io
+        ROGUE_erlang_io_format
+        ROGUE_dbg
+        ROGUE_spawned_process
+        ROGUE_task
+      )
+
+      {stdout, stderr} =
+        run_in_child("""
+        IO.puts("ROGUE_io_puts")
+        IO.write("ROGUE_io_write\\n")
+        IO.inspect(:ROGUE_io_inspect)
+        IO.puts(:user, "ROGUE_io_puts_user")
+        IO.puts(:stdio, "ROGUE_io_puts_stdio")
+        IO.puts(:standard_io, "ROGUE_io_puts_standard_io")
+        :io.format("~s~n", ["ROGUE_erlang_io_format"])
+
+        value = "ROGUE_dbg"
+        dbg(value)
+
+        parent = self()
+        spawn(fn -> IO.puts("ROGUE_spawned_process"); send(parent, :spawned_done) end)
+        receive do :spawned_done -> :ok after 2000 -> :ok end
+
+        Task.async(fn -> IO.puts("ROGUE_task") end) |> Task.await()
+
+        # install/0 is idempotent, and the protocol channel still works.
+        {:ok, ^device} = Expert.StdioRedirect.install()
+        IO.binwrite(device, "#{@protocol}")
+        """)
+
+      assert stdout =~ @protocol, "protocol output missing from stdout"
+      refute stderr =~ @protocol, "protocol output leaked onto stderr"
+
+      for sentinel <- sentinels do
+        refute stdout =~ sentinel, "#{sentinel} leaked onto the protocol channel (stdout)"
+        assert stderr =~ sentinel, "#{sentinel} did not reach stderr"
+      end
+    end
+
+    test "stray IO on a remote node (over distribution) lands on this node's stderr" do
+      {stdout, stderr} =
+        run_in_child("""
+        # Globally-unique names: :erlang.unique_integer resets per VM, so a
+        # parallel matrix sharing a host/epmd would otherwise collide on `mgr1@`.
+        suffix = "\#{System.pid()}_\#{:erlang.unique_integer([:positive])}"
+        peer = :"peer\#{suffix}@127.0.0.1"
+
+        result =
+          try do
+            if match?({:ok, _}, Node.start(:"mgr\#{suffix}@127.0.0.1", :longnames)) do
+              Node.set_cookie(:expert_isolation_test_cookie)
+
+              Port.open({:spawn_executable, System.find_executable("elixir")}, [
+                :binary, :exit_status, :stderr_to_stdout, line: 65536,
+                args: ["--name", Atom.to_string(peer), "--cookie", "expert_isolation_test_cookie",
+                       "--no-halt", "-e", "Process.sleep(:infinity)"]
+              ])
+
+              connected? =
+                Enum.reduce_while(1..80, false, fn _, _ ->
+                  if Node.connect(peer) == true and peer in Node.list() do
+                    {:halt, true}
+                  else
+                    Process.sleep(200)
+                    {:cont, false}
+                  end
+                end)
+
+              if connected? do
+                # Runs on the peer; the worker inherits our (redirected) group leader.
+                :erpc.call(peer, IO, :puts, ["REMOTE_STRAY_SENTINEL"])
+                :erpc.cast(peer, System, :halt, [0])
+                :ok
+              else
+                :setup_failed
+              end
+            else
+              :setup_failed
+            end
+          rescue
+            _ -> :setup_failed
+          end
+
+        case result do
+          :ok -> IO.binwrite(device, "#{@protocol}")
+          :setup_failed -> IO.binwrite(device, "SETUP_UNAVAILABLE")
+        end
+        """)
+
+      if stdout =~ "SETUP_UNAVAILABLE" do
+        # No peer node here (Windows, or epmd/distribution flakiness); the redirect itself is
+        # covered by the local test, so don't fail on it.
+        IO.puts(:stderr, "[stdio_redirect] distributed case skipped: no peer node available")
+      else
+        assert stdout =~ @protocol
+
+        refute stdout =~ "REMOTE_STRAY_SENTINEL",
+               "remote-node stray IO leaked onto the protocol channel (this is the original bug)"
+
+        assert stderr =~ "REMOTE_STRAY_SENTINEL",
+               "remote-node stray IO did not reach the manager's stderr"
+      end
+    end
+  end
+
+  defp run_in_child(body) do
+    tmp = System.tmp_dir!()
+    # Qualify with the OS pid so parallel matrix jobs sharing /tmp don't collide.
+    suffix = "#{System.pid()}_#{:erlang.unique_integer([:positive])}"
+    script_path = Path.join(tmp, "stdio_redirect_script_#{suffix}.exs")
+    result_path = Path.join(tmp, "stdio_redirect_result_#{suffix}.bin")
+
+    script = """
+    {:ok, stdout_io} = StringIO.open("")
+    {:ok, stderr_io} = StringIO.open("")
+    Process.unregister(:user)
+    Process.register(stdout_io, :user)
+    Process.unregister(:standard_error)
+    Process.register(stderr_io, :standard_error)
+
+    {:ok, device} = Expert.StdioRedirect.install()
+
+    result =
+      try do
+        #{body}
+
+        {_, out} = StringIO.contents(stdout_io)
+        {_, err} = StringIO.contents(stderr_io)
+        {:captured, out, err}
+      rescue
+        e -> {:child_error, Exception.format(:error, e, __STACKTRACE__)}
+      end
+
+    File.write!(#{inspect(result_path)}, :erlang.term_to_binary(result))
+    """
+
+    File.write!(script_path, script)
+    on_exit(fn -> File.rm(script_path) end)
+    on_exit(fn -> File.rm(result_path) end)
+
+    {output, status} = run_elixir(script_path)
+
+    if not File.exists?(result_path) do
+      flunk("child produced no result file (exit #{status})\n--- child output ---\n#{output}")
+    end
+
+    result_path
+    |> File.read!()
+    |> :erlang.binary_to_term()
+    |> case do
+      {:captured, out, err} -> {out, err}
+      {:child_error, message} -> flunk("child raised:\n#{message}")
+    end
+  end
+
+  defp run_elixir(script_path) do
+    elixir = System.find_executable("elixir")
+
+    pa =
+      [Mix.Project.build_path(), "lib", "*", "ebin"]
+      |> Path.join()
+      |> Path.wildcard()
+      |> Enum.flat_map(&["-pa", &1])
+
+    args = pa ++ [script_path]
+
+    case :os.type() do
+      {:win32, _} -> System.cmd("cmd", ["/c", elixir | args], stderr_to_stdout: true)
+      _ -> System.cmd(elixir, args, stderr_to_stdout: true)
+    end
+  end
+end