From 322e28c4503ed99d6c36ad9f450a3c0e515b4a8d Mon Sep 17 00:00:00 2001
From: SUTER Frederic <frederic.suter@cc.in2p3.fr>
Date: Wed, 10 Nov 2021 11:34:15 +0100
Subject: [PATCH] use smart pointers for kernel timers

---
 include/simgrid/kernel/Timer.hpp | 6 +++---
 src/kernel/EngineImpl.cpp        | 5 +----
 src/kernel/activity/ExecImpl.cpp | 1 +
 src/kernel/actor/ActorImpl.hpp   | 2 +-
 src/kernel/timer/Timer.cpp       | 8 +++-----
 src/simix/popping_private.hpp    | 4 ++--
 6 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/include/simgrid/kernel/Timer.hpp b/include/simgrid/kernel/Timer.hpp
index 6add5c2cee..d9af16420a 100644
--- a/include/simgrid/kernel/Timer.hpp
+++ b/include/simgrid/kernel/Timer.hpp
@@ -19,7 +19,7 @@ namespace timer {
 
 inline auto& kernel_timers() // avoid static initialization order fiasco
 {
-  using TimerQelt = std::pair<double, Timer*>;
+  using TimerQelt = std::pair<double, std::shared_ptr<Timer>>;
   static boost::heap::fibonacci_heap<TimerQelt, boost::heap::compare<xbt::HeapComparator<TimerQelt>>> value;
   return value;
 }
@@ -38,12 +38,12 @@ public:
 
   void remove();
 
-  template <class F> static inline Timer* set(double date, F callback)
+  template <class F> static inline std::shared_ptr<Timer> set(double date, F callback)
   {
     return set(date, xbt::Task<void()>(std::move(callback)));
   }
 
-  static Timer* set(double date, xbt::Task<void()>&& callback);
+  static std::shared_ptr<Timer> set(double date, xbt::Task<void()>&& callback);
   static double next() { return kernel_timers().empty() ? -1.0 : kernel_timers().top().first; }
 
   /** Handle any pending timer. Returns if something was actually run. */
diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp
index e2f5158c29..bddc871701 100644
--- a/src/kernel/EngineImpl.cpp
+++ b/src/kernel/EngineImpl.cpp
@@ -334,10 +334,7 @@ void EngineImpl::shutdown()
   /* Finish context module and SURF */
   instance_->destroy_context_factory();
 
-  while (not timer::kernel_timers().empty()) {
-    delete timer::kernel_timers().top().second;
-    timer::kernel_timers().pop();
-  }
+  timer::kernel_timers().clear();
 
   tmgr_finalize();
   sg_platf_exit();
diff --git a/src/kernel/activity/ExecImpl.cpp b/src/kernel/activity/ExecImpl.cpp
index 4e3f3f23ef..0e79088f8a 100644
--- a/src/kernel/activity/ExecImpl.cpp
+++ b/src/kernel/activity/ExecImpl.cpp
@@ -181,6 +181,7 @@ void ExecImpl::set_exception(actor::ActorImpl* issuer)
                  to_c_str(state_));
   }
 }
+
 void ExecImpl::finish()
 {
   XBT_DEBUG("ExecImpl::finish() in state %s", to_c_str(state_));
diff --git a/src/kernel/actor/ActorImpl.hpp b/src/kernel/actor/ActorImpl.hpp
index b5ae3c0556..5ce712f78a 100644
--- a/src/kernel/actor/ActorImpl.hpp
+++ b/src/kernel/actor/ActorImpl.hpp
@@ -75,7 +75,7 @@ public:
       std::make_shared<std::vector<std::function<void(bool)>>>();
 
   std::function<void()> code_;
-  timer::Timer* kill_timer_ = nullptr;
+  std::shared_ptr<timer::Timer> kill_timer_ = nullptr;
 
 private:
   /* Refcounting */
diff --git a/src/kernel/timer/Timer.cpp b/src/kernel/timer/Timer.cpp
index f9d85e781b..af0b2af6fd 100644
--- a/src/kernel/timer/Timer.cpp
+++ b/src/kernel/timer/Timer.cpp
@@ -11,9 +11,9 @@ namespace simgrid {
 namespace kernel {
 namespace timer {
 
-Timer* Timer::set(double date, xbt::Task<void()>&& callback)
+std::shared_ptr<Timer> Timer::set(double date, xbt::Task<void()>&& callback)
 {
-  auto* timer    = new Timer(date, std::move(callback));
+  auto timer     = std::make_shared<Timer>(date, std::move(callback));
   timer->handle_ = kernel_timers().emplace(std::make_pair(date, timer));
   return timer;
 }
@@ -22,7 +22,6 @@ Timer* Timer::set(double date, xbt::Task<void()>&& callback)
 void Timer::remove()
 {
   kernel_timers().erase(handle_);
-  delete this;
 }
 
 /** Handle any pending timer. Returns if something was actually run. */
@@ -32,10 +31,9 @@ bool Timer::execute_all()
   while (not kernel_timers().empty() && s4u::Engine::get_clock() >= kernel_timers().top().first) {
     result = true;
     // FIXME: make the timers being real callbacks (i.e. provide dispatchers that read and expand the args)
-    Timer* timer = kernel_timers().top().second;
+    std::shared_ptr<Timer> timer = kernel_timers().top().second;
     kernel_timers().pop();
     timer->callback();
-    delete timer;
   }
   return result;
 }
diff --git a/src/simix/popping_private.hpp b/src/simix/popping_private.hpp
index 13ab4f17ac..15bfb57bbf 100644
--- a/src/simix/popping_private.hpp
+++ b/src/simix/popping_private.hpp
@@ -45,8 +45,8 @@ union u_smx_scalar {
  */
 struct s_smx_simcall {
   simgrid::simix::Simcall call_                      = simgrid::simix::Simcall::NONE;
-  smx_actor_t issuer_                                = nullptr;
-  simgrid::kernel::timer::Timer* timeout_cb_         = nullptr; // Callback to timeouts
+  simgrid::kernel::actor::ActorImpl* issuer_                 = nullptr;
+  std::shared_ptr<simgrid::kernel::timer::Timer> timeout_cb_ = nullptr; // Callback to timeouts
   simgrid::kernel::actor::SimcallObserver* observer_ = nullptr; // makes that simcall observable by the MC
   unsigned int mc_max_consider_ =
       0; // How many times this simcall should be used. If >1, this will be a fork in the state space.
-- 
2.20.1