From 5e1b4fa835cedc144a2ebe97fda0d139d63d3058 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Thu, 26 Jan 2017 18:23:25 +0100 Subject: [PATCH] allow java actors to kill themselves --- examples/java/process/kill/Killer.java | 1 + examples/java/process/kill/process_kill.tesh | 8 ++- src/bindings/java/JavaContext.cpp | 50 ++++++++++++++----- src/bindings/java/jmsg.cpp | 8 +-- src/bindings/java/jmsg_process.cpp | 11 ++-- src/bindings/java/jmsg_process.h | 11 +++- .../java/org/simgrid/msg/Process.java | 27 +++++----- src/simix/ActorImpl.cpp | 2 +- 8 files changed, 75 insertions(+), 43 deletions(-) diff --git a/examples/java/process/kill/Killer.java b/examples/java/process/kill/Killer.java index e03b69c2bc..18b0645cdb 100644 --- a/examples/java/process/kill/Killer.java +++ b/examples/java/process/kill/Killer.java @@ -35,5 +35,6 @@ public class Killer extends Process { poorVictim.kill(); Msg.info("Ok, goodbye now."); + exit(); // This would be more useful if not placed on the last line } } diff --git a/examples/java/process/kill/process_kill.tesh b/examples/java/process/kill/process_kill.tesh index 2b8dabaf26..cca85d0941 100644 --- a/examples/java/process/kill/process_kill.tesh +++ b/examples/java/process/kill/process_kill.tesh @@ -1,14 +1,12 @@ #! tesh -! output sort 19 - $ java -classpath ${classpath:=.} process/kill/Main ${srcdir:=.}/../platforms/small_platform.xml --lof=no_loc > [0.000000] [jmsg/INFO] Using regular java threads. -> [11.000000] [jmsg/INFO] MSG_main finished; Cleaning up the simulation... +> [Jacquelin:killer:(1) 0.000000] [jmsg/INFO] Hello! > [Boivin:victim:(2) 0.000000] [jmsg/INFO] Hello! > [Boivin:victim:(2) 0.000000] [jmsg/INFO] Suspending myself -> [Boivin:victim:(2) 10.000000] [jmsg/INFO] OK, OK. Let's work -> [Jacquelin:killer:(1) 0.000000] [jmsg/INFO] Hello! > [Jacquelin:killer:(1) 10.000000] [jmsg/INFO] Resume Process +> [Boivin:victim:(2) 10.000000] [jmsg/INFO] OK, OK. Let's work > [Jacquelin:killer:(1) 11.000000] [jmsg/INFO] Kill Process > [Jacquelin:killer:(1) 11.000000] [jmsg/INFO] Ok, goodbye now. +> [11.000000] [jmsg/INFO] MSG_main finished; Cleaning up the simulation... diff --git a/src/bindings/java/JavaContext.cpp b/src/bindings/java/JavaContext.cpp index 54bc03fe57..6298d26cf5 100644 --- a/src/bindings/java/JavaContext.cpp +++ b/src/bindings/java/JavaContext.cpp @@ -9,14 +9,14 @@ #include #include -#include -#include -#include -#include #include "JavaContext.hpp" #include "jxbt_utilities.h" +#include "src/simix/smx_private.h" #include "xbt/dynar.h" -#include "../../simix/smx_private.h" +#include +#include +#include +#include extern JavaVM *__java_vm; @@ -125,19 +125,43 @@ void* JavaContext::wrapper(void *data) void JavaContext::stop() { - /* I am the current process and I am dying */ + /* I was asked to die (either with kill() or because of a failed element) */ if (this->iwannadie) { this->iwannadie = 0; JNIEnv *env = get_current_thread_env(); XBT_DEBUG("Gonna launch Killed Error"); - // TODO Adrien, if the process has not been created at the java layer, why should we raise the exception/error at the java level (this happens - // for instance during the migration process that creates at the C level two processes: one on the SRC node and one on the DST node, if the DST process is killed. - // it is not required to raise an exception at the JAVA level, the low level should be able to manage such an issue correctly but this is not the case right now unfortunately ... - // TODO it will be nice to have the name of the process to help the end-user to know which Process has been killed - // jxbt_throw_by_name(env, "org/simgrid/msg/ProcessKilledError", bprintf("Process %s killed :) (file smx_context_java.c)", MSG_process_get_name( (msg_process_t)context) )); + // When the process wants to stop before its regular end, we should cut its call stack quickly. + // The easiest way to do so is to raise an exception that will be catched in its top calling level. + // + // For that, we raise a ProcessKilledError that is catched in Process::run() (in msg/Process.java) + // + // Throwing a Java exception to stop the actor may be an issue for pure C actors + // (as the ones created for the VM migration). The Java exception will not be catched anywhere. + // Bad things happen currently if these actors get killed, unfortunately. jxbt_throw_by_name(env, "org/simgrid/msg/ProcessKilledError", - bprintf("Process %s killed :) (file JavaContext.cpp)", - this->process()->name.c_str() )); + bprintf("Process %s killed from file JavaContext.cpp)", this->process()->name.c_str())); + + // (remember that throwing a java exception from C does not break the C execution path. + // Instead, it marks the exception to be raised when returning to the Java world and + // continues to execute the C function until it ends or returns). + + // Once the Java stack is marked to be unrolled, a C cancel_error is raised to kill the simcall + // on which the killed actor is blocked (if any). + // Not doing so would prevent the actor to notice that it's dead, leading to segfaults when it wakes up. + // This is dangerous: if the killed actor is not actually blocked, the cancel_error will not get catched. + // But it should be OK in most cases: + // - If I kill myself, I must do so with Process.kill(). + // The binding of this function in jmsg_process.cpp adds a try/catch around the MSG_process_kill() leading to us + // - If I kill someone else that is blocked, the cancel_error will unblock it. + // + // A problem remains probably if I kill a process that is ready_to_run in the same scheduling round. + // I guess that this will kill the whole simulation because the victim does not catch the exception. + // The only solution I see to that problem would be to completely rewrite the process killing sequence + // (also in C) so that it's based on regular C++ exceptions that would be catched anyway. + // In other words, we need to do in C++ what we do in Java for sake of uniformity. + // + // Plus, C++ RAII would work in that case, too. + XBT_DEBUG("Trigger a cancel error at the C level"); THROWF(cancel_error, 0, "process cancelled"); } else { diff --git a/src/bindings/java/jmsg.cpp b/src/bindings/java/jmsg.cpp index f19a1da799..f8fff3947d 100644 --- a/src/bindings/java/jmsg.cpp +++ b/src/bindings/java/jmsg.cpp @@ -26,6 +26,8 @@ #include "JavaContext.hpp" +#include + /* Shut up some errors in eclipse online compiler. I wish such a pimple wouldn't be needed */ #ifndef JNIEXPORT #define JNIEXPORT @@ -267,16 +269,14 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_Msg_energyInit() { */ static void run_jprocess(JNIEnv *env, jobject jprocess) { - xbt_assert(jprocess != nullptr, "Process not created..."); - //wait for the process to be able to begin - //TODO: Cache it + // wait for the process's start time jfieldID jprocess_field_Process_startTime = jxbt_get_sfield(env, "org/simgrid/msg/Process", "startTime", "D"); jdouble startTime = env->GetDoubleField(jprocess, jprocess_field_Process_startTime); if (startTime > MSG_get_clock()) MSG_process_sleep(startTime - MSG_get_clock()); //Execution of the "run" method. jmethodID id = jxbt_get_smethod(env, "org/simgrid/msg/Process", "run", "()V"); - xbt_assert( (id != nullptr), "Method not found..."); + xbt_assert((id != nullptr), "Method run() not found..."); env->CallVoidMethod(jprocess, id); } diff --git a/src/bindings/java/jmsg_process.cpp b/src/bindings/java/jmsg_process.cpp index eb7a29a046..6ec0684663 100644 --- a/src/bindings/java/jmsg_process.cpp +++ b/src/bindings/java/jmsg_process.cpp @@ -27,11 +27,6 @@ jfieldID jprocess_field_Process_name; jfieldID jprocess_field_Process_pid; jfieldID jprocess_field_Process_ppid; -JNIEXPORT void JNICALL -Java_org_simgrid_msg_Process_exit(JNIEnv *env, jobject jprocess) { - -} - jobject native_to_java_process(msg_process_t process) { simgrid::kernel::context::JavaContext* context = (simgrid::kernel::context::JavaContext*) MSG_process_get_smx_ctx(process); @@ -354,7 +349,11 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_kill(JNIEnv * env, jobject j return; } - MSG_process_kill(process); + try { + MSG_process_kill(process); + } catch (xbt_ex& ex) { + XBT_VERB("This process just killed itself."); + } } JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_migrate(JNIEnv * env, jobject jprocess, jobject jhost) diff --git a/src/bindings/java/jmsg_process.h b/src/bindings/java/jmsg_process.h index 963b7b33ef..6704662c54 100644 --- a/src/bindings/java/jmsg_process.h +++ b/src/bindings/java/jmsg_process.h @@ -15,6 +15,15 @@ SG_BEGIN_DECL(); +/* Shut up some errors in eclipse online compiler. I wish such a pimple wouldn't be needed */ +#ifndef JNIEXPORT +#define JNIEXPORT +#endif +#ifndef JNICALL +#define JNICALL +#endif +/* end of eclipse-mandated pimple */ + //Cached java fields extern jfieldID jprocess_field_Process_bind; extern jfieldID jprocess_field_Process_host; @@ -24,8 +33,6 @@ extern jfieldID jprocess_field_Process_name; extern jfieldID jprocess_field_Process_pid; extern jfieldID jprocess_field_Process_ppid; -JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_exit(JNIEnv *env, jobject); - jobject native_to_java_process(msg_process_t process); /** diff --git a/src/bindings/java/org/simgrid/msg/Process.java b/src/bindings/java/org/simgrid/msg/Process.java index 3d36996497..01f94b664a 100644 --- a/src/bindings/java/org/simgrid/msg/Process.java +++ b/src/bindings/java/org/simgrid/msg/Process.java @@ -46,7 +46,7 @@ public abstract class Process implements Runnable { * a native process. Even if this attribute is public you must never * access to it. It is set automatically during the build of the object. */ - private long bind; + private long bind = 0; /** Indicates if the process is started */ boolean started; /** @@ -70,25 +70,21 @@ public abstract class Process implements Runnable { */ private double killTime = -1; - private String name; + private String name = null; private int pid = -1; private int ppid = -1; private Host host = null; /** The arguments of the method function of the process. */ - private ArrayList args; + private ArrayList args = new ArrayList<>(); /** Default constructor */ protected Process() { this.id = nextProcessId++; - this.name = null; - this.bind = 0; - this.args = new ArrayList<>(); } - /** * Constructs a new process from the name of a host and his name. The method * function of the process doesn't have argument. @@ -189,6 +185,9 @@ public abstract class Process implements Runnable { * SimGrid sometimes have issues when you kill processes that are currently communicating and such. We are working on it to fix the issues. */ public native void kill(); + public static void kill(Process p) { + p.kill(); + } /** Suspends the process. See {@link #resume()} to resume it afterward */ public native void suspend(); @@ -277,7 +276,7 @@ public abstract class Process implements Runnable { */ public native void migrate(Host host); /** - * Makes the current process sleep until millis millisecondes have elapsed. + * Makes the current process sleep until millis milliseconds have elapsed. * You should note that unlike "waitFor" which takes seconds, this method takes milliseconds. * FIXME: Not optimal, maybe we should have two native functions. * @param millis the length of time to sleep in milliseconds. @@ -292,7 +291,7 @@ public abstract class Process implements Runnable { * milliseconds and nanoseconds. * Overloads Thread.sleep. * @param millis the length of time to sleep in milliseconds. - * @param nanos additionnal nanoseconds to sleep. + * @param nanos additional nanoseconds to sleep. */ public static native void sleep(long millis, int nanos) throws HostFailureException; /** @@ -325,14 +324,15 @@ public abstract class Process implements Runnable { } this.main(args); - } catch(MsgException e) { + } + catch(MsgException e) { e.printStackTrace(); Msg.info("Unexpected behavior. Stopping now"); System.exit(1); } catch(ProcessKilledError pk) { + /* The process was killed before its end. With a kill() or something. */ } - exit(); } /** @@ -343,7 +343,10 @@ public abstract class Process implements Runnable { */ public abstract void main(String[]args) throws MsgException; - public native void exit(); + /** Stops the execution of the current actor */ + public void exit() { + this.kill(); + } /** * Class initializer, to initialize various JNI stuff */ diff --git a/src/simix/ActorImpl.cpp b/src/simix/ActorImpl.cpp index 5fefcd31d1..9715c65360 100644 --- a/src/simix/ActorImpl.cpp +++ b/src/simix/ActorImpl.cpp @@ -175,7 +175,7 @@ ActorImpl::~ActorImpl() void create_maestro(std::function code) { smx_actor_t maestro = nullptr; - /* Create maestro process and intilialize it */ + /* Create maestro process and initialize it */ maestro = new simgrid::simix::ActorImpl(); maestro->pid = simix_process_maxpid++; maestro->ppid = -1; -- 2.20.1