[fix] Explicitly expire child processes of a Task.Supervisor (#1873)

* Explicitly expire child processes of a Task.Supervisor

* Also make a fallback exit for dynamicsupervisor

* To truly exit process, must kill with fire

* If process fails to start, handle it more gracefully

lib/screens/ol_crowding/dynamic_supervisor.ex

@@ -1,8 +1,10 @@
 defmodule Screens.OlCrowding.DynamicSupervisor do
   @moduledoc false
 
+  require Logger
+
   use DynamicSupervisor
-  alias Screens.OlCrowding.Logger
+  alias Screens.OlCrowding.LogCrowdingInfo
 
   def start_link(init_arg) do
     DynamicSupervisor.start_link(__MODULE__, init_arg, name: __MODULE__)
@@ -27,9 +29,9 @@ defmodule Screens.OlCrowding.DynamicSupervisor do
         fetch_params
       ) do
     spec = %{
-      id: Logger,
+      id: LogCrowdingInfo,
       start:
-        {Logger, :start_link,
+        {LogCrowdingInfo, :start_link,
          [
            %{
              original_crowding_levels: original_crowding_levels,
@@ -48,6 +50,18 @@ defmodule Screens.OlCrowding.DynamicSupervisor do
       restart: :transient
     }
 
-    DynamicSupervisor.start_child(__MODULE__, spec)
+    case DynamicSupervisor.start_child(__MODULE__, spec) do
+      {:ok, child_pid} ->
+        _ = :timer.exit_after(10_000, child_pid, :kill)
+
+      {:ok, child_pid, _} ->
+        _ = :timer.exit_after(10_000, child_pid, :kill)
+
+      {:error, error} ->
+        Logger.error("crowding_dyn_supervisor_process_error #{inspect(error)}")
+
+      _ ->
+        Logger.warn("Something went wrong with starting the crowding dynamic supervisor process")
+    end
   end
 end

lib/screens/ol_crowding/logger.ex

@@ -1,4 +1,4 @@
-defmodule Screens.OlCrowding.Logger do
+defmodule Screens.OlCrowding.LogCrowdingInfo do
   @moduledoc false
 
   require Logger

lib/screens/screens_by_alert/memcache.ex

@@ -27,6 +27,7 @@ defmodule Screens.ScreensByAlert.Memcache do
   ```
   """
   alias Screens.ScreensByAlert.Memcache.TaskSupervisor
+  alias Screens.Util
 
   require Logger
 
@@ -56,10 +57,16 @@ defmodule Screens.ScreensByAlert.Memcache do
 
     # To avoid bottlenecks and unnecessarily blocking the caller, run in a separate task process
     _ =
-      Task.Supervisor.start_child(TaskSupervisor, fn ->
-        update_screens_last_updated_key(screen_id, now)
-        Enum.each(alert_ids, &update_alert_key(&1, screen_id, now))
-      end)
+      Task.Supervisor.start_child(
+        TaskSupervisor,
+        Util.fn_with_timeout(
+          fn ->
+            update_screens_last_updated_key(screen_id, now)
+            Enum.each(alert_ids, &update_alert_key(&1, screen_id, now))
+          end,
+          10_000
+        )
+      )
 
     :ok
   end

lib/screens/screens_by_alert/self_refresh_runner.ex

@@ -6,6 +6,7 @@ defmodule Screens.ScreensByAlert.SelfRefreshRunner do
 
   alias Screens.Config.Screen
   alias Screens.ScreensByAlert
+  alias Screens.Util
 
   # (Not a real module--just a name assigned to the Task.Supervisor process that supervises each simulated data request run)
   alias Screens.ScreensByAlert.SelfRefreshRunner.TaskSupervisor
@@ -70,9 +71,15 @@ defmodule Screens.ScreensByAlert.SelfRefreshRunner do
     # process from going down if an exception is raised while running
     # ScreenData.by_screen_id/1 for some screen.
     Enum.each(screen_ids_to_refresh, fn screen_id ->
-      Task.Supervisor.start_child(TaskSupervisor, fn ->
-        @screen_data_fn.(screen_id, skip_serialize: true)
-      end)
+      Task.Supervisor.start_child(
+        TaskSupervisor,
+        Util.fn_with_timeout(
+          fn ->
+            @screen_data_fn.(screen_id, skip_serialize: true)
+          end,
+          10_000
+        )
+      )
     end)
 
     {:noreply, state}

lib/screens/util.ex

@@ -218,4 +218,16 @@ defmodule Screens.Util do
   def translate_carriage_occupancy_status(:full), do: :crowded
   def translate_carriage_occupancy_status(:not_accepting_passengers), do: :closed
   def translate_carriage_occupancy_status(_), do: nil
+
+  @doc """
+    Adds a timeout to a function. Mainly used for child processes of a Task.Supervisor
+    which don't come with a timeout by default.
+  """
+  @spec fn_with_timeout((() -> val), non_neg_integer()) :: (() -> val) when val: any()
+  def fn_with_timeout(fun, timeout) do
+    fn ->
+      _ = :timer.exit_after(timeout, :kill)
+      fun.()
+    end
+  end
 end