do not downcast activities to the wrong type just to get their name

- I use a protected method to keep my data private. Doing otherwise
  would trigger sonar to complain.
- The code is not particularly clear this way, but I fail to see how
  to do otherwise: I don't want the getter to become a virtual method.
- This was reported by UndefSan now that we have a test that deadlocks.

diff --git a/src/kernel/activity/ActivityImpl.hpp b/src/kernel/activity/ActivityImpl.hpp
index 6718efb..7cf2c0b 100644 (file)
--- a/src/kernel/activity/ActivityImpl.hpp
+++ b/src/kernel/activity/ActivityImpl.hpp
@@ -36,6 +36,7 @@ enum class State {
 
 class XBT_PUBLIC ActivityImpl {
   std::atomic_int_fast32_t refcount_{0};
 
 class XBT_PUBLIC ActivityImpl {
   std::atomic_int_fast32_t refcount_{0};

+  std::string name_ = "";

 
 public:
   virtual ~ActivityImpl();
 
 public:
   virtual ~ActivityImpl();


@@ -44,6 +45,18 @@ public:
   std::list<smx_simcall_t> simcalls_;   /* List of simcalls waiting for this activity */
   resource::Action* surf_action_ = nullptr;
 
   std::list<smx_simcall_t> simcalls_;   /* List of simcalls waiting for this activity */
   resource::Action* surf_action_ = nullptr;
 

+protected:
+  void inline set_name(const std::string& name)
+  {
+    // This is to keep name_ private while allowing ActivityImpl_T<??> to set it and then return a Ptr to qualified
+    // child type
+    name_ = name;
+  }
+
+public:
+  const std::string& get_name() { return name_; }
+  const char* get_cname() { return name_.c_str(); }
+

   bool test();
   void wait_for(actor::ActorImpl* issuer, double timeout);
   virtual ActivityImpl& set_timeout(double) { THROW_UNIMPLEMENTED; }
   bool test();
   void wait_for(actor::ActorImpl* issuer, double timeout);
   virtual ActivityImpl& set_timeout(double) { THROW_UNIMPLEMENTED; }


@@ -68,19 +81,19 @@ public:
   static xbt::signal<void(ActivityImpl const&)> on_resumed;
 };
 
   static xbt::signal<void(ActivityImpl const&)> on_resumed;
 };
 

+/* This class exists to allow chained setters as in exec->set_name()->set_priority()->set_blah()
+ * The difficulty is that set_name() must return a qualified child class, not the generic ancestor
+ * But the getter is still in the ancestor to be usable on generic activities with no downcast */

 template <class AnyActivityImpl> class ActivityImpl_T : public ActivityImpl {
 private:
 template <class AnyActivityImpl> class ActivityImpl_T : public ActivityImpl {
 private:

-  std::string name_             = "";

   std::string tracing_category_ = "";
 
 public:
   AnyActivityImpl& set_name(const std::string& name)
   {
   std::string tracing_category_ = "";
 
 public:
   AnyActivityImpl& set_name(const std::string& name)
   {

-    name_ = name;
+    ActivityImpl::set_name(name);

     return static_cast<AnyActivityImpl&>(*this);
   }
     return static_cast<AnyActivityImpl&>(*this);
   }

-  const std::string& get_name() { return name_; }
-  const char* get_cname() { return name_.c_str(); }

 
   AnyActivityImpl& set_tracing_category(const std::string& category)
   {
 
   AnyActivityImpl& set_tracing_category(const std::string& category)
   {

diff --git a/src/simix/smx_global.cpp b/src/simix/smx_global.cpp
index 1c38673..6187806 100644 (file)
--- a/src/simix/smx_global.cpp
+++ b/src/simix/smx_global.cpp
@@ -230,10 +230,6 @@ void Global::display_all_actor_status()
 
     if (actor->waiting_synchro_) {
       const char* synchro_description = "unknown";
 
     if (actor->waiting_synchro_) {
       const char* synchro_description = "unknown";

-      // we don't care about the Activity type to get its name, use RawImpl
-      const char* name = boost::static_pointer_cast<kernel::activity::ActivityImpl_T<kernel::activity::RawImpl>>(
-                             actor->waiting_synchro_)
-                             ->get_cname();

 
       if (boost::dynamic_pointer_cast<kernel::activity::ExecImpl>(actor->waiting_synchro_) != nullptr)
         synchro_description = "execution";
 
       if (boost::dynamic_pointer_cast<kernel::activity::ExecImpl>(actor->waiting_synchro_) != nullptr)
         synchro_description = "execution";


@@ -252,8 +248,8 @@ void Global::display_all_actor_status()
 
       XBT_INFO("Actor %ld (%s@%s): waiting for %s activity %#zx (%s) in state %d to finish", actor->get_pid(),
                actor->get_cname(), actor->get_host()->get_cname(), synchro_description,
 
       XBT_INFO("Actor %ld (%s@%s): waiting for %s activity %#zx (%s) in state %d to finish", actor->get_pid(),
                actor->get_cname(), actor->get_host()->get_cname(), synchro_description,

-               (xbt_log_no_loc ? (size_t)0xDEADBEEF : (size_t)actor->waiting_synchro_.get()), name,
-               (int)actor->waiting_synchro_->state_);
+               (xbt_log_no_loc ? (size_t)0xDEADBEEF : (size_t)actor->waiting_synchro_.get()),
+               actor->waiting_synchro_->get_cname(), (int)actor->waiting_synchro_->state_);

     } else {
       XBT_INFO("Actor %ld (%s@%s)", actor->get_pid(), actor->get_cname(), actor->get_host()->get_cname());
     }
     } else {
       XBT_INFO("Actor %ld (%s@%s)", actor->get_pid(), actor->get_cname(), actor->get_host()->get_cname());
     }