Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
Remove 2 (out of 3) horrible hacks around Java contexts
authorMartin Quinson <martin.quinson@ens-rennes.fr>
Wed, 23 Jan 2019 06:37:18 +0000 (07:37 +0100)
committerMartin Quinson <martin.quinson@ens-rennes.fr>
Wed, 23 Jan 2019 06:37:36 +0000 (07:37 +0100)
Previously, we did not even try to kill java contexts because we
failed to do so in some cases.  Now, we try to kill them just like we
kill the other contexts, even if we don't always succeed.

One difficulty comes from the fact that some threads are created from
the C++ (from the deployment file) and then attached to the JVM, while
some others are created from Java directly (when calling new
Process().start()). The C++ vs Java stack is directly inverted between
cases, complicating the try/catch logic.

In practice, in this changeset, we try harder to convert between the
C++ StopRequest and the Java ProcessKilledError at the language
boundaries.

Also, we don't assert that JavaThread::stop() can actually detach the
thread: it won't for threads that are born in Java because their Java
stack was not unwinded yet. So throw a ProcessKilledError for that.

This is not prefect either since we still need to brutally kill the
JVM after the simulation using exit(0). I added some debug functions
to get more information. Just comment the first call to exit(0) in
jmsg.cpp to see the state of all user threads that we failed to
destroy.

They are all in the C++ world, without any [Java] stack, not blocked
on any Java synchronization object. I suspect that the system thread
is terminated, but the C++ std::thread object is still referenced
somewhere, preventing it from being garbage collected. That's weird
that the JVM still waits for them before ending (and thus mandating
the last the Horrible Hack -- exit(0) in the C++ side of the JVM
system process).

Gosh this is complicated. I'd love to have something like pybind11
dealing with all that shit automatically...

src/bindings/java/JavaContext.cpp
src/bindings/java/jmsg.cpp
src/bindings/java/jmsg_process.cpp
src/bindings/java/jmsg_task.cpp
src/bindings/java/org/simgrid/msg/Process.java
src/kernel/context/ContextThread.cpp
src/simix/ActorImpl.cpp
src/xbt/exception.cpp
teshsuite/java/sleephostoff/sleephostoff.tesh
tools/cmake/Java.cmake

index c8b6db5..f65f7e6 100644 (file)
@@ -64,51 +64,21 @@ void JavaContext::start_hook()
 
 void JavaContext::stop()
 {
-  /* I was asked to die (either with kill() or because of a failed element) */
-  if (this->iwannadie) {
-    this->iwannadie = 0;
-    JNIEnv* env     = this->jenv_;
-    XBT_DEBUG("Gonna launch Killed Error");
-    // 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",
-                       std::string("Process ") + this->get_actor()->get_cname() + " killed from file JavaContext.cpp");
-
-    // (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 {
-    ThreadContext::stop();
     JNIEnv* env = this->jenv_;
     env->DeleteGlobalRef(this->jprocess_);
     XBT_ATTRIB_UNUSED jint error = __java_vm->DetachCurrentThread();
-    xbt_assert((error == JNI_OK), "The thread couldn't be detached.");
-  }
+    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_ERROR("Cannot detach the current thread");
+      // simgrid::xbt::Backtrace().display();
+    }
+    ThreadContext::stop();
 }
 
 }}} // namespace simgrid::kernel::context
index 085486b..1af0c84 100644 (file)
@@ -148,8 +148,17 @@ JNIEXPORT void JNICALL JNICALL Java_org_simgrid_msg_Msg_run(JNIEnv * env, jclass
   for (auto const& elm : java_storage_map)
     jstorage_unref(env, elm.second);
 
-  /* FIXME: don't be of such an EXTREM BRUTALITY to stop the jvm. Sorry I don't get it working otherwise.
-   * See the comment in ActorImpl.cpp::SIMIX_process_kill() */
+  // Don't even report the surviving threads, just to pass the tests...
+  exit(0);
+
+  /* Display the status of remaining threads. None should survive, but who knows */
+  jclass clsProcess = jxbt_get_class(env, "org/simgrid/msg/Process");
+  jmethodID idDebug = jxbt_get_static_jmethod(env, clsProcess, "debugAllThreads", "()V");
+  xbt_assert(idDebug != nullptr, "Method Process.debugAllThreads() not found...");
+  env->CallStaticVoidMethod(clsProcess, idDebug);
+
+  /* FIXME: don't be of such an EXTREM BRUTALITY to stop the jvm.
+   * Sorry I don't get it working otherwise: some thread survive their own end, and I fail to do anything better */
   exit(0);
 }
 
@@ -263,6 +272,14 @@ static void run_jprocess(JNIEnv *env, jobject jprocess)
   xbt_assert((id != nullptr), "Method Process.run() not found...");
 
   env->CallVoidMethod(jprocess, id);
+  if (env->ExceptionOccurred()) {
+    XBT_DEBUG("Something went wrong in this Java actor, forget about it.");
+    env->ExceptionClear();
+    XBT_ATTRIB_UNUSED jint error = __java_vm->DetachCurrentThread();
+    xbt_assert(error == JNI_OK, "Cannot detach failing thread");
+    // simgrid::xbt::Backtrace().display();
+    throw simgrid::kernel::context::Context::StopRequest();
+  }
 }
 
 /** Create a Java org.simgrid.msg.Process with the arguments and run it */
index 3e1faae..0b569d1 100644 (file)
@@ -240,7 +240,12 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_sleep(JNIEnv *env, jclass cl
 JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_waitFor(JNIEnv * env, jobject jprocess, jdouble jseconds)
 {
   msg_error_t rv;
-  rv = MSG_process_sleep((double)jseconds);
+  try {
+    rv = MSG_process_sleep((double)jseconds);
+  } catch (simgrid::kernel::context::Context::StopRequest& e) {
+    jxbt_throw_by_name(env, "org/simgrid/msg/ProcessKilledError", "Process killed");
+  }
+
   if (env->ExceptionOccurred())
     return;
   if (rv != MSG_OK) {
@@ -259,10 +264,9 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_kill(JNIEnv * env, jobject j
   }
   try {
     MSG_process_kill(process);
-  } catch (xbt_ex& ex) {
-    XBT_VERB("Process %s just committed a suicide", MSG_process_get_name(process));
-    xbt_assert(process == MSG_process_self(),
-               "Killing a process should not raise an exception if it's not a suicide. Please report that bug.");
+  } catch (simgrid::kernel::context::Context::StopRequest& e) {
+    // XBT_INFO("Convert a StopRequest into a ProcessKilledError");
+    jxbt_throw_by_name(env, "org/simgrid/msg/ProcessKilledError", "Process killed");
   }
 }
 
index 3207b73..f146abe 100644 (file)
@@ -6,6 +6,7 @@
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
 #include "simgrid/s4u/Host.hpp"
+#include "src/kernel/context/Context.hpp"
 
 #include "jmsg.hpp"
 #include "jmsg_host.h"
@@ -126,7 +127,12 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_Task_execute(JNIEnv * env, jobject j
     return;
   }
   msg_error_t rv;
-  rv = MSG_task_execute(task);
+  try {
+    rv = MSG_task_execute(task);
+  } catch (simgrid::kernel::context::Context::StopRequest& e) {
+    jxbt_throw_by_name(env, "org/simgrid/msg/ProcessKilledError", "Process killed");
+  }
+
   if (env->ExceptionOccurred())
     return;
   if (rv != MSG_OK) {
@@ -281,12 +287,17 @@ JNIEXPORT jobject JNICALL Java_org_simgrid_msg_Task_receive(JNIEnv* env, jclass
   msg_task_t task = nullptr;
 
   const char *alias = env->GetStringUTFChars(jalias, 0);
-  msg_error_t rv    = MSG_task_receive_ext(&task, alias, (double)jtimeout, /*host*/ nullptr);
+  msg_error_t rv;
+  try {
+    rv = MSG_task_receive_ext(&task, alias, (double)jtimeout, /*host*/ nullptr);
+  } catch (simgrid::kernel::context::Context::StopRequest& e) {
+    jxbt_throw_by_name(env, "org/simgrid/msg/ProcessKilledError", "Process killed");
+  }
   env->ReleaseStringUTFChars(jalias, alias);
   if (env->ExceptionOccurred())
     return nullptr;
   if (rv != MSG_OK) {
-    jmsg_throw_status(env,rv);
+    jmsg_throw_status(env, rv);
     return nullptr;
   }
   jobject jtask_global = (jobject) MSG_task_get_data(task);
index 18664ac..0fb5c6f 100644 (file)
@@ -5,8 +5,12 @@
 
 package org.simgrid.msg;
 
-import java.util.Arrays;
+import java.lang.management.ManagementFactory;
+import java.lang.management.ThreadInfo;
+import java.lang.management.ThreadMXBean;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
 /**
  * A process may be defined as a code, with some private data, executing
@@ -301,6 +305,8 @@ public abstract class Process implements Runnable {
                }
                catch(ProcessKilledError pk) {
                        /* The process was killed before its end. With a kill() or something. */
+                       //Msg.info("Forwarding a PKE");
+                       throw pk;
                }       
        }
 
@@ -331,4 +337,44 @@ public abstract class Process implements Runnable {
         */
        public static native int getCount();
 
+        public static void debugAllThreads() {
+            final ThreadMXBean threadMXBean = ManagementFactory.getThreadMXBean();
+               long[] deads = threadMXBean.findDeadlockedThreads();
+               if (deads != null)
+                       for (long dead : deads) 
+                               System.err.println("Thread deadlocked: "+dead);
+
+               // Search remaining threads that are not main nor daemon
+            List<Long> ids = new ArrayList<>();
+            for (Thread t : Thread.getAllStackTraces().keySet()) {
+                if (! t.isDaemon() && !t.getName().equals("main"))
+                    ids.add(t.getId());
+            }
+            if (ids.size() > 0) {
+               long[] id_array = new long[ids.size()];
+               for (int i=0; i<ids.size(); i++) 
+                       id_array[i] = ids.get(i);
+
+                final ThreadInfo[] threadInfos = threadMXBean.getThreadInfo(id_array, true, true);
+                final StringBuilder dump = new StringBuilder();
+                for (ThreadInfo threadInfo : threadInfos) {
+                    dump.append('"');
+                    dump.append(threadInfo.getThreadName());
+                    dump.append("\" ");
+                    final Thread.State state = threadInfo.getThreadState();
+                    dump.append("\n   java.lang.Thread.State: ");
+                    dump.append(state);
+                    final StackTraceElement[] stackTraceElements = threadInfo.getStackTrace();
+                    for (final StackTraceElement stackTraceElement : stackTraceElements) {
+                        dump.append("\n        at ");
+                        dump.append(stackTraceElement);
+                    }
+                    dump.append("\n   In native? "+threadInfo.isInNative()+"\n");
+                    dump.append("   Suspended? "+threadInfo.isSuspended()+"\n");
+                    dump.append("   Waiting for: "+threadInfo.getLockInfo()+"\n");                   
+                    dump.append("\n\n");
+                }
+                System.err.println(dump);
+            }
+        }
 }
index 205e186..cc2529a 100644 (file)
@@ -103,7 +103,7 @@ void *ThreadContext::wrapper(void *param)
     if (not context->is_maestro()) // Just in case somebody detached maestro
       context->Context::stop();
   } catch (StopRequest const&) {
-    XBT_DEBUG("Caught a StopRequest");
+    XBT_DEBUG("Caught a StopRequest in Thread::wrapper");
     xbt_assert(not context->is_maestro(), "Maestro shall not receive StopRequests, even when detached.");
   } catch (simgrid::Exception const& e) {
     XBT_INFO("Actor killed by an uncatched exception %s", simgrid::xbt::demangle(typeid(e).name()).get());
@@ -116,6 +116,8 @@ void *ThreadContext::wrapper(void *param)
   stack.ss_flags = SS_DISABLE;
   sigaltstack(&stack, nullptr);
 #endif
+  XBT_DEBUG("Terminating");
+  Context::set_current(nullptr);
   return nullptr;
 }
 
index 8071b9c..9bf3f15 100644 (file)
@@ -473,30 +473,8 @@ void SIMIX_process_kill(smx_actor_t actor, smx_actor_t issuer)
   actor->exception           = nullptr;
 
   // Forcefully kill the actor if its host is turned off. Not an HostFailureException because you should not survive that
-  if (actor->host_->is_off()) {
-    /* HORRIBLE HACK: Don't throw an StopRequest exception in Java, because it breaks sometimes.
-     *
-     * It seems to break for the actors started from the Java world, with new Process()
-     * while it works for the ones started from the C world, with the deployment file.
-     * When it happens, the simulation stops brutally with a message "untrapped exception StopRequest".
-     *
-     * From what I understand, it works for the native actors because they have a nice try/catch block around their main
-     * but I fail to have something like that for pure Java actors. That's probably a story of C->Java vs Java->C
-     * calling conventions. The right solution may be to have try/catch(StopRequest) blocks around each native call in
-     * JNI. ie, protect every Java->C++ call from C++ exceptions. But this sounds long and painful to do before we
-     * switch to an automatic generator such as SWIG. For now, we don't throw here that exception that we sometimes fail
-     * to catch.
-     *
-     * One of the unfortunate outcome is that the threads started from the deployment file are not stopped anymore.
-     * Or maybe this is the actors stopping gracefully as opposed to the killed ones? Or maybe this is absolutely all
-     * actors of the Java simulation? I'm not sure. Anyway. Because of them, the simulation hangs at the end, waiting
-     * for them to stop but they won't. The current answer to that is very brutal:
-     * we do a "exit(0)" to kill the JVM from the C code after the call to MSG_run(). Definitely unpleasant.
-     */
-
-    if (simgrid::kernel::context::factory_initializer == nullptr) // Only Java sets a factory_initializer, for now
-      actor->throw_exception(std::make_exception_ptr(simgrid::kernel::context::Context::StopRequest("host failed")));
-  }
+  if (actor->host_->is_off())
+    actor->throw_exception(std::make_exception_ptr(simgrid::kernel::context::Context::StopRequest("host failed")));
 
   /* destroy the blocking synchro if any */
   if (actor->waiting_synchro != nullptr) {
@@ -743,6 +721,7 @@ void SIMIX_process_yield(smx_actor_t self)
   if (self->context_->iwannadie) {
 
     XBT_DEBUG("Process %s@%s is dead", self->get_cname(), self->host_->get_cname());
+    // throw simgrid::kernel::context::Context::StopRequest(); Does not seem to properly kill the actor
     self->context_->stop();
     THROW_IMPOSSIBLE;
   }
index efb2c60..4fd03bc 100644 (file)
@@ -4,6 +4,7 @@
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
 #include "simgrid/Exception.hpp"
+#include "src/kernel/context/Context.hpp"
 #include <xbt/config.hpp>
 #include <xbt/log.hpp>
 
@@ -145,6 +146,12 @@ static void handler()
     std::abort();
   }
 
+  catch (simgrid::kernel::context::Context::StopRequest& e) {
+    XBT_ERROR("Received a StopRequest at the top-level exception handler. Maybe a Java->C++ call that is not protected "
+              "in a try/catch?");
+    show_backtrace(bt);
+  }
+
   // We don't know how to manage other exceptions
   catch (...) {
     // If there was another handler let's delegate to it
@@ -156,7 +163,6 @@ static void handler()
       std::abort();
     }
   }
-
 }
 
 void install_exception_handler()
index 781268d..cdb73b7 100644 (file)
@@ -6,7 +6,7 @@ $ ${javacmd:=java} -classpath ${classpath:=.} sleephostoff.SleepHostOff ${srcdir
 > [  0.010000] (2:Sleeper@Tremblay) I'm not dead
 > [  0.020000] (1:TestRunner@Fafard) Stop Tremblay
 > [  0.020000] (2:Sleeper@Tremblay) I'm not dead
-> [  0.020000] (2:Sleeper@Tremblay) catch HostException: Host Failure 
 > [  0.020000] (1:TestRunner@Fafard) Tremblay has been stopped
+> [  0.020000] (2:Sleeper@Tremblay) catch HostException: Host Failure 
 > [  0.320000] (1:TestRunner@Fafard) Test sleep seems ok, cool! (number of Process : 1, it should be 1 (i.e. the Test one))
 > [  0.320000] (0:maestro@) MSG_main finished; Terminating the simulation...
index 4aaf5a3..f9cee49 100644 (file)
@@ -32,6 +32,7 @@ set_property(TARGET simgrid-java
              APPEND PROPERTY INCLUDE_DIRECTORIES "${INTERNAL_INCLUDES}")
 
 target_link_libraries(simgrid-java simgrid)
+add_dependencies(tests simgrid-java)
 
 get_target_property(COMMON_INCLUDES simgrid-java INCLUDE_DIRECTORIES)
 if (COMMON_INCLUDES)