memory leak while calling java from c++ with jni

Question

I have a Java-Application, that merges some large files. The Java-Application isn't under my control. The result from the Java-Applicationis returned as a approimatly 90 Mb large string to my C++ Program where I use it in some algorythms. I call the execute-mathod several times. My problem is, every time I call the Java-Application it reserves more memory but doesnt free it. From this Garbage collection and JNI call question I had the idea to call the garbage collector manually but it frees no memory. Any idea to fix that problem?

Here is my C++-Program

void JavaWrapperClass::CreateVM(string Classpath)
{
        Classpath.insert(0,"-Djava.class.path=");
                             // Pointer to native interface
        //================== prepare loading of Java VM ============================
        JavaVMInitArgs vm_args;                        // Initialization arguments
        JavaVMOption* options = new JavaVMOption[2];   // JVM invocation options

        options[0].optionString =const_cast<char*>(Classpath.c_str()); // where to find java .class
        string maxMemOption=string("-Xmx")+to_string(logicalSolverMaxMem)+"m";
        options[1].optionString=const_cast<char*>(maxMemOption.c_str());

        vm_args.version = JNI_VERSION_1_8;             // minimum Java version
        vm_args.nOptions = 2;                          // number of options
        vm_args.options = options;
        vm_args.ignoreUnrecognized = false;     // invalid options make the JVM init fail

        //=============== load and initialize Java VM and JNI interface =============
        jint rc = JNI_CreateJavaVM(&jvm, (void**) &env, &vm_args);  // YES !!
        delete options;    // we then no longer need the initialisation options.
        if (rc != JNI_OK)
        {
            throw bad_exception();
        }
}

const string* JavaWrapperClass::Execute(const string& Filename, const string& HV, const string& NV,
        const string& FileId)
{

    mergedFilesStr.erase();
    mergedFilesStr.shrink_to_fit();


    jclass javaClass = env->FindClass("Path_to/My_Class");  // try to find the class
    if (javaClass == nullptr)
    {
        throw JavaWrapper_JNI_runtime_exception("class Path_to/My_Class not initialized!");
    }

    jmethodID ctor = env->GetMethodID(javaClass, "<init>", "()V");  // FIND AN OBJECT CONSTRUCTOR
    if (ctor == nullptr)
    {
        env->DeleteLocalRef(javaClass);
        throw JavaWrapper_JNI_runtime_exception("Constructor not found");
    }

    jobject javaObject;
    javaObject = env->NewObject(javaClass, ctor);
    if (javaObject==nullptr)
    {
        env->DeleteLocalRef(javaClass);
        throw JavaWrapper_JNI_runtime_exception("Could not create Java-Object");
    }

    jmethodID mid =
            env->GetMethodID(javaClass, "execute",
                    "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;"); // find method
    if (mid == nullptr)
    {
        env->DeleteLocalRef(javaObject);
        env->DeleteLocalRef(javaClass);
        throw JavaWrapper_JNI_runtime_exception("Method string execute(String odx_Filename, String HV, String NV, String FileId) not found !");
    }
    else
    {
        logger->debug("Found JAVA method execute. => Call execute");
        jstring filename = env->NewStringUTF(Filename.c_str());
        jstring hv = env->NewStringUTF(HV.c_str());
        jstring nv = env->NewStringUTF(NV.c_str());
        jstring FileId = env->NewStringUTF(FileId.c_str());
        jstring retString = (jstring) env->CallObjectMethod(javaObject, 
            mid, filename, hv, nv, FileId);   // call the method "execute" with arguments.

        jboolean isCopy=JNI_TRUE;
        const char *mergedFilesPtr;
        mergedFilesPtr = env->GetStringUTFChars(retString, &isCopy);
        mergedFilesStr= new string(mergedFilesPtr);


        if (isCopy == JNI_TRUE) 
        {
            //Release memory from Return-String
            env->ReleaseStringUTFChars(retString, mergedFilesPtr);
        }
        callGarbageCollector();

        env->DeleteLocalRef(filename);
        env->DeleteLocalRef(hv);
        env->DeleteLocalRef(nv);
        env->DeleteLocalRef(FileId);
    }

    env->DeleteLocalRef(javaObject);
    env->DeleteLocalRef(javaClass);
    callGarbageCollector();

    return &mergedFilesStr;
}

void JavaWrapperClass::callGarbageCollector()
{
    jclass    systemClass    = nullptr;
    jmethodID systemGCMethod = nullptr;

    systemClass    = env->FindClass("java/lang/System");
    systemGCMethod = env->GetStaticMethodID(systemClass, "gc", "()V");
    env->CallStaticVoidMethod(systemClass, systemGCMethod);

    env->DeleteLocalRef(systemClass);
}

`mergedFilesStr` seem to be recreated anyway whithout being deleted in `mergedFilesStr = new string(mergedFilesPtr);` — dvhh, Apr 10 '18 at 09:42
Are you editing your question to clear leaks? you are invalidating answers by doing so. Please review your code before posting it online. — HMD, Apr 10 '18 at 10:11
@Hamed Sorry, I was too slow with editing the code. I reversed my last editing. Now it should be ok to all Answers — Heiko P, Apr 10 '18 at 10:27
I already answered you, but as @AlanBirtles mentioned the problem here is `mergedFilesStr` is not a pointer type that you can use `new` operator on it, so this code will not even compile in first place. Please double check and compile your code then post it online to get accurate answers. — HMD, Apr 10 '18 at 10:31
BTW, I don't mind deleting my answer, please just review your code and post the correct version of it that you have leak on it, I didn't say reverse to the wrong version, If you are not used `new` for `mergedFilesStr` and it's not pointer, it's ok, I will delete my answer. Just post the right code. — HMD, Apr 10 '18 at 10:38
*the Java-Applicationis returned as a approimatly 90 Mb large string to my C++ Program* For something like that, just run the Java application in a child process via something like `popen()` if you're running on Linux. Just have the Java application write the data to `stdout`, and read the output from `popen()`. — Andrew Henle, Apr 10 '18 at 11:27

HMD · Answer 1 · 2018-04-10T10:03:59.273

0

There are a couple issues with your memory handling here:

in void JavaWrapperClass::CreateVM(string Classpath) function, you are using new[] operator on options variable but then you are deleting it using delete. This is UB. you must use delete[] to delete this pointer correctly.
in const string* JavaWrapperClass::Execute(const string& Filename, const string& HV, const string& NV, const string& FileId) function you are returning a pointer to string named mergedFilesStr. In your function buddy you are allocating this pointer and then returning it, So you need to delete it after you are done using it (do you?). I can see you used erase() and shrink_to_fit() in the beginning of your function but beware that this function has nothing to do with deleting your allocated pointer, So you need to delete it your self like this : delete mergedFilesStr;

Anyway the better approach is using smart pointers. you can avoid this kind of manual memory management problems by using them, They are awesome.

edited Apr 10 '18 at 10:03

answered Apr 10 '18 at 09:58

HMD

2,202
6
24
37

Thank you for your help. The problem isn't in the mergedFilesStr. For testing I didn't copied mergedFilesPtr to mergedFilesStr and returned an empty String. Same problem. Your tip with spart pointers i will try. – Heiko P Apr 10 '18 at 10:44
@HeikoP You are welcome, But I think you are getting it wrong, empty string means `""`, but this has noting to do with allocating pointer, increasing memory in time is not about what this string contains, it's about the pointer you are using dynamic allocation on it using `new`. So if by empty string you mean `nullptr` and you removed `new` operator and leak is still there, you are right, but if you just using empty string and you are still using `new`, I say that leak is still there. – HMD Apr 10 '18 at 10:50

Alan Birtles · Answer 2 · 2018-04-10T10:05:36.983

0

You should always call ReleaseStringUTFChars regardless of whether isCopy is true or not: Should you call ReleaseStringUTFChars if GetStringUTFChars returned a copy?.

I don't know how mergedFilesStr is being decalred/used but it could also be the source of your leak. Your example seems to use it as an object at the beginning of ::Execute but then as a pointer later on? In fact I don't see how the code as posted even compiles.

edited Apr 10 '18 at 10:05

answered Apr 10 '18 at 09:59

Alan Birtles

32,622
4
31
60

isCopy: Yes, it's just some information about an implementation detail. Commonly, one would pass NULL to ignore this "optional output parameter". – Tom Blodget Apr 10 '18 at 16:15

memory leak while calling java from c++ with jni

2 Answers2