From 3c8d194664ce85b24c568dd547b094ff87681b21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Mon, 21 Oct 2024 22:44:03 +0800 Subject: [PATCH 1/3] Implement mix listener to respect concurrent compilations --- installer/templates/phx_single/mix.exs | 3 +- .../phx_umbrella/apps/app_name_web/mix.exs | 3 +- lib/phoenix/code_reloader.ex | 17 +++ lib/phoenix/code_reloader/mix_listener.ex | 72 +++++++++ lib/phoenix/code_reloader/server.ex | 137 ++++++++++++++---- 5 files changed, 203 insertions(+), 29 deletions(-) create mode 100644 lib/phoenix/code_reloader/mix_listener.ex diff --git a/installer/templates/phx_single/mix.exs b/installer/templates/phx_single/mix.exs index 60fe5b2677..8282523114 100644 --- a/installer/templates/phx_single/mix.exs +++ b/installer/templates/phx_single/mix.exs @@ -13,7 +13,8 @@ defmodule <%= @app_module %>.MixProject do elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, aliases: aliases(), - deps: deps() + deps: deps(), + listeners: [Phoenix.CodeReloader] ] end diff --git a/installer/templates/phx_umbrella/apps/app_name_web/mix.exs b/installer/templates/phx_umbrella/apps/app_name_web/mix.exs index 12f347b4df..620aa84ec4 100644 --- a/installer/templates/phx_umbrella/apps/app_name_web/mix.exs +++ b/installer/templates/phx_umbrella/apps/app_name_web/mix.exs @@ -13,7 +13,8 @@ defmodule <%= @web_namespace %>.MixProject do elixirc_paths: elixirc_paths(Mix.env()), start_permanent: Mix.env() == :prod, aliases: aliases(), - deps: deps() + deps: deps(), + listeners: [Phoenix.CodeReloader] ] end diff --git a/lib/phoenix/code_reloader.ex b/lib/phoenix/code_reloader.ex index 809d55525b..b5b24a1009 100644 --- a/lib/phoenix/code_reloader.ex +++ b/lib/phoenix/code_reloader.ex @@ -31,6 +31,19 @@ defmodule Phoenix.CodeReloader do This function is a no-op and returns `:ok` if Mix is not available. + The reloader should also be configured as Mix listener in project's + mix.exs file (since Elixir v1.18): + + def project do + [ + ..., + listeners: [Phoenix.CodeReloader] + ] + end + + This way the reloader can notice whenever the project is compiled + concurrently. + ## Options * `:reloadable_args` - additional CLI args to pass to the compiler tasks. @@ -57,6 +70,10 @@ defmodule Phoenix.CodeReloader do @spec sync :: :ok defdelegate sync, to: Phoenix.CodeReloader.Server + @doc false + @spec child_spec(keyword) :: Supervisor.child_spec() + defdelegate child_spec(opts), to: Phoenix.CodeReloader.MixListener + ## Plug @behaviour Plug diff --git a/lib/phoenix/code_reloader/mix_listener.ex b/lib/phoenix/code_reloader/mix_listener.ex new file mode 100644 index 0000000000..f136314ee9 --- /dev/null +++ b/lib/phoenix/code_reloader/mix_listener.ex @@ -0,0 +1,72 @@ +defmodule Phoenix.CodeReloader.MixListener do + @moduledoc false + + use GenServer + + @name __MODULE__ + + @spec start_link(keyword) :: GenServer.on_start() + def start_link(_opts) do + GenServer.start_link(__MODULE__, {}, name: @name) + end + + @spec started? :: boolean() + def started? do + Process.whereis(Phoenix.CodeReloader.MixListener) != nil + end + + @doc """ + Unloads all modules invalidated by external compilations. + + Only reloads modules from the given apps. + """ + @spec purge([atom()]) :: :ok + def purge(apps) do + GenServer.call(@name, {:purge, apps}, :infinity) + end + + @impl true + def init({}) do + {:ok, %{to_purge: %{}}} + end + + @impl true + def handle_call({:purge, apps}, _from, state) do + for app <- apps, modules = state.to_purge[app] do + purge_modules(modules) + end + + {:reply, :ok, %{state | to_purge: %{}}} + end + + @impl true + def handle_info({:modules_compiled, info}, state) do + if info.os_pid == System.pid() do + # Ignore compilations from ourselves, because the modules are + # already updated in memory + {:noreply, state} + else + %{changed: changed, removed: removed} = info.modules_diff + + state = + update_in(state.to_purge[info.app], fn to_purge -> + to_purge = to_purge || MapSet.new() + to_purge = Enum.into(changed, to_purge) + Enum.into(removed, to_purge) + end) + + {:noreply, state} + end + end + + def handle_info(_message, state) do + {:noreply, state} + end + + defp purge_modules(modules) do + for module <- modules do + :code.purge(module) + :code.delete(module) + end + end +end diff --git a/lib/phoenix/code_reloader/server.ex b/lib/phoenix/code_reloader/server.ex index 8f56f423c4..f1e822c156 100644 --- a/lib/phoenix/code_reloader/server.ex +++ b/lib/phoenix/code_reloader/server.ex @@ -63,23 +63,39 @@ defmodule Phoenix.CodeReloader.Server do apps = endpoint.config(:reloadable_apps) || default_reloadable_apps() args = Keyword.get(opts, :reloadable_args, ["--no-all-warnings"]) - # We do a backup of the endpoint in case compilation fails. - # If so we can bring it back to finish the request handling. - backup = load_backup(endpoint) froms = all_waiting([from], endpoint) - {res, out} = - proxy_io(fn -> - try do - mix_compile(Code.ensure_loaded(Mix.Task), compilers, apps, args, state.timestamp) - catch - :exit, {:shutdown, 1} -> - :error - - kind, reason -> - IO.puts(Exception.format(kind, reason, __STACKTRACE__)) - :error - end + {backup, res, out} = + with_build_lock(fn -> + purge_fallback? = + if Phoenix.CodeReloader.MixListener.started?() do + Phoenix.CodeReloader.MixListener.purge(apps) + false + else + warn_missing_mix_listener() + true + end + + # We do a backup of the endpoint in case compilation fails. + # If so we can bring it back to finish the request handling. + backup = load_backup(endpoint) + + {res, out} = + proxy_io(fn -> + try do + task_loaded = Code.ensure_loaded(Mix.Task) + mix_compile(task_loaded, compilers, apps, args, state.timestamp, purge_fallback?) + catch + :exit, {:shutdown, 1} -> + :error + + kind, reason -> + IO.puts(Exception.format(kind, reason, __STACKTRACE__)) + :error + end + end) + + {backup, res, out} end) reply = @@ -175,7 +191,34 @@ defmodule Phoenix.CodeReloader.Server do defp purge_protocols(_path), do: :ok end - defp mix_compile({:module, Mix.Task}, compilers, apps_to_reload, compile_args, timestamp) do + defp warn_missing_mix_listener do + listeners_supported? = Version.match?(System.version(), ">= 1.18.0-dev") + + if listeners_supported? do + IO.warn(""" + a Mix listener expected by Phoenix.CodeReloader is missing. + + Please add the listener to your mix.exs configuration, like so: + + def project do + [ + ..., + listeners: [Phoenix.CodeReloader] + ] + end + + """) + end + end + + defp mix_compile( + {:module, Mix.Task}, + compilers, + apps_to_reload, + compile_args, + timestamp, + purge_fallback? + ) do config = Mix.Project.config() path = Mix.Project.consolidation_path(config) @@ -184,8 +227,25 @@ defmodule Phoenix.CodeReloader.Server do purge_protocols(path) end - mix_compile_deps(Mix.Dep.cached(), apps_to_reload, compile_args, compilers, timestamp, path) - mix_compile_project(config[:app], apps_to_reload, compile_args, compilers, timestamp, path) + mix_compile_deps( + Mix.Dep.cached(), + apps_to_reload, + compile_args, + compilers, + timestamp, + path, + purge_fallback? + ) + + mix_compile_project( + config[:app], + apps_to_reload, + compile_args, + compilers, + timestamp, + path, + purge_fallback? + ) if config[:consolidate_protocols] do # If we are consolidating protocols, we need to purge all of its modules @@ -198,30 +258,46 @@ defmodule Phoenix.CodeReloader.Server do :ok end - defp mix_compile({:error, _reason}, _, _, _, _) do + defp mix_compile({:error, _reason}, _, _, _, _, _) do raise "the Code Reloader is enabled but Mix is not available. If you want to " <> "use the Code Reloader in production or inside an escript, you must add " <> ":mix to your applications list. Otherwise, you must disable code reloading " <> "in such environments" end - defp mix_compile_deps(deps, apps_to_reload, compile_args, compilers, timestamp, path) do + defp mix_compile_deps( + deps, + apps_to_reload, + compile_args, + compilers, + timestamp, + path, + purge_fallback? + ) do for dep <- deps, dep.app in apps_to_reload do Mix.Dep.in_dependency(dep, fn _ -> - mix_compile_unless_stale_config(compilers, compile_args, timestamp, path) + mix_compile_unless_stale_config(compilers, compile_args, timestamp, path, purge_fallback?) end) end end - defp mix_compile_project(nil, _, _, _, _, _), do: :ok - - defp mix_compile_project(app, apps_to_reload, compile_args, compilers, timestamp, path) do + defp mix_compile_project(nil, _, _, _, _, _, _), do: :ok + + defp mix_compile_project( + app, + apps_to_reload, + compile_args, + compilers, + timestamp, + path, + purge_fallback? + ) do if app in apps_to_reload do - mix_compile_unless_stale_config(compilers, compile_args, timestamp, path) + mix_compile_unless_stale_config(compilers, compile_args, timestamp, path, purge_fallback?) end end - defp mix_compile_unless_stale_config(compilers, compile_args, timestamp, path) do + defp mix_compile_unless_stale_config(compilers, compile_args, timestamp, path, purge_fallback?) do manifests = Mix.Tasks.Compile.Elixir.manifests() configs = Mix.Project.config_files() config = Mix.Project.config() @@ -230,7 +306,7 @@ defmodule Phoenix.CodeReloader.Server do [] -> # If the manifests are more recent than the timestamp, # someone updated this app behind the scenes, so purge all beams. - if Mix.Utils.stale?(manifests, [timestamp]) do + if purge_fallback? and Mix.Utils.stale?(manifests, [timestamp]) do purge_modules(Path.join(Mix.Project.app_path(config), "ebin")) end @@ -351,4 +427,11 @@ defmodule Phoenix.CodeReloader.Server do Logger.configure(compile_time_application: logger_config_app) end end + + # TODO: remove once we depend on Elixir 1.18 + if Code.ensure_loaded?(Mix.Project) and function_exported?(Mix.Project, :with_build_lock, 1) do + defp with_build_lock(fun), do: Mix.Project.with_build_lock(fun) + else + defp with_build_lock(fun), do: fun.() + end end From d4fcb1b2e2b3347e7978e21cd4d2107c70cdc8f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Mon, 21 Oct 2024 16:58:30 +0200 Subject: [PATCH 2/3] Update lib/phoenix/code_reloader.ex --- lib/phoenix/code_reloader.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/phoenix/code_reloader.ex b/lib/phoenix/code_reloader.ex index b5b24a1009..433cc75476 100644 --- a/lib/phoenix/code_reloader.ex +++ b/lib/phoenix/code_reloader.ex @@ -31,7 +31,7 @@ defmodule Phoenix.CodeReloader do This function is a no-op and returns `:ok` if Mix is not available. - The reloader should also be configured as Mix listener in project's + The reloader should also be configured as a Mix listener in project's mix.exs file (since Elixir v1.18): def project do From f4fab1c65903a5ad57380cacc5c92179edab5796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Mon, 21 Oct 2024 17:00:44 +0200 Subject: [PATCH 3/3] Update lib/phoenix/code_reloader/server.ex --- lib/phoenix/code_reloader/server.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/phoenix/code_reloader/server.ex b/lib/phoenix/code_reloader/server.ex index f1e822c156..16ff92bf19 100644 --- a/lib/phoenix/code_reloader/server.ex +++ b/lib/phoenix/code_reloader/server.ex @@ -306,6 +306,7 @@ defmodule Phoenix.CodeReloader.Server do [] -> # If the manifests are more recent than the timestamp, # someone updated this app behind the scenes, so purge all beams. + # TODO: remove once we depend on Elixir 1.18 if purge_fallback? and Mix.Utils.stale?(manifests, [timestamp]) do purge_modules(Path.join(Mix.Project.app_path(config), "ebin")) end