Fix a MC serialization bug around WaitAny

The individual wait activities that are embeeded in a WaitAny or
TestAny do not have any call_location associated, as the call_location
is associated to the observer, not the activity. The previous code was
thus not serializing the call location of all those activities while
serializing them, but it's a pity because the Checker code deserialize
these call_location in any case. This was resulting in the beginning
of the next transition to be used as location, naturally breaking the
deserialization afterward.

Instead, the call location of the TestAny or WaitAny is now used for
each and every wait activities embedded in the *Any call. This may
challenge the MC implementation, but it sounds like a sensible idea:
In such case, the location of the embedded wait IS the one of the
englobing TestAny or WaitAny call.

In some sense, that's very close to how we handle the timeout that is
given by the englobing call, not the embedded ones, in such situation.

diff --git a/src/kernel/actor/CommObserver.cpp b/src/kernel/actor/CommObserver.cpp
index 5022bda..43f1b7e 100644 (file)
--- a/src/kernel/actor/CommObserver.cpp
+++ b/src/kernel/actor/CommObserver.cpp
@@ -39,7 +39,8 @@ void ActivityTestanySimcall::prepare(int times_considered)
   else
     next_value_ = -1;
 }
-static void serialize_activity_test(const activity::ActivityImpl* act, std::stringstream& stream)
+static void serialize_activity_test(const activity::ActivityImpl* act, std::string const& call_location,
+                                    std::stringstream& stream)
 {
   if (const auto* comm = dynamic_cast<activity::CommImpl const*>(act)) {
     stream << "  " << (short)mc::Transition::Type::COMM_TEST;
@@ -47,6 +48,7 @@ static void serialize_activity_test(const activity::ActivityImpl* act, std::stri
     stream << ' ' << (comm->src_actor_ != nullptr ? comm->src_actor_->get_pid() : -1);
     stream << ' ' << (comm->dst_actor_ != nullptr ? comm->dst_actor_->get_pid() : -1);
     stream << ' ' << comm->get_mailbox_id();
+    stream << ' ' << call_location;
   } else {
     stream << (short)mc::Transition::Type::UNKNOWN;
     XBT_CRITICAL("Unknown transition in a test any. Bad things may happen");
@@ -58,16 +60,18 @@ static std::string to_string_activity_test(const activity::ActivityImpl* act)
     return "CommTest(comm_id:" + std::to_string(comm->get_id()) +
            " src:" + std::to_string(comm->src_actor_ != nullptr ? comm->src_actor_->get_pid() : -1) +
            " dst:" + std::to_string(comm->dst_actor_ != nullptr ? comm->dst_actor_->get_pid() : -1) +
-           " mbox:" + std::to_string(comm->get_mailbox_id());
+           " mbox:" + std::to_string(comm->get_mailbox_id()) + ")";
   } else {
     return "TestUnknownType()";
   }
 }
 void ActivityTestanySimcall::serialize(std::stringstream& stream) const
 {
+  XBT_DEBUG("Serialize %s", to_string().c_str());
   stream << (short)mc::Transition::Type::TESTANY << ' ' << activities_.size() << ' ';
   for (auto const* act : activities_) {
-    serialize_activity_test(act, stream);
+    // fun_call of each activity embedded in the TestAny is not known, so let's use the location of the TestAny itself
+    serialize_activity_test(act, fun_call_, stream);
     stream << ' ';
   }
   stream << fun_call_;
@@ -75,22 +79,28 @@ void ActivityTestanySimcall::serialize(std::stringstream& stream) const
 std::string ActivityTestanySimcall::to_string() const
 {
   std::stringstream buffer("TestAny(");
+  bool first = true;
   for (auto const* act : activities_) {
+    if (first)
+      first = false;
+    else
+      buffer << " | ";
     buffer << to_string_activity_test(act);
   }
+  buffer << ")";
   return buffer.str();
 }
 
 void ActivityTestSimcall::serialize(std::stringstream& stream) const
 {
-  serialize_activity_test(activity_, stream);
-  stream << ' ' << fun_call_;
+  serialize_activity_test(activity_, fun_call_, stream);
 }
 std::string ActivityTestSimcall::to_string() const
 {
   return to_string_activity_test(activity_);
 }
-static void serialize_activity_wait(const activity::ActivityImpl* act, bool timeout, std::stringstream& stream)
+static void serialize_activity_wait(const activity::ActivityImpl* act, bool timeout, std::string const& call_location,
+                                    std::stringstream& stream)
 {
   if (const auto* comm = dynamic_cast<activity::CommImpl const*>(act)) {
     stream << (short)mc::Transition::Type::COMM_WAIT << ' ';
@@ -99,6 +109,7 @@ static void serialize_activity_wait(const activity::ActivityImpl* act, bool time
     stream << ' ' << (comm->src_actor_ != nullptr ? comm->src_actor_->get_pid() : -1);
     stream << ' ' << (comm->dst_actor_ != nullptr ? comm->dst_actor_->get_pid() : -1);
     stream << ' ' << comm->get_mailbox_id();
+    stream << ' ' << call_location;
   } else {
     stream << (short)mc::Transition::Type::UNKNOWN;
   }
@@ -118,14 +129,15 @@ static std::string to_string_activity_wait(const activity::ActivityImpl* act)
 
 void ActivityWaitSimcall::serialize(std::stringstream& stream) const
 {
-  serialize_activity_wait(activity_, timeout_ > 0, stream);
-  stream << ' ' << fun_call_;
+  serialize_activity_wait(activity_, timeout_ > 0, fun_call_, stream);
 }
 void ActivityWaitanySimcall::serialize(std::stringstream& stream) const
 {
+  XBT_DEBUG("Serialize %s", to_string().c_str());
   stream << (short)mc::Transition::Type::WAITANY << ' ' << activities_.size() << ' ';
   for (auto const* act : activities_) {
-    serialize_activity_wait(act, timeout_ > 0, stream);
+    // fun_call of each activity embedded in the WaitAny is not known, so let's use the location of the WaitAny itself
+    serialize_activity_wait(act, timeout_ > 0, fun_call_, stream);
     stream << ' ';
   }
   stream << fun_call_;
@@ -137,9 +149,15 @@ std::string ActivityWaitSimcall::to_string() const
 std::string ActivityWaitanySimcall::to_string() const
 {
   std::stringstream buffer("WaitAny(");
+  bool first = true;
   for (auto const* act : activities_) {
-    buffer << to_string_activity_wait(act);
+    if (first)
+      first = false;
+    else
+      buffer << " | ";
+    buffer << to_string_activity_test(act);
   }
+  buffer << ")";
   return buffer.str();
 }
 ActivityWaitanySimcall::ActivityWaitanySimcall(ActorImpl* actor, const std::vector<activity::ActivityImpl*>& activities,

diff --git a/src/mc/transition/TransitionAny.cpp b/src/mc/transition/TransitionAny.cpp
index 220b7cb..3ea33cc 100644 (file)
--- a/src/mc/transition/TransitionAny.cpp
+++ b/src/mc/transition/TransitionAny.cpp
@@ -21,7 +21,7 @@ TestAnyTransition::TestAnyTransition(aid_t issuer, int times_considered, std::st
   xbt_assert(stream >> size);
   for (int i = 0; i < size; i++) {
     Transition* t = deserialize_transition(issuer, 0, stream);
-    XBT_DEBUG("TestAny received a transition %s", t->to_string(true).c_str());
+    XBT_INFO("TestAny received a transition %s", t->to_string(true).c_str());
     transitions_.push_back(t);
   }
 }
@@ -50,6 +50,7 @@ WaitAnyTransition::WaitAnyTransition(aid_t issuer, int times_considered, std::st
   xbt_assert(stream >> size);
   for (int i = 0; i < size; i++) {
     Transition* t = deserialize_transition(issuer, 0, stream);
+    XBT_INFO("WaitAny received transition %d/%d %s", (i + 1), size, t->to_string(true).c_str());
     transitions_.push_back(t);
   }
 }

diff --git a/src/mc/transition/TransitionComm.cpp b/src/mc/transition/TransitionComm.cpp
index 01cf287..3cbf62a 100644 (file)
--- a/src/mc/transition/TransitionComm.cpp
+++ b/src/mc/transition/TransitionComm.cpp
@@ -32,8 +32,8 @@ CommWaitTransition::CommWaitTransition(aid_t issuer, int times_considered, std::
     : Transition(Type::COMM_WAIT, issuer, times_considered)
 {
   xbt_assert(stream >> timeout_ >> comm_ >> sender_ >> receiver_ >> mbox_ >> call_location_);
-  XBT_DEBUG("CommWaitTransition %s comm:%u, sender:%ld receiver:%ld mbox:%u", (timeout_ ? "timeout" : "no-timeout"),
-            comm_, sender_, receiver_, mbox_);
+  XBT_DEBUG("CommWaitTransition %s comm:%u, sender:%ld receiver:%ld mbox:%u call_loc:%s",
+            (timeout_ ? "timeout" : "no-timeout"), comm_, sender_, receiver_, mbox_, call_location_.c_str());
 }
 std::string CommWaitTransition::to_string(bool verbose) const
 {
@@ -69,7 +69,8 @@ CommTestTransition::CommTestTransition(aid_t issuer, int times_considered, std::
     : Transition(Type::COMM_TEST, issuer, times_considered)
 {
   xbt_assert(stream >> comm_ >> sender_ >> receiver_ >> mbox_ >> call_location_);
-  XBT_DEBUG("CommTestTransition comm:%u, sender:%ld receiver:%ld mbox:%u", comm_, sender_, receiver_, mbox_);
+  XBT_DEBUG("CommTestTransition comm:%u, sender:%ld receiver:%ld mbox:%u call_loc:%s", comm_, sender_, receiver_, mbox_,
+            call_location_.c_str());
 }
 std::string CommTestTransition::to_string(bool verbose) const
 {
@@ -106,6 +107,7 @@ CommRecvTransition::CommRecvTransition(aid_t issuer, int times_considered, std::
     : Transition(Type::COMM_ASYNC_RECV, issuer, times_considered)
 {
   xbt_assert(stream >> comm_ >> mbox_ >> tag_ >> call_location_);
+  XBT_DEBUG("CommRecvTransition comm:%u, mbox:%u tag:%d call_loc:%s", comm_, mbox_, tag_, call_location_.c_str());
 }
 std::string CommRecvTransition::to_string(bool verbose) const
 {
@@ -170,7 +172,7 @@ CommSendTransition::CommSendTransition(aid_t issuer, int times_considered, std::
     : Transition(Type::COMM_ASYNC_SEND, issuer, times_considered)
 {
   xbt_assert(stream >> comm_ >> mbox_ >> tag_ >> call_location_);
-  XBT_DEBUG("SendTransition comm:%u mbox:%u", comm_, mbox_);
+  XBT_DEBUG("SendTransition comm:%u mbox:%u tag:%d call_loc:%s", comm_, mbox_, tag_, call_location_.c_str());
 }
 std::string CommSendTransition::to_string(bool verbose = false) const
 {