From a96479d66281806d9e372a705a1bdf4ebf6f8c84 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Tue, 7 Mar 2017 17:11:31 +0100 Subject: [PATCH] Plug many memleaks in the Java bindings This also (fix #134) --- src/bindings/java/JavaContext.cpp | 5 ++--- src/bindings/java/jmsg.cpp | 7 +++---- src/bindings/java/jmsg_process.cpp | 16 ++++++---------- src/bindings/java/jmsg_task.cpp | 2 +- src/bindings/java/jmsg_vm.cpp | 14 ++++++++++---- src/bindings/java/jmsg_vm.h | 9 +++++++++ src/bindings/java/org/simgrid/msg/Process.java | 2 +- src/plugins/vm/VirtualMachineImpl.cpp | 5 ++++- src/plugins/vm/VirtualMachineImpl.hpp | 5 +++-- 9 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/bindings/java/JavaContext.cpp b/src/bindings/java/JavaContext.cpp index ba70dde478..dcb588379e 100644 --- a/src/bindings/java/JavaContext.cpp +++ b/src/bindings/java/JavaContext.cpp @@ -106,10 +106,9 @@ void* JavaContext::wrapper(void *data) //Attach the thread to the JVM JNIEnv *env; - XBT_ATTRIB_UNUSED jint error = - __java_vm->AttachCurrentThread((void **)&env, nullptr); + XBT_ATTRIB_UNUSED jint error = __java_vm->AttachCurrentThread((void**)&env, nullptr); xbt_assert((error == JNI_OK), "The thread could not be attached to the JVM"); - context->jenv = get_current_thread_env(); + context->jenv = env; //Wait for the first scheduling round to happen. xbt_os_sem_acquire(context->begin); //Create the "Process" object if needed. diff --git a/src/bindings/java/jmsg.cpp b/src/bindings/java/jmsg.cpp index 944919d0ff..389a671835 100644 --- a/src/bindings/java/jmsg.cpp +++ b/src/bindings/java/jmsg.cpp @@ -52,10 +52,9 @@ JavaVM *get_java_VM() JNIEnv *get_current_thread_env() { - JNIEnv *env; - - __java_vm->AttachCurrentThread((void **) &env, nullptr); - return env; + using simgrid::kernel::context::JavaContext; + JavaContext* ctx = static_cast(xbt_os_thread_get_extra_data()); + return ctx->jenv; } void jmsg_throw_status(JNIEnv *env, msg_error_t status) { diff --git a/src/bindings/java/jmsg_process.cpp b/src/bindings/java/jmsg_process.cpp index 612e38b552..b089ee625b 100644 --- a/src/bindings/java/jmsg_process.cpp +++ b/src/bindings/java/jmsg_process.cpp @@ -79,7 +79,6 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_create(JNIEnv * env, jobject msg_process_t process; /* the native process to create */ msg_host_t host; /* Where that process lives */ - const char* hostname = env->GetStringUTFChars((jstring)jhostname, 0); /* get the name of the java process */ jname = jprocess_get_name(jprocess_arg, env); @@ -88,15 +87,15 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_create(JNIEnv * env, jobject return; } const char* name = env->GetStringUTFChars(jname, 0); - name = xbt_strdup(name); /* bind/retrieve the msg host */ + const char* hostname = env->GetStringUTFChars((jstring)jhostname, 0); host = MSG_host_by_name(hostname); - if (!(host)) { /* not bound */ jxbt_throw_host_not_found(env, hostname); return; } + env->ReleaseStringUTFChars((jstring)jhostname, hostname); /* create a global java process instance */ jprocess = jprocess_ref(jprocess_arg, env); @@ -105,8 +104,6 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_create(JNIEnv * env, jobject return; } - /* Retrieve the kill time from the process */ - jdouble jkill = env->GetDoubleField(jprocess, jprocess_field_Process_killTime); /* Actually build the MSG process */ process = MSG_process_create_with_environment(name, [](int argc, char** argv) -> int { smx_actor_t process = SIMIX_process_self(); @@ -119,14 +116,13 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_Process_create(JNIEnv * env, jobject host, /*argc, argv, properties*/ 0, nullptr, nullptr); - MSG_process_set_kill_time(process, (double)jkill); + env->ReleaseStringUTFChars(jname, name); /* bind the java process instance to the native process */ jprocess_bind(jprocess, process, env); - /* release our reference to the process name (variable name becomes invalid) */ - //FIXME : This line should be uncommented but with mac it doesn't work. BIG WARNING - //env->ReleaseStringUTFChars(jname, name); - env->ReleaseStringUTFChars((jstring) jhostname, hostname); + /* Retrieve the kill time from the process */ + jdouble jkill = env->GetDoubleField(jprocess, jprocess_field_Process_killTime); + MSG_process_set_kill_time(process, (double)jkill); /* sets the PID and the PPID of the process */ env->SetIntField(jprocess, jprocess_field_Process_pid,(jint) MSG_process_get_PID(process)); diff --git a/src/bindings/java/jmsg_task.cpp b/src/bindings/java/jmsg_task.cpp index f0da73932b..73a9ba0904 100644 --- a/src/bindings/java/jmsg_task.cpp +++ b/src/bindings/java/jmsg_task.cpp @@ -370,6 +370,7 @@ JNIEXPORT jobject JNICALL Java_org_simgrid_msg_Task_receive(JNIEnv * env, jclass const char *alias = env->GetStringUTFChars(jalias, 0); msg_error_t rv = MSG_task_receive_ext(&task, alias, (double) jtimeout, host); + env->ReleaseStringUTFChars(jalias, alias); if (env->ExceptionOccurred()) return nullptr; if (rv != MSG_OK) { @@ -383,7 +384,6 @@ JNIEXPORT jobject JNICALL Java_org_simgrid_msg_Task_receive(JNIEnv * env, jclass env->DeleteGlobalRef(jtask_global); MSG_task_set_data(task, nullptr); - env->ReleaseStringUTFChars(jalias, alias); return (jobject) jtask_local; } diff --git a/src/bindings/java/jmsg_vm.cpp b/src/bindings/java/jmsg_vm.cpp index 12d1fe113a..1c4d84e55f 100644 --- a/src/bindings/java/jmsg_vm.cpp +++ b/src/bindings/java/jmsg_vm.cpp @@ -8,6 +8,7 @@ #include "jmsg_vm.h" #include "jmsg_host.h" #include "jxbt_utilities.h" +#include "src/plugins/vm/VirtualMachineImpl.hpp" #include "xbt/ex.hpp" XBT_LOG_EXTERNAL_DEFAULT_CATEGORY(jmsg); @@ -36,7 +37,7 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_VM_nativeInit(JNIEnv *env, jclass cl JNIEXPORT jint JNICALL Java_org_simgrid_msg_VM_isCreated(JNIEnv * env, jobject jvm) { msg_vm_t vm = jvm_get_native(env,jvm); - return (jint) MSG_vm_is_created(vm); + return MSG_vm_is_created(vm); } JNIEXPORT jint JNICALL Java_org_simgrid_msg_VM_isRunning(JNIEnv * env, jobject jvm) @@ -69,10 +70,9 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_VM_create(JNIEnv* env, jobject jvm, msg_host_t host = jhost_get_native(env, jhost); const char* name = env->GetStringUTFChars(jname, 0); - name = xbt_strdup(name); - msg_vm_t vm = MSG_vm_create(host, name, static_cast(jramsize), static_cast(jmig_netspeed), static_cast(jdp_intensity)); + env->ReleaseStringUTFChars(jname, name); jvm_bind(env, jvm, vm); } @@ -92,7 +92,13 @@ JNIEXPORT void JNICALL Java_org_simgrid_msg_VM_start(JNIEnv *env, jobject jvm) JNIEXPORT void JNICALL Java_org_simgrid_msg_VM_shutdown(JNIEnv *env, jobject jvm) { msg_vm_t vm = jvm_get_native(env,jvm); - MSG_vm_shutdown(vm); + if (vm) { + MSG_vm_shutdown(vm); + auto vmList = &simgrid::vm::VirtualMachineImpl::allVms_; + vmList->erase( + std::remove_if(vmList->begin(), vmList->end(), [vm](simgrid::s4u::VirtualMachine* it) { return vm == it; }), + vmList->end()); + } } JNIEXPORT void JNICALL Java_org_simgrid_msg_VM_internalmig(JNIEnv *env, jobject jvm, jobject jhost) diff --git a/src/bindings/java/jmsg_vm.h b/src/bindings/java/jmsg_vm.h index e19776ca9e..a0efd27141 100644 --- a/src/bindings/java/jmsg_vm.h +++ b/src/bindings/java/jmsg_vm.h @@ -13,6 +13,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 */ + void jvm_bind(JNIEnv *env, jobject jvm, msg_vm_t vm); msg_vm_t jvm_get_native(JNIEnv *env, jobject jvm); diff --git a/src/bindings/java/org/simgrid/msg/Process.java b/src/bindings/java/org/simgrid/msg/Process.java index 752f41ebc5..f96cf87ed9 100644 --- a/src/bindings/java/org/simgrid/msg/Process.java +++ b/src/bindings/java/org/simgrid/msg/Process.java @@ -306,7 +306,7 @@ public abstract class Process implements Runnable { } } - /** This method runs the process. Il calls the method function that you must overwrite. */ + /** This method runs the process. It calls the method function that you must overwrite. */ @Override public void run() { diff --git a/src/plugins/vm/VirtualMachineImpl.cpp b/src/plugins/vm/VirtualMachineImpl.cpp index b658bfb2ba..356403d34f 100644 --- a/src/plugins/vm/VirtualMachineImpl.cpp +++ b/src/plugins/vm/VirtualMachineImpl.cpp @@ -116,7 +116,10 @@ extern "C" int VirtualMachineImpl::~VirtualMachineImpl() { onVmDestruction(this); - allVms_.erase(find(allVms_.begin(), allVms_.end(), piface_)); + /* I was already removed from the allVms set if the VM was destroyed cleanly */ + auto iter = find(allVms_.begin(), allVms_.end(), piface_); + if (iter != allVms_.end()) + allVms_.erase(iter); /* dirty page tracking */ unsigned int size = xbt_dict_size(dp_objs); diff --git a/src/plugins/vm/VirtualMachineImpl.hpp b/src/plugins/vm/VirtualMachineImpl.hpp index 3ec7abb107..d9b4ed6549 100644 --- a/src/plugins/vm/VirtualMachineImpl.hpp +++ b/src/plugins/vm/VirtualMachineImpl.hpp @@ -21,7 +21,7 @@ namespace vm { ***********/ class XBT_PRIVATE VMModel; -class XBT_PRIVATE VirtualMachineImpl; +XBT_PUBLIC_CLASS VirtualMachineImpl; // Made visible to the Java plugin /************* * Callbacks * @@ -50,7 +50,8 @@ extern XBT_PRIVATE simgrid::xbt::signal * @brief SURF VM interface class * @details A VM represent a virtual machine */ -class VirtualMachineImpl : public surf::HostImpl { +XBT_PUBLIC_CLASS VirtualMachineImpl : public surf::HostImpl +{ friend simgrid::s4u::VirtualMachine; public: -- 2.20.1