From 5dd3b82e1ff280e894f96bf57c40116c3c1364eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Tue, 20 Apr 2021 20:03:13 +0200 Subject: [PATCH] Don't register named supervisors on remote node (#230) --- .../runtime/erl_dist/evaluator_supervisor.ex | 19 ++++++++++--------- lib/livebook/runtime/erl_dist/manager.ex | 12 +++++++----- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/livebook/runtime/erl_dist/evaluator_supervisor.ex b/lib/livebook/runtime/erl_dist/evaluator_supervisor.ex index 6b239c926..38017e6db 100644 --- a/lib/livebook/runtime/erl_dist/evaluator_supervisor.ex +++ b/lib/livebook/runtime/erl_dist/evaluator_supervisor.ex @@ -8,10 +8,8 @@ defmodule Livebook.Runtime.ErlDist.EvaluatorSupervisor do alias Livebook.Evaluator - @name __MODULE__ - def start_link(opts \\ []) do - DynamicSupervisor.start_link(__MODULE__, opts, name: @name) + DynamicSupervisor.start_link(__MODULE__, opts) end @impl true @@ -22,9 +20,12 @@ defmodule Livebook.Runtime.ErlDist.EvaluatorSupervisor do @doc """ Spawns a new evaluator. """ - @spec start_evaluator() :: {:ok, Evaluator.t()} | {:error, any()} - def start_evaluator() do - case DynamicSupervisor.start_child(@name, {Evaluator, [formatter: Evaluator.StringFormatter]}) do + @spec start_evaluator(pid()) :: {:ok, Evaluator.t()} | {:error, any()} + def start_evaluator(supervisor) do + case DynamicSupervisor.start_child( + supervisor, + {Evaluator, [formatter: Evaluator.StringFormatter]} + ) do {:ok, pid} -> {:ok, pid} {:ok, pid, _} -> {:ok, pid} :ignore -> {:error, :ignore} @@ -35,9 +36,9 @@ defmodule Livebook.Runtime.ErlDist.EvaluatorSupervisor do @doc """ Terminates the given evaluator. """ - @spec terminate_evaluator(Evaluator.t()) :: :ok - def terminate_evaluator(evaluator) do - DynamicSupervisor.terminate_child(@name, evaluator) + @spec terminate_evaluator(pid(), Evaluator.t()) :: :ok + def terminate_evaluator(supervisor, evaluator) do + DynamicSupervisor.terminate_child(supervisor, evaluator) :ok end end diff --git a/lib/livebook/runtime/erl_dist/manager.ex b/lib/livebook/runtime/erl_dist/manager.ex index 61672a943..45aa2fd14 100644 --- a/lib/livebook/runtime/erl_dist/manager.ex +++ b/lib/livebook/runtime/erl_dist/manager.ex @@ -125,9 +125,9 @@ defmodule Livebook.Runtime.ErlDist.Manager do Process.flag(:trap_exit, true) - {:ok, _} = ErlDist.EvaluatorSupervisor.start_link() + {:ok, evaluator_supervisor} = ErlDist.EvaluatorSupervisor.start_link() {:ok, io_forward_gl_pid} = ErlDist.IOForwardGL.start_link() - {:ok, _} = Task.Supervisor.start_link(name: CompletionTaskSupervisor) + {:ok, completion_supervisor} = Task.Supervisor.start_link() # Set `ignore_module_conflict` only for the Manager lifetime. initial_ignore_module_conflict = Code.compiler_options()[:ignore_module_conflict] @@ -143,6 +143,8 @@ defmodule Livebook.Runtime.ErlDist.Manager do %{ owner: nil, evaluators: %{}, + evaluator_supervisor: evaluator_supervisor, + completion_supervisor: completion_supervisor, initial_ignore_module_conflict: initial_ignore_module_conflict, original_standard_error: original_standard_error }} @@ -243,7 +245,7 @@ defmodule Livebook.Runtime.ErlDist.Manager do Evaluator.request_completion_items(evaluator, send_to, ref, hint, evaluation_ref) else # Since there's no evaluator, we may as well get the completion items here. - Task.Supervisor.start_child(CompletionTaskSupervisor, fn -> + Task.Supervisor.start_child(state.completion_supervisor, fn -> binding = [] env = :elixir.env_for_eval([]) items = Livebook.Completion.get_completion_items(hint, binding, env) @@ -258,7 +260,7 @@ defmodule Livebook.Runtime.ErlDist.Manager do if Map.has_key?(state.evaluators, container_ref) do state else - {:ok, evaluator} = ErlDist.EvaluatorSupervisor.start_evaluator() + {:ok, evaluator} = ErlDist.EvaluatorSupervisor.start_evaluator(state.evaluator_supervisor) Process.monitor(evaluator) %{state | evaluators: Map.put(state.evaluators, container_ref, evaluator)} end @@ -267,7 +269,7 @@ defmodule Livebook.Runtime.ErlDist.Manager do defp discard_evaluator(state, container_ref) do case Map.fetch(state.evaluators, container_ref) do {:ok, evaluator} -> - ErlDist.EvaluatorSupervisor.terminate_evaluator(evaluator) + ErlDist.EvaluatorSupervisor.terminate_evaluator(state.evaluator_supervisor, evaluator) %{state | evaluators: Map.delete(state.evaluators, container_ref)} :error ->