From 3580b0137eab12ca216d9847823c86918b10dd53 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sat, 12 Mar 2022 12:25:00 +0100 Subject: [PATCH] Simplify the Context::stop() and reduce duplication --- src/bindings/java/JavaContext.cpp | 34 ++++++++++++++++----------- src/bindings/java/JavaContext.hpp | 2 +- src/kernel/context/Context.cpp | 2 ++ src/kernel/context/ContextSwapped.cpp | 9 +------ src/kernel/context/ContextSwapped.hpp | 1 - src/kernel/context/ContextThread.cpp | 13 ++-------- src/kernel/context/ContextThread.hpp | 2 -- 7 files changed, 26 insertions(+), 37 deletions(-) diff --git a/src/bindings/java/JavaContext.cpp b/src/bindings/java/JavaContext.cpp index a532ef540d..c7c5168829 100644 --- a/src/bindings/java/JavaContext.cpp +++ b/src/bindings/java/JavaContext.cpp @@ -8,6 +8,7 @@ #include "JavaContext.hpp" #include "jxbt_utilities.hpp" #include "simgrid/Exception.hpp" +#include "src/kernel/actor/ActorImpl.hpp" #include #include @@ -60,21 +61,26 @@ void JavaContext::start_hook() this->jenv_ = env; } -void JavaContext::stop_hook() +void JavaContext::stop() { - JNIEnv* env = this->jenv_; - env->DeleteGlobalRef(this->jprocess_); - jint error = __java_vm->DetachCurrentThread(); - if (error != JNI_OK) { - /* This is probably a Java thread, ie an actor not created from the XML (and thus from the C++), - * but from Java with something like new Process().start(). - * - * We should not even try to detach such threads. Instead, we throw a Java exception that will raise up - * until run_jprocess(), IIUC. - */ - jxbt_throw_by_name(env, "org/simgrid/msg/ProcessKilledError", "Process killed"); - XBT_DEBUG("Cannot detach the current thread"); - } + this->get_actor()->cleanup_from_self(); + + /* Unregister the thread from the JVM */ + JNIEnv* env = this->jenv_; + env->DeleteGlobalRef(this->jprocess_); + jint error = __java_vm->DetachCurrentThread(); + if (error != JNI_OK) { + /* This is probably a Java thread, ie an actor not created from the XML (and thus from the C++), + * but from Java with something like new Process().start(). + * + * We should not even try to detach such threads. Instead, we throw a Java exception that will raise up + * until run_jprocess(), IIUC. + */ + jxbt_throw_by_name(env, "org/simgrid/msg/ProcessKilledError", "Process killed"); + XBT_DEBUG("Cannot detach the current thread"); + } + + throw ForcefulKillException(); // clean RAII variables with the dedicated exception } }}} // namespace simgrid::kernel::context diff --git a/src/bindings/java/JavaContext.hpp b/src/bindings/java/JavaContext.hpp index 42b932b899..8af830e61a 100644 --- a/src/bindings/java/JavaContext.hpp +++ b/src/bindings/java/JavaContext.hpp @@ -33,7 +33,7 @@ public: JavaContext(std::function&& code, actor::ActorImpl* actor); void start_hook() override; - void stop_hook() override; + void stop() override; }; class JavaContextFactory : public ContextFactory { diff --git a/src/kernel/context/Context.cpp b/src/kernel/context/Context.cpp index 627eb36391..40ded32583 100644 --- a/src/kernel/context/Context.cpp +++ b/src/kernel/context/Context.cpp @@ -5,6 +5,7 @@ #include "mc/mc.h" +#include "simgrid/Exception.hpp" #include "simgrid/s4u/Host.hpp" #include "src/kernel/activity/CommImpl.hpp" #include "src/kernel/context/Context.hpp" @@ -139,6 +140,7 @@ Context::~Context() void Context::stop() { this->actor_->cleanup_from_self(); + throw ForcefulKillException(); // clean RAII variables with the dedicated exception } void Context::set_wannadie(bool value) diff --git a/src/kernel/context/ContextSwapped.cpp b/src/kernel/context/ContextSwapped.cpp index 9950d44169..868c19bb80 100644 --- a/src/kernel/context/ContextSwapped.cpp +++ b/src/kernel/context/ContextSwapped.cpp @@ -49,7 +49,7 @@ void smx_ctx_wrapper(simgrid::kernel::context::SwappedContext* context) #endif try { (*context)(); - context->Context::stop(); + context->stop(); } catch (simgrid::ForcefulKillException const&) { XBT_DEBUG("Caught a ForcefulKillException"); } catch (simgrid::Exception const& e) { @@ -178,13 +178,6 @@ unsigned char* SwappedContext::get_stack_bottom() const #endif } -void SwappedContext::stop() -{ - Context::stop(); - /* We must cut the actor execution using an exception to properly free the C++ RAII variables */ - throw ForcefulKillException(); -} - void SwappedContext::swap_into(SwappedContext* to) { #if HAVE_SANITIZER_ADDRESS_FIBER_SUPPORT diff --git a/src/kernel/context/ContextSwapped.hpp b/src/kernel/context/ContextSwapped.hpp index 297eb698d9..7902de78e7 100644 --- a/src/kernel/context/ContextSwapped.hpp +++ b/src/kernel/context/ContextSwapped.hpp @@ -54,7 +54,6 @@ public: void suspend() override; virtual void resume(); - XBT_ATTRIB_NORETURN void stop() override; void swap_into(SwappedContext* to); diff --git a/src/kernel/context/ContextThread.cpp b/src/kernel/context/ContextThread.cpp index 488e53fc72..f5a1f7a5e6 100644 --- a/src/kernel/context/ContextThread.cpp +++ b/src/kernel/context/ContextThread.cpp @@ -97,10 +97,8 @@ void ThreadContext::wrapper(ThreadContext* context) try { (*context)(); - if (not context->is_maestro()) { // Just in case somebody detached maestro - context->Context::stop(); - context->stop_hook(); - } + if (not context->is_maestro()) // Just in case somebody detached maestro + context->stop(); } catch (ForcefulKillException const&) { XBT_DEBUG("Caught a ForcefulKillException in Thread::wrapper"); xbt_assert(not context->is_maestro(), "Maestro shall not receive ForcefulKillExceptions, even when detached."); @@ -140,13 +138,6 @@ void ThreadContext::yield() this->end_.release(); } -void ThreadContext::stop() -{ - Context::stop(); - stop_hook(); - throw ForcefulKillException(); -} - void ThreadContext::suspend() { this->yield(); diff --git a/src/kernel/context/ContextThread.hpp b/src/kernel/context/ContextThread.hpp index ac271f93de..a61a9f95c4 100644 --- a/src/kernel/context/ContextThread.hpp +++ b/src/kernel/context/ContextThread.hpp @@ -24,7 +24,6 @@ public: ThreadContext(const ThreadContext&) = delete; ThreadContext& operator=(const ThreadContext&) = delete; ~ThreadContext() override; - XBT_ATTRIB_NORETURN void stop() override; void suspend() override; void attach_start() override; void attach_stop() override; @@ -44,7 +43,6 @@ private: void yield(); // match a call to yield() virtual void start_hook() { /* empty placeholder, called after start(). Used in parallel mode and Java */} virtual void yield_hook() { /* empty placeholder, called before yield(). Used in parallel mode */} - virtual void stop_hook() { /* empty placeholder, called at stop(). Used in Java */} static void wrapper(ThreadContext* context); }; -- 2.20.1