From 72b7977026be2ec4497220e10118d392a76b310b Mon Sep 17 00:00:00 2001 From: Arnaud Giersch Date: Tue, 26 Mar 2019 15:06:34 +0100 Subject: [PATCH 1/1] Tidy SIMIX_process on_exit callbacks. This is a follow-up to commit 08e94eb0482589e4b287cbea301b84daf52635bd "Sanitize the prototype of Actor::on_exit() callbacks". --- include/simgrid/simix.h | 8 +++-- src/kernel/actor/ActorImpl.cpp | 60 ++++++++++++++++------------------ src/kernel/actor/ActorImpl.hpp | 7 +--- src/s4u/s4u_Actor.cpp | 14 ++++---- 4 files changed, 42 insertions(+), 47 deletions(-) diff --git a/include/simgrid/simix.h b/include/simgrid/simix.h index 25a83c607a..1a6bb09d9d 100644 --- a/include/simgrid/simix.h +++ b/include/simgrid/simix.h @@ -148,12 +148,14 @@ XBT_ATTRIB_DEPRECATED_v325("Please use ActorImpl::set_user_data()") XBT_PUBLIC XBT_ATTRIB_DEPRECATED_v325("Please use ActorImpl::get_user_data()") XBT_PUBLIC void* SIMIX_process_self_get_data(); XBT_ATTRIB_DEPRECATED_v325("Please manifest if you actually need this function") XBT_PUBLIC int SIMIX_process_has_pending_comms(smx_actor_t process); -XBT_PUBLIC void SIMIX_process_on_exit(smx_actor_t process, int_f_pvoid_pvoid_t fun, void* data); +XBT_ATTRIB_DEPRECATED_v325("Please use SIMIX_process_on_exit(smx_actor_t, const std::function&)") XBT_PUBLIC + void SIMIX_process_on_exit(smx_actor_t process, int_f_pvoid_pvoid_t fun, void* data); SG_END_DECL() #ifdef __cplusplus -XBT_PUBLIC void SIMIX_process_on_exit(smx_actor_t process, - const std::function& fun, void* data); +XBT_ATTRIB_DEPRECATED_v325("Please use SIMIX_process_on_exit(smx_actor_t, const std::function&)") XBT_PUBLIC + void SIMIX_process_on_exit(smx_actor_t process, const std::function& fun, void* data); +XBT_PUBLIC void SIMIX_process_on_exit(smx_actor_t process, const std::function& fun); #endif /****************************** Communication *********************************/ diff --git a/src/kernel/actor/ActorImpl.cpp b/src/kernel/actor/ActorImpl.cpp index 95dc8405a2..2c3a48a63e 100644 --- a/src/kernel/actor/ActorImpl.cpp +++ b/src/kernel/actor/ActorImpl.cpp @@ -141,11 +141,11 @@ void ActorImpl::cleanup() } // Execute the termination callbacks - smx_process_exit_status_t exit_status = (context_->iwannadie) ? SMX_EXIT_FAILURE : SMX_EXIT_SUCCESS; + bool failed = context_->iwannadie; while (not on_exit.empty()) { - s_smx_process_exit_fun_t exit_fun = on_exit.back(); + auto exit_fun = on_exit.back(); on_exit.pop_back(); - (exit_fun.fun)(exit_status, exit_fun.arg); + exit_fun(failed); } /* cancel non-blocking activities */ @@ -278,18 +278,6 @@ double ActorImpl::get_kill_time() return kill_timer ? kill_timer->get_date() : 0; } -static void dying_daemon(int /*exit_status*/, void* data) -{ - std::vector* vect = &simix_global->daemons; - - auto it = std::find(vect->begin(), vect->end(), static_cast(data)); - xbt_assert(it != vect->end(), "The dying daemon is not a daemon after all. Please report that bug."); - - /* Don't move the whole content since we don't really care about the order */ - std::swap(*it, vect->back()); - vect->pop_back(); -} - void ActorImpl::yield() { XBT_DEBUG("Yield actor '%s'", get_cname()); @@ -333,7 +321,15 @@ void ActorImpl::daemonize() if (not daemon_) { daemon_ = true; simix_global->daemons.push_back(this); - SIMIX_process_on_exit(this, dying_daemon, this); + SIMIX_process_on_exit(this, [this](bool) { + auto& vect = simix_global->daemons; + auto it = std::find(vect.begin(), vect.end(), this); + xbt_assert(it != vect.end(), "The dying daemon is not a daemon after all. Please report that bug."); + + /* Don't move the whole content since we don't really care about the order */ + std::swap(*it, vect.back()); + vect.pop_back(); + }); } } @@ -401,17 +397,12 @@ void ActorImpl::resume() activity::ActivityImplPtr ActorImpl::join(ActorImpl* actor, double timeout) { - activity::ActivityImplPtr res = this->sleep(timeout); - intrusive_ptr_add_ref(res.get()); - SIMIX_process_on_exit(actor, - [](int, void* arg) { - auto sleep = static_cast(arg); - if (sleep->surf_action_) - sleep->surf_action_->finish(resource::Action::State::FINISHED); - intrusive_ptr_release(sleep); - }, - res.get()); - return res; + activity::ActivityImplPtr sleep = this->sleep(timeout); + SIMIX_process_on_exit(actor, [sleep](bool) { + if (sleep->surf_action_) + sleep->surf_action_->finish(resource::Action::State::FINISHED); + }); + return sleep; } activity::ActivityImplPtr ActorImpl::sleep(double duration) @@ -736,14 +727,21 @@ smx_actor_t SIMIX_process_from_PID(aid_t PID) void SIMIX_process_on_exit(smx_actor_t actor, int_f_pvoid_pvoid_t fun, void* data) { - SIMIX_process_on_exit(actor, [fun](int a, void* b) { fun((void*)(intptr_t)a, b); }, data); + SIMIX_process_on_exit(actor, [fun, data](bool failed) { + intptr_t status = failed ? SMX_EXIT_FAILURE : SMX_EXIT_SUCCESS; + fun(reinterpret_cast(status), data); + }); } -void SIMIX_process_on_exit(smx_actor_t actor, const std::function& fun, void* data) +void SIMIX_process_on_exit(smx_actor_t actor, const std::function& fun, void* data) { - xbt_assert(actor, "current process not found: are you in maestro context ?"); + SIMIX_process_on_exit(actor, [fun, data](bool failed) { fun(failed ? SMX_EXIT_FAILURE : SMX_EXIT_SUCCESS, data); }); +} - actor->on_exit.emplace_back(s_smx_process_exit_fun_t{fun, data}); +void SIMIX_process_on_exit(smx_actor_t actor, const std::function& fun) +{ + xbt_assert(actor, "current process not found: are you in maestro context ?"); + actor->on_exit.emplace_back(fun); } /** @brief Restart a process, starting it again from the beginning. */ diff --git a/src/kernel/actor/ActorImpl.hpp b/src/kernel/actor/ActorImpl.hpp index cef9f09895..f4ee3a8857 100644 --- a/src/kernel/actor/ActorImpl.hpp +++ b/src/kernel/actor/ActorImpl.hpp @@ -15,11 +15,6 @@ #include #include -struct s_smx_process_exit_fun_t { - std::function fun; - void* arg; -}; - namespace simgrid { namespace kernel { namespace actor { @@ -70,7 +65,7 @@ public: activity::ActivityImplPtr waiting_synchro = nullptr; /* the current blocking synchro if any */ std::list comms; /* the current non-blocking communication synchros */ s_smx_simcall simcall; - std::vector on_exit; /* list of functions executed when the process dies */ + std::vector> on_exit; /* list of functions executed when the process dies */ std::function code; simix::Timer* kill_timer = nullptr; diff --git a/src/s4u/s4u_Actor.cpp b/src/s4u/s4u_Actor.cpp index 01c2763203..6e49f993a6 100644 --- a/src/s4u/s4u_Actor.cpp +++ b/src/s4u/s4u_Actor.cpp @@ -101,22 +101,22 @@ void Actor::set_auto_restart(bool autorestart) }); } -void Actor::on_exit(int_f_pvoid_pvoid_t fun, - void* data) /* deprecated: cleanup SIMIX_process_on_exit: change prototype of second parameter and - remove the last one */ +void Actor::on_exit(int_f_pvoid_pvoid_t fun, void* data) /* deprecated */ { - simix::simcall([this, fun, data] { SIMIX_process_on_exit(pimpl_, fun, data); }); + on_exit([fun, data](bool failed) { + intptr_t status = failed ? SMX_EXIT_FAILURE : SMX_EXIT_SUCCESS; + fun(reinterpret_cast(status), data); + }); } void Actor::on_exit(const std::function& fun, void* data) /* deprecated */ { - on_exit([fun, data](bool exit) { fun(exit, data); }); + on_exit([fun, data](bool failed) { fun(failed ? SMX_EXIT_FAILURE : SMX_EXIT_SUCCESS, data); }); } void Actor::on_exit(const std::function& fun) const { - simix::simcall( - [this, fun] { SIMIX_process_on_exit(pimpl_, [fun](int a, void* /*data*/) { fun(a != 0); }, nullptr); }); + simix::simcall([this, &fun] { SIMIX_process_on_exit(pimpl_, fun); }); } void Actor::migrate(Host* new_host) -- 2.20.1