From: Arnaud Giersch <arnaud.giersch@univ-fcomte.fr>
Date: Thu, 25 Apr 2019 06:18:29 +0000 (+0200)
Subject: Fix again FG #11 (mixing daemonize and auto-restart).
X-Git-Tag: v3.22.2~42
X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/4ca7055f4a76fa00332cfa1b80f0759cc1c0dd12?ds=sidebyside

Fix again FG #11 (mixing daemonize and auto-restart).

Main changes:
* don't modify (clean) the on_exit vector on cleanup, but detach its shared_ptr instead;
* define ActorImpl::undaemonize() for daemon cleanup, and use it instead of using an
  on_exit callback (ensures that it is not added multiple times);
* complete test 'tesh-s4u-actor-autorestart' with an auto-restarting daemon.
---

diff --git a/src/kernel/actor/ActorImpl.cpp b/src/kernel/actor/ActorImpl.cpp
index ec818b9a90..18ca939b97 100644
--- a/src/kernel/actor/ActorImpl.cpp
+++ b/src/kernel/actor/ActorImpl.cpp
@@ -139,12 +139,14 @@ void ActorImpl::cleanup()
     watched_hosts.insert(get_host()->get_name());
   }
 
-  // Execute the termination callbacks
-  bool failed = context_->iwannadie;
-  for (auto exit_fun = on_exit->crbegin(); exit_fun != on_exit->crend(); ++exit_fun)
-    (*exit_fun)(failed);
-  if (not has_to_auto_restart())
-    on_exit->clear();
+  if (on_exit) {
+    // Execute the termination callbacks
+    bool failed = context_->iwannadie;
+    for (auto exit_fun = on_exit->crbegin(); exit_fun != on_exit->crend(); ++exit_fun)
+      (*exit_fun)(failed);
+    on_exit.reset();
+  }
+  undaemonize();
 
   /* cancel non-blocking activities */
   for (auto activity : comms)
@@ -304,15 +306,20 @@ void ActorImpl::daemonize()
   if (not daemon_) {
     daemon_ = true;
     simix_global->daemons.push_back(this);
-    SIMIX_process_on_exit(this, [this](bool) {
-      auto& vect = simix_global->daemons;
-      auto it    = std::find(vect.begin(), vect.end(), this);
-      xbt_assert(it != vect.end(), "The dying daemon is not a daemon after all. Please report that bug.");
-
-      /* Don't move the whole content since we don't really care about the order */
-      std::swap(*it, vect.back());
-      vect.pop_back();
-    });
+  }
+}
+
+void ActorImpl::undaemonize()
+{
+  if (daemon_) {
+    auto& vect = simix_global->daemons;
+    auto it    = std::find(vect.begin(), vect.end(), this);
+    xbt_assert(it != vect.end(), "The dying daemon is not a daemon after all. Please report that bug.");
+    /* Don't move the whole content since we don't really care about the order */
+
+    std::swap(*it, vect.back());
+    vect.pop_back();
+    daemon_ = false;
   }
 }
 
diff --git a/src/kernel/actor/ActorImpl.hpp b/src/kernel/actor/ActorImpl.hpp
index bbc6a5a9f5..98336cfb4d 100644
--- a/src/kernel/actor/ActorImpl.hpp
+++ b/src/kernel/actor/ActorImpl.hpp
@@ -65,7 +65,7 @@ public:
   std::list<activity::ActivityImplPtr> comms;          /* the current non-blocking communication synchros */
   s_smx_simcall simcall;
   /* list of functions executed when the process dies */
-  const std::shared_ptr<std::vector<std::function<void(bool)>>> on_exit =
+  std::shared_ptr<std::vector<std::function<void(bool)>>> on_exit =
       std::make_shared<std::vector<std::function<void(bool)>>>();
 
   std::function<void()> code;
@@ -98,6 +98,9 @@ public:
   /* S4U/implem interfaces */
 private:
   s4u::Actor piface_; // Our interface is part of ourselves
+
+  void undaemonize();
+
 public:
   s4u::ActorPtr iface() { return s4u::ActorPtr(&piface_); }
   s4u::Actor* ciface() { return &piface_; }
diff --git a/teshsuite/s4u/actor-autorestart/actor-autorestart.cpp b/teshsuite/s4u/actor-autorestart/actor-autorestart.cpp
index a14e77e591..c52156e769 100644
--- a/teshsuite/s4u/actor-autorestart/actor-autorestart.cpp
+++ b/teshsuite/s4u/actor-autorestart/actor-autorestart.cpp
@@ -18,16 +18,31 @@ static void dummy()
   }
 }
 
+static void dummy_daemon()
+{
+  simgrid::s4u::Actor::self()->daemonize();
+  while (1) {
+    XBT_INFO("Hello from the infinite loop");
+    simgrid::s4u::this_actor::sleep_for(80.0);
+  }
+}
+
 static void autostart()
 {
   simgrid::s4u::Host* host = simgrid::s4u::Host::by_name("Fafard");
-  XBT_INFO("starting a dummy process on %s ", host->get_cname());
 
+  XBT_INFO("starting a dummy process on %s", host->get_cname());
   simgrid::s4u::ActorPtr dummy_actor = simgrid::s4u::Actor::create("Dummy", host, dummy);
   dummy_actor->on_exit([](bool) { XBT_INFO("On_exit callback set before autorestart"); });
   dummy_actor->set_auto_restart(true);
   dummy_actor->on_exit([](bool) { XBT_INFO("On_exit callback set after autorestart"); });
 
+  XBT_INFO("starting a daemon process on %s", host->get_cname());
+  simgrid::s4u::ActorPtr daemon_actor = simgrid::s4u::Actor::create("Daemon", host, dummy_daemon);
+  daemon_actor->on_exit([](bool) { XBT_INFO("On_exit callback set before autorestart"); });
+  daemon_actor->set_auto_restart(true);
+  daemon_actor->on_exit([](bool) { XBT_INFO("On_exit callback set after autorestart"); });
+
   simgrid::s4u::this_actor::sleep_for(50);
 
   XBT_INFO("powering off %s", host->get_cname());
diff --git a/teshsuite/s4u/actor-autorestart/actor-autorestart.tesh b/teshsuite/s4u/actor-autorestart/actor-autorestart.tesh
index 753900613b..05e8162949 100644
--- a/teshsuite/s4u/actor-autorestart/actor-autorestart.tesh
+++ b/teshsuite/s4u/actor-autorestart/actor-autorestart.tesh
@@ -1,12 +1,21 @@
 $ ./actor-autorestart ${platfdir}/small_platform.xml
-> [Tremblay:Autostart:(1) 0.000000] [s4u_test/INFO] starting a dummy process on Fafard 
+> [Tremblay:Autostart:(1) 0.000000] [s4u_test/INFO] starting a dummy process on Fafard
 > [Fafard:Dummy:(2) 0.000000] [s4u_test/INFO] I start
+> [Tremblay:Autostart:(1) 0.000000] [s4u_test/INFO] starting a daemon process on Fafard
+> [Fafard:Daemon:(3) 0.000000] [s4u_test/INFO] Hello from the infinite loop
 > [Tremblay:Autostart:(1) 50.000000] [s4u_test/INFO] powering off Fafard
 > [Fafard:Dummy:(2) 50.000000] [s4u_test/INFO] On_exit callback set after autorestart
 > [Fafard:Dummy:(2) 50.000000] [s4u_test/INFO] On_exit callback set before autorestart
+> [Fafard:Daemon:(3) 50.000000] [s4u_test/INFO] On_exit callback set after autorestart
+> [Fafard:Daemon:(3) 50.000000] [s4u_test/INFO] On_exit callback set before autorestart
 > [Tremblay:Autostart:(1) 60.000000] [s4u_test/INFO] powering on Fafard
-> [Fafard:Dummy:(3) 60.000000] [s4u_test/INFO] I start
-> [Fafard:Dummy:(3) 260.000000] [s4u_test/INFO] I stop
-> [Fafard:Dummy:(3) 260.000000] [s4u_test/INFO] On_exit callback set after autorestart
-> [Fafard:Dummy:(3) 260.000000] [s4u_test/INFO] On_exit callback set before autorestart
+> [Fafard:Dummy:(4) 60.000000] [s4u_test/INFO] I start
+> [Fafard:Daemon:(5) 60.000000] [s4u_test/INFO] Hello from the infinite loop
+> [Fafard:Daemon:(5) 140.000000] [s4u_test/INFO] Hello from the infinite loop
+> [Fafard:Daemon:(5) 220.000000] [s4u_test/INFO] Hello from the infinite loop
+> [Fafard:Dummy:(4) 260.000000] [s4u_test/INFO] I stop
+> [Fafard:Dummy:(4) 260.000000] [s4u_test/INFO] On_exit callback set after autorestart
+> [Fafard:Dummy:(4) 260.000000] [s4u_test/INFO] On_exit callback set before autorestart
+> [Fafard:Daemon:(5) 260.000000] [s4u_test/INFO] On_exit callback set after autorestart
+> [Fafard:Daemon:(5) 260.000000] [s4u_test/INFO] On_exit callback set before autorestart
 > [260.000000] [s4u_test/INFO] Simulation time 260