Introduce a Trait to deal with autorestart matter in ActorImpl, and fix bugs

The daemonized behavior was not correctly saved, as noted by the
updated tesh-actor-autorestart test

diff --git a/src/kernel/actor/ActorImpl.cpp b/src/kernel/actor/ActorImpl.cpp
index 2676839..37dadd2 100644 (file)
--- a/src/kernel/actor/ActorImpl.cpp
+++ b/src/kernel/actor/ActorImpl.cpp
@@ -490,9 +490,8 @@ ActorImplPtr ActorImpl::create(const std::string& name, const ActorCode& code, v
 }
 ActorImplPtr ActorImpl::create(ProcessArg* args)
 {
-  actor::ActorImplPtr actor   = actor::ActorImpl::create(args->name, args->code, nullptr, args->host, nullptr);
-  auto* naked_actor           = actor.get();
-  naked_actor->restart_count_ = args->restart_count_;
+  ActorImplPtr actor    = ActorImpl::create(args->name, args->code, nullptr, args->host, nullptr);
+  actor->restart_count_ = args->restart_count_;
   actor->set_properties(args->properties);
   if (args->on_exit)
     *actor->on_exit = *args->on_exit;

diff --git a/src/kernel/actor/ActorImpl.hpp b/src/kernel/actor/ActorImpl.hpp
index e9dc43f..32d37ed 100644 (file)
--- a/src/kernel/actor/ActorImpl.hpp
+++ b/src/kernel/actor/ActorImpl.hpp
@@ -21,13 +21,23 @@ namespace kernel {
 namespace actor {
 class ProcessArg;
 
-class XBT_PUBLIC ActorImpl : public xbt::PropertyHolder {
+class XBT_PUBLIC ActorRestartingTrait {
+  bool auto_restart_ = false;
+  int restart_count_ = 0;
+
+  friend ActorImpl;
+
+public:
+  bool has_to_auto_restart() const { return auto_restart_; }
+  void set_auto_restart(bool autorestart) { auto_restart_ = autorestart; }
+  int get_restart_count() const { return restart_count_; }
+};
+
+class XBT_PUBLIC ActorImpl : public xbt::PropertyHolder, public ActorRestartingTrait {
   s4u::Host* host_   = nullptr; /* the host on which the actor is running */
   aid_t pid_         = 0;
   aid_t ppid_        = -1;
   bool daemon_       = false; /* Daemon actors are automatically killed when the last non-daemon leaves */
-  bool auto_restart_ = false;
-  int restart_count_ = 0;
   unsigned stacksize_; // set to default value in constructor
 
   std::vector<activity::MailboxImpl*> mailboxes;
@@ -62,9 +72,6 @@ public:
   bool is_daemon() const { return daemon_; } /** Whether this actor has been daemonized */
   bool is_maestro() const; /** Whether this actor is actually maestro (cheap call but may segfault before actor creation
                               / after terminaison) */
-  bool has_to_auto_restart() const { return auto_restart_; }
-  void set_auto_restart(bool autorestart) { auto_restart_ = autorestart; }
-  int get_restart_count() { return restart_count_; }
   void set_stacksize(unsigned stacksize) { stacksize_ = stacksize; }
   unsigned get_stacksize() const { return stacksize_; }
 
@@ -163,7 +170,7 @@ public:
   double kill_time                                                         = 0.0;
   const std::unordered_map<std::string, std::string> properties{};
   bool auto_restart                                                        = false;
-  bool daemon_                                                             = false;
+  bool daemon_;
   /* list of functions executed when the actor dies */
   const std::shared_ptr<std::vector<std::function<void(bool)>>> on_exit;
   int restart_count_ = 0;
@@ -174,7 +181,7 @@ public:
 
   explicit ProcessArg(const std::string& name, const std::function<void()>& code, void* data, s4u::Host* host,
                       double kill_time, const std::unordered_map<std::string, std::string>& properties,
-                      bool auto_restart, int restart_count)
+                      bool auto_restart, bool daemon, int restart_count)
       : name(name)
       , code(code)
       , data(data)
@@ -182,6 +189,7 @@ public:
       , kill_time(kill_time)
       , properties(properties)
       , auto_restart(auto_restart)
+      , daemon_(daemon)
       , restart_count_(restart_count)
   {
   }

diff --git a/src/surf/sg_platf.cpp b/src/surf/sg_platf.cpp
index 62ae0fd..268b015 100644 (file)
--- a/src/surf/sg_platf.cpp
+++ b/src/surf/sg_platf.cpp
@@ -477,13 +477,13 @@ void sg_platf_new_actor(simgrid::kernel::routing::ActorCreationArgs* actor)
   simgrid::kernel::actor::ActorCode code = factory(std::move(actor->args));
 
   auto* arg = new simgrid::kernel::actor::ProcessArg(actor_name, code, nullptr, host, kill_time, actor->properties,
-                                                     auto_restart, /*restart_count=*/0);
+                                                     auto_restart, /*daemon=*/false, /*restart_count=*/0);
 
   host->get_impl()->add_actor_at_boot(arg);
 
   if (start_time > simgrid::s4u::Engine::get_clock()) {
     arg = new simgrid::kernel::actor::ProcessArg(actor_name, code, nullptr, host, kill_time, actor->properties,
-                                                 auto_restart, /*restart_count=*/0);
+                                                 auto_restart, /*daemon=*/false, /*restart_count=*/0);
 
     XBT_DEBUG("Actor %s@%s will be started at time %f", arg->name.c_str(), arg->host->get_cname(), start_time);
     simgrid::kernel::timer::Timer::set(start_time, [arg, auto_restart]() {

diff --git a/teshsuite/s4u/actor-autorestart/actor-autorestart.cpp b/teshsuite/s4u/actor-autorestart/actor-autorestart.cpp
index f115f97..f896cee 100644 (file)
--- a/teshsuite/s4u/actor-autorestart/actor-autorestart.cpp
+++ b/teshsuite/s4u/actor-autorestart/actor-autorestart.cpp
@@ -10,13 +10,12 @@ XBT_LOG_NEW_DEFAULT_CATEGORY(s4u_test, "Messages specific for this s4u example")
 static void dummy()
 {
   XBT_INFO("I start");
-    simgrid::s4u::this_actor::sleep_for(200);
-    XBT_INFO("I stop");
+  simgrid::s4u::this_actor::sleep_for(200);
+  XBT_INFO("I stop");
 }
 
 static void dummy_daemon()
 {
-  simgrid::s4u::Actor::self()->daemonize();
   while (simgrid::s4u::this_actor::get_host()->is_on()) {
     XBT_INFO("Hello from the infinite loop");
     simgrid::s4u::this_actor::sleep_for(80.0);
@@ -29,14 +28,14 @@ static void autostart()
 
   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->on_exit([](bool failed) { XBT_INFO("Dummy actor %s.", failed ? "failed" : "terminating"); });
   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 failed) { XBT_INFO("Daemon actor %s.", failed ? "failed" : "terminating"); });
+  daemon_actor->daemonize()->set_auto_restart(true);
   daemon_actor->on_exit([](bool) { XBT_INFO("On_exit callback set after autorestart"); });
 
   simgrid::s4u::this_actor::sleep_for(50);

diff --git a/teshsuite/s4u/actor-autorestart/actor-autorestart.tesh b/teshsuite/s4u/actor-autorestart/actor-autorestart.tesh
index 05e8162..6b38943 100644 (file)
--- a/teshsuite/s4u/actor-autorestart/actor-autorestart.tesh
+++ b/teshsuite/s4u/actor-autorestart/actor-autorestart.tesh
@@ -5,9 +5,9 @@ $ ./actor-autorestart ${platfdir}/small_platform.xml
 > [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:Dummy:(2) 50.000000] [s4u_test/INFO] Dummy actor failed.
 > [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
+> [Fafard:Daemon:(3) 50.000000] [s4u_test/INFO] Daemon actor failed.
 > [Tremblay:Autostart:(1) 60.000000] [s4u_test/INFO] powering on Fafard
 > [Fafard:Dummy:(4) 60.000000] [s4u_test/INFO] I start
 > [Fafard:Daemon:(5) 60.000000] [s4u_test/INFO] Hello from the infinite loop
@@ -15,7 +15,7 @@ $ ./actor-autorestart ${platfdir}/small_platform.xml
 > [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:Dummy:(4) 260.000000] [s4u_test/INFO] Dummy actor terminating.
 > [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
+> [Fafard:Daemon:(5) 260.000000] [s4u_test/INFO] Daemon actor failed.
 > [260.000000] [s4u_test/INFO] Simulation time 260