From a41a7f329f4931c404907ccf78a390b274780275 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 2 Jul 2023 15:29:04 +0200 Subject: [PATCH] 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. --- src/kernel/actor/CommObserver.cpp | 38 ++++++++++++++++++++-------- src/mc/transition/TransitionAny.cpp | 3 ++- src/mc/transition/TransitionComm.cpp | 10 +++++--- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/kernel/actor/CommObserver.cpp b/src/kernel/actor/CommObserver.cpp index 5022bdaae3..43f1b7e26d 100644 --- 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(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(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& activities, diff --git a/src/mc/transition/TransitionAny.cpp b/src/mc/transition/TransitionAny.cpp index 220b7cb501..3ea33cca38 100644 --- 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 01cf287c88..3cbf62ac81 100644 --- 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 { -- 2.20.1