From 84f504218e6e541881d121ba0382f132ff1cf135 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 13 Mar 2022 12:25:05 +0100 Subject: [PATCH] Move the actor lifecycle markers from Context to ActorImpl and reduce Context.hpp visibility --- src/bindings/java/jmsg_task.cpp | 1 - src/bindings/java/jmsg_vm.cpp | 1 - src/bindings/python/simgrid_python.cpp | 1 - src/kernel/EngineImpl.cpp | 2 +- src/kernel/activity/CommImpl.cpp | 9 ++++----- src/kernel/activity/ExecImpl.cpp | 4 ++-- src/kernel/activity/IoImpl.cpp | 3 +-- src/kernel/activity/Synchro.cpp | 3 +-- src/kernel/actor/ActorImpl.cpp | 25 +++++++++++++++---------- src/kernel/actor/ActorImpl.hpp | 8 ++++++++ src/kernel/actor/Simcall.cpp | 2 +- src/kernel/context/Context.cpp | 6 ------ src/kernel/context/Context.hpp | 6 ------ src/kernel/context/ContextBoost.hpp | 1 - src/mc/mc_record.cpp | 1 - src/s4u/s4u_Actor.cpp | 3 +-- src/xbt/exception.cpp | 1 - 17 files changed, 34 insertions(+), 43 deletions(-) diff --git a/src/bindings/java/jmsg_task.cpp b/src/bindings/java/jmsg_task.cpp index 02d2d3314a..d5686e2238 100644 --- a/src/bindings/java/jmsg_task.cpp +++ b/src/bindings/java/jmsg_task.cpp @@ -7,7 +7,6 @@ #include "simgrid/Exception.hpp" #include "simgrid/s4u/Host.hpp" -#include "src/kernel/context/Context.hpp" #include "jmsg.hpp" #include "jmsg_host.h" diff --git a/src/bindings/java/jmsg_vm.cpp b/src/bindings/java/jmsg_vm.cpp index b8153a3635..48cd4abe7b 100644 --- a/src/bindings/java/jmsg_vm.cpp +++ b/src/bindings/java/jmsg_vm.cpp @@ -10,7 +10,6 @@ #include "jxbt_utilities.hpp" #include "simgrid/Exception.hpp" #include "simgrid/plugins/live_migration.h" -#include "src/kernel/context/Context.hpp" #include "src/kernel/resource/VirtualMachineImpl.hpp" XBT_LOG_EXTERNAL_DEFAULT_CATEGORY(java); diff --git a/src/bindings/python/simgrid_python.cpp b/src/bindings/python/simgrid_python.cpp index 60ce5f652f..f79cfb04df 100644 --- a/src/bindings/python/simgrid_python.cpp +++ b/src/bindings/python/simgrid_python.cpp @@ -24,7 +24,6 @@ #include "simgrid/kernel/ProfileBuilder.hpp" #include "simgrid/kernel/routing/NetPoint.hpp" -#include "src/kernel/context/Context.hpp" #include #include #include diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp index b105c175be..79f12eca54 100644 --- a/src/kernel/EngineImpl.cpp +++ b/src/kernel/EngineImpl.cpp @@ -454,7 +454,7 @@ void EngineImpl::run_all_actors() instance_->get_context_factory()->run_all(); for (auto const& actor : actors_to_run_) - if (actor->context_->to_be_freed()) + if (actor->to_be_freed()) actor->cleanup_from_kernel(); actors_to_run_.swap(actors_that_ran_); diff --git a/src/kernel/activity/CommImpl.cpp b/src/kernel/activity/CommImpl.cpp index a383818e10..1e6483d66b 100644 --- a/src/kernel/activity/CommImpl.cpp +++ b/src/kernel/activity/CommImpl.cpp @@ -14,7 +14,6 @@ #include "src/kernel/activity/CommImpl.hpp" #include "src/kernel/activity/MailboxImpl.hpp" #include "src/kernel/actor/SimcallObserver.hpp" -#include "src/kernel/context/Context.hpp" #include "src/kernel/resource/CpuImpl.hpp" #include "src/kernel/resource/LinkImpl.hpp" #include "src/kernel/resource/StandardLinkImpl.hpp" @@ -481,7 +480,7 @@ void CommImpl::set_exception(actor::ActorImpl* issuer) case State::SRC_HOST_FAILURE: if (issuer == src_actor_) - issuer->context_->set_wannadie(); + issuer->set_wannadie(); else { set_state(State::FAILED); issuer->exception_ = std::make_exception_ptr(NetworkFailureException(XBT_THROW_POINT, "Remote peer failed")); @@ -490,7 +489,7 @@ void CommImpl::set_exception(actor::ActorImpl* issuer) case State::DST_HOST_FAILURE: if (issuer == dst_actor_) - issuer->context_->set_wannadie(); + issuer->set_wannadie(); else { set_state(State::FAILED); issuer->exception_ = std::make_exception_ptr(NetworkFailureException(XBT_THROW_POINT, "Remote peer failed")); @@ -554,10 +553,10 @@ void CommImpl::finish() /* Check out for errors */ if (not simcall->issuer_->get_host()->is_on()) { - simcall->issuer_->context_->set_wannadie(); + simcall->issuer_->set_wannadie(); } else { // Do not answer to dying actors - if (not simcall->issuer_->context_->wannadie()) { + if (not simcall->issuer_->wannadie()) { set_exception(simcall->issuer_); simcall->issuer_->simcall_answer(); } diff --git a/src/kernel/activity/ExecImpl.cpp b/src/kernel/activity/ExecImpl.cpp index 360ec95f5d..b33dac4abd 100644 --- a/src/kernel/activity/ExecImpl.cpp +++ b/src/kernel/activity/ExecImpl.cpp @@ -184,7 +184,7 @@ void ExecImpl::set_exception(actor::ActorImpl* issuer) if (issuer->get_host()->is_on()) issuer->exception_ = std::make_exception_ptr(HostFailureException(XBT_THROW_POINT, "Host failed")); else /* else, the actor will be killed with no possibility to survive */ - issuer->context_->set_wannadie(); + issuer->set_wannadie(); break; case State::CANCELED: @@ -219,7 +219,7 @@ void ExecImpl::finish() if (simcall->issuer_->get_host()->is_on()) simcall->issuer_->simcall_answer(); else - simcall->issuer_->context_->set_wannadie(); + simcall->issuer_->set_wannadie(); } } diff --git a/src/kernel/activity/IoImpl.cpp b/src/kernel/activity/IoImpl.cpp index 07dff0fd84..cdd593d202 100644 --- a/src/kernel/activity/IoImpl.cpp +++ b/src/kernel/activity/IoImpl.cpp @@ -12,7 +12,6 @@ #include "src/kernel/activity/IoImpl.hpp" #include "src/kernel/actor/ActorImpl.hpp" #include "src/kernel/actor/SimcallObserver.hpp" -#include "src/kernel/context/Context.hpp" #include "src/kernel/resource/CpuImpl.hpp" #include "src/kernel/resource/DiskImpl.hpp" #include "src/mc/mc_replay.hpp" @@ -118,7 +117,7 @@ void IoImpl::set_exception(actor::ActorImpl* issuer) { switch (get_state()) { case State::FAILED: - issuer->context_->set_wannadie(); + issuer->set_wannadie(); static_cast(get_iface())->complete(s4u::Activity::State::FAILED); issuer->exception_ = std::make_exception_ptr(StorageFailureException(XBT_THROW_POINT, "Storage failed")); break; diff --git a/src/kernel/activity/Synchro.cpp b/src/kernel/activity/Synchro.cpp index c9f58022e2..ea5e660218 100644 --- a/src/kernel/activity/Synchro.cpp +++ b/src/kernel/activity/Synchro.cpp @@ -8,7 +8,6 @@ #include "src/kernel/activity/Synchro.hpp" #include "src/kernel/actor/ActorImpl.hpp" -#include "src/kernel/context/Context.hpp" #include "src/kernel/resource/CpuImpl.hpp" XBT_LOG_NEW_DEFAULT_SUBCATEGORY(ker_synchro, kernel, @@ -66,7 +65,7 @@ void SynchroImpl::post() void SynchroImpl::set_exception(actor::ActorImpl* issuer) { if (get_state() == State::FAILED) { - issuer->context_->set_wannadie(); + issuer->set_wannadie(); issuer->exception_ = std::make_exception_ptr(HostFailureException(XBT_THROW_POINT, "Host failed")); } else { xbt_assert(get_state() == State::SRC_TIMEOUT, "Internal error in SynchroImpl::finish() unexpected synchro state %s", diff --git a/src/kernel/actor/ActorImpl.cpp b/src/kernel/actor/ActorImpl.cpp index f62d97c7a6..8acb38fa1c 100644 --- a/src/kernel/actor/ActorImpl.cpp +++ b/src/kernel/actor/ActorImpl.cpp @@ -166,11 +166,11 @@ void ActorImpl::cleanup_from_kernel() void ActorImpl::cleanup_from_self() { xbt_assert(not ActorImpl::is_maestro(), "Cleanup_from_self called from maestro on '%s'", get_cname()); - context_->set_to_be_freed(); + set_to_be_freed(); if (on_exit) { // Execute the termination callbacks - bool failed = context_->wannadie(); + bool failed = wannadie(); for (auto exit_fun = on_exit->crbegin(); exit_fun != on_exit->crend(); ++exit_fun) (*exit_fun)(failed); on_exit.reset(); @@ -193,14 +193,14 @@ void ActorImpl::cleanup_from_self() simcall_.timeout_cb_ = nullptr; } - context_->set_wannadie(false); // don't let the simcall's yield() do a Context::stop(), to avoid infinite loops + set_wannadie(false); // don't let the simcall's yield() do a Context::stop(), to avoid infinite loops actor::simcall_answered([this] { s4u::Actor::on_termination(*get_ciface()); }); - context_->set_wannadie(); + set_wannadie(); } void ActorImpl::exit() { - context_->set_wannadie(); + set_wannadie(); suspended_ = false; exception_ = nullptr; @@ -225,7 +225,7 @@ void ActorImpl::exit() void ActorImpl::kill(ActorImpl* actor) const { xbt_assert(not actor->is_maestro(), "Killing maestro is a rather bad idea."); - if (actor->context_->wannadie()) { + if (actor->wannadie()) { XBT_DEBUG("Ignoring request to kill actor %s@%s that is already dead", actor->get_cname(), actor->host_->get_cname()); return; @@ -275,7 +275,7 @@ void ActorImpl::yield() /* Ok, maestro returned control to us */ XBT_DEBUG("Control returned to me: '%s'", get_cname()); - if (context_->wannadie()) { + if (wannadie()) { XBT_DEBUG("Actor %s@%s is dead", get_cname(), host_->get_cname()); context_->stop(); THROW_IMPOSSIBLE; @@ -298,7 +298,7 @@ void ActorImpl::yield() } } #if HAVE_SMPI - if (not context_->wannadie()) + if (not wannadie()) smpi_switch_data_segment(get_iface()); #endif } @@ -354,7 +354,7 @@ void ActorImpl::resume() { XBT_IN("actor = %p", this); - if (context_->wannadie()) { + if (wannadie()) { XBT_VERB("Ignoring request to resume an actor that is currently dying."); return; } @@ -375,7 +375,7 @@ void ActorImpl::resume() activity::ActivityImplPtr ActorImpl::join(const ActorImpl* actor, double timeout) { activity::ActivityImplPtr sleep = this->sleep(timeout); - if (actor->context_->wannadie() || actor->context_->to_be_freed()) { + if (actor->wannadie() || actor->to_be_freed()) { if (sleep->surf_action_) sleep->surf_action_->finish(resource::Action::State::FINISHED); } else { @@ -506,6 +506,11 @@ ActorImplPtr ActorImpl::create(ProcessArg* args) actor->daemonize(); return actor; } +void ActorImpl::set_wannadie(bool value) +{ + XBT_DEBUG("Actor %s gonna die.", get_cname()); + iwannadie_ = value; +} void create_maestro(const std::function& code) { diff --git a/src/kernel/actor/ActorImpl.hpp b/src/kernel/actor/ActorImpl.hpp index 0dc8751f0d..c4310e0512 100644 --- a/src/kernel/actor/ActorImpl.hpp +++ b/src/kernel/actor/ActorImpl.hpp @@ -40,6 +40,8 @@ class XBT_PUBLIC ActorImpl : public xbt::PropertyHolder, public ActorRestartingT aid_t ppid_ = -1; bool daemon_ = false; /* Daemon actors are automatically killed when the last non-daemon leaves */ unsigned stacksize_; // set to default value in constructor + bool iwannadie_ = false; // True if we need to do some cleanups in actor mode. + bool to_be_freed_ = false; // True if cleanups in actor mode done, but cleanups in kernel mode pending std::vector mailboxes_; friend activity::MailboxImpl; @@ -63,6 +65,12 @@ public: const xbt::string& get_name() const { return name_; } const char* get_cname() const { return name_.c_str(); } + // Life-cycle + bool wannadie() const { return iwannadie_; } + void set_wannadie(bool value = true); + bool to_be_freed() const { return to_be_freed_; } + void set_to_be_freed() { to_be_freed_ = true; } + // Accessors to private fields s4u::Host* get_host() const { return host_; } void set_host(s4u::Host* dest); diff --git a/src/kernel/actor/Simcall.cpp b/src/kernel/actor/Simcall.cpp index 4fa6c44f49..b379c9f80d 100644 --- a/src/kernel/actor/Simcall.cpp +++ b/src/kernel/actor/Simcall.cpp @@ -25,7 +25,7 @@ void ActorImpl::simcall_handle(int times_considered) XBT_DEBUG("Handling simcall %p: %s", &simcall_, simcall_.get_cname()); if (simcall_.observer_ != nullptr) simcall_.observer_->prepare(times_considered); - if (context_->wannadie()) + if (wannadie()) return; xbt_assert(simcall_.call_ != Simcall::Type::NONE, "Asked to do the noop syscall on %s@%s", get_cname(), diff --git a/src/kernel/context/Context.cpp b/src/kernel/context/Context.cpp index 40ded32583..c6728df38e 100644 --- a/src/kernel/context/Context.cpp +++ b/src/kernel/context/Context.cpp @@ -142,12 +142,6 @@ void Context::stop() this->actor_->cleanup_from_self(); throw ForcefulKillException(); // clean RAII variables with the dedicated exception } - -void Context::set_wannadie(bool value) -{ - XBT_DEBUG("Actor %s gonna die.", actor_->get_cname()); - iwannadie_ = value; -} AttachContext::~AttachContext() = default; } // namespace context diff --git a/src/kernel/context/Context.hpp b/src/kernel/context/Context.hpp index 473af7a0c5..f7dc07d7fa 100644 --- a/src/kernel/context/Context.hpp +++ b/src/kernel/context/Context.hpp @@ -51,8 +51,6 @@ class XBT_PUBLIC Context { std::function code_; actor::ActorImpl* actor_ = nullptr; - bool iwannadie_ = false; // True if we need to do some cleanups in actor mode. - bool to_be_freed_ = false; // True if cleanups in actor mode done, but cleanups in kernel mode pending bool is_maestro_; void declare_context(std::size_t size); @@ -66,10 +64,6 @@ public: Context& operator=(const Context&) = delete; virtual ~Context(); - bool wannadie() const { return iwannadie_; } - void set_wannadie(bool value = true); - bool to_be_freed() const { return to_be_freed_; } - void set_to_be_freed() { to_be_freed_ = true; } bool is_maestro() const { return is_maestro_; } void operator()() const { code_(); } bool has_code() const { return static_cast(code_); } diff --git a/src/kernel/context/ContextBoost.hpp b/src/kernel/context/ContextBoost.hpp index 752844391b..5a877c4ee4 100644 --- a/src/kernel/context/ContextBoost.hpp +++ b/src/kernel/context/ContextBoost.hpp @@ -22,7 +22,6 @@ #include #include "src/internal_config.h" -#include "src/kernel/context/Context.hpp" #include "src/kernel/context/ContextSwapped.hpp" namespace simgrid { diff --git a/src/mc/mc_record.cpp b/src/mc/mc_record.cpp index 6e566b35fe..ee371dcbc3 100644 --- a/src/mc/mc_record.cpp +++ b/src/mc/mc_record.cpp @@ -5,7 +5,6 @@ #include "src/mc/mc_record.hpp" #include "src/kernel/activity/CommImpl.hpp" -#include "src/kernel/context/Context.hpp" #include "src/mc/mc_base.hpp" #include "src/mc/mc_replay.hpp" #include "src/mc/transition/Transition.hpp" diff --git a/src/s4u/s4u_Actor.cpp b/src/s4u/s4u_Actor.cpp index def186271f..27b2dca710 100644 --- a/src/s4u/s4u_Actor.cpp +++ b/src/s4u/s4u_Actor.cpp @@ -14,7 +14,6 @@ #include "src/include/mc/mc.h" #include "src/kernel/EngineImpl.hpp" #include "src/kernel/actor/ActorImpl.hpp" -#include "src/kernel/context/Context.hpp" #include "src/mc/mc_replay.hpp" #include "src/surf/HostImpl.hpp" @@ -112,7 +111,7 @@ void Actor::join(double timeout) const kernel::actor::ActorImpl* issuer = kernel::actor::ActorImpl::self(); const kernel::actor::ActorImpl* target = pimpl_; kernel::actor::simcall_blocking([issuer, target, timeout] { - if (target->context_->wannadie()) { + if (target->wannadie()) { // The joined actor is already finished, just wake up the issuer right away issuer->simcall_answer(); } else { diff --git a/src/xbt/exception.cpp b/src/xbt/exception.cpp index 6d6cc5f1a8..bcc183bbd8 100644 --- a/src/xbt/exception.cpp +++ b/src/xbt/exception.cpp @@ -4,7 +4,6 @@ * under the terms of the license (GNU LGPL) which comes with this package. */ #include "simgrid/Exception.hpp" -#include "src/kernel/context/Context.hpp" #include #include -- 2.20.1