From efcea33a5bbfbbf71994881befa2305fcbe238d2 Mon Sep 17 00:00:00 2001
From: Bastien Chamagne <bastien@archethic.net>
Date: Wed, 30 Aug 2023 11:44:14 +0200
Subject: [PATCH] HydratingCache now properly handles a timeout of the hydrate
 function

---
 .../oracle_chain/services/hydrating_cache.ex  | 31 ++++++++++---------
 .../services/hydrating_cache_test.exs         | 18 +++++++++--
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/lib/archethic/oracle_chain/services/hydrating_cache.ex b/lib/archethic/oracle_chain/services/hydrating_cache.ex
index 8ae127c24..abbc799f6 100644
--- a/lib/archethic/oracle_chain/services/hydrating_cache.ex
+++ b/lib/archethic/oracle_chain/services/hydrating_cache.ex
@@ -8,7 +8,9 @@ defmodule Archethic.OracleChain.Services.HydratingCache do
   use GenServer
   @vsn Mix.Project.config()[:version]
 
+  alias Archethic.TaskSupervisor
   alias Archethic.Utils
+
   require Logger
 
   defmodule State do
@@ -78,7 +80,7 @@ defmodule Archethic.OracleChain.Services.HydratingCache do
         }
       ) do
     hydrating_task =
-      Task.async(fn ->
+      Task.Supervisor.async_nolink(TaskSupervisor, fn ->
         try do
           {:ok, apply(m, f, a)}
         rescue
@@ -93,8 +95,9 @@ defmodule Archethic.OracleChain.Services.HydratingCache do
     {:noreply, %State{state | hydrating_task: hydrating_task}}
   end
 
-  def handle_info({:kill_hydrating_task, task}, state) do
-    Task.shutdown(task, :brutal_kill)
+  def handle_info({:kill_hydrating_task, %Task{pid: pid}}, state) do
+    # Task.shutdown will not send DOWN msg
+    Process.exit(pid, :kill)
 
     {:noreply, state}
   end
@@ -103,7 +106,6 @@ defmodule Archethic.OracleChain.Services.HydratingCache do
         {ref, result},
         state = %State{
           mfa: {m, f, a},
-          refresh_interval: refresh_interval,
           ttl_timer: ttl_timer,
           ttl: ttl,
           hydrating_task: %Task{ref: ref_task}
@@ -123,15 +125,7 @@ defmodule Archethic.OracleChain.Services.HydratingCache do
         nil
       end
 
-    # start a new hydrate timer
-    hydrating_timer = Process.send_after(self(), :hydrate, next_tick_in_seconds(refresh_interval))
-
-    new_state = %{
-      state
-      | ttl_timer: ttl_timer,
-        hydrating_task: nil,
-        hydrating_timer: hydrating_timer
-    }
+    new_state = %{state | ttl_timer: ttl_timer}
 
     case result do
       {:ok, value} ->
@@ -143,7 +137,16 @@ defmodule Archethic.OracleChain.Services.HydratingCache do
     end
   end
 
-  def handle_info({:DOWN, _ref, :process, _, _}, state), do: {:noreply, state}
+  def handle_info(
+        {:DOWN, _ref, :process, _, _},
+        state = %State{refresh_interval: refresh_interval}
+      ) do
+    # we always receive a DOWN on success/error/timeout
+    # so this is the best place to cleanup & start a new timer
+    hydrating_timer = Process.send_after(self(), :hydrate, next_tick_in_seconds(refresh_interval))
+
+    {:noreply, %{state | hydrating_task: nil, hydrating_timer: hydrating_timer}}
+  end
 
   def handle_info(:discard_value, state) do
     {:noreply, %State{state | value: nil, ttl_timer: nil}}
diff --git a/test/archethic/oracle_chain/services/hydrating_cache_test.exs b/test/archethic/oracle_chain/services/hydrating_cache_test.exs
index efe80b19a..319b72992 100644
--- a/test/archethic/oracle_chain/services/hydrating_cache_test.exs
+++ b/test/archethic/oracle_chain/services/hydrating_cache_test.exs
@@ -130,19 +130,31 @@ defmodule ArchethicCache.OracleChain.Services.HydratingCacheTest do
   test "should kill the task if hydrating function takes longer than timeout" do
     {:ok, pid} =
       HydratingCache.start_link(
-        mfa: {Process, :sleep, [100]},
-        refresh_interval: 100,
-        hydrating_function_timeout: 10,
+        mfa: {Process, :sleep, [200]},
+        refresh_interval: 200,
+        hydrating_function_timeout: 100,
         ttl: :infinity
       )
 
     :erlang.trace(pid, true, [:receive])
+
+    state_begin = :sys.get_state(pid)
+
+    Process.sleep(100)
+
     assert_receive {:trace, ^pid, :receive, :hydrate}
     assert_receive {:trace, ^pid, :receive, {:kill_hydrating_task, _}}
 
+    # check task has been killed but not genserver
     refute Process.alive?(:sys.get_state(pid).hydrating_task.pid)
     assert Process.alive?(pid)
 
+    # genserver still able to reply
     assert :error = HydratingCache.get(pid)
+
+    Process.sleep(200)
+
+    # make sure a new timer has been started
+    assert :sys.get_state(pid).hydrating_timer != state_begin.hydrating_timer
   end
 end