2

IMPORTANT NOTE: This snippet of code is not a native function called from Java. Our process is written in C++, we instantiate a Java VM, and we call Java functions from C++, but never the other way around: Java methods never calls native functions, but native functions instantiate Java objects and call Java functions on them.

We are doing something like this:

void some_fun()
{
    // env is a JNIEnv*, cls (and cls2) a jclass, and init (and mid) a jmethodID
    jobject obj = env->NewObject(cls, init);
    fill_obj(obj, cpp_data);
    env->callStaticVoidMethod(cls2, mid, obj);
    // env->DeleteLocalRef(obj); // Added out of desperation.
}

Where fill_obj depends on the kind of cpp_data that must be set as fields of obj. For example, if the Java class cls contains an ArrayList, cpp_data will contain, for example, a std::vector. So the fill_obj overload will look like this:

void fill_obj(jobject obj, SomeType const& cpp_data)
{
   std::vector<SomeSubType> const& v = cpp_data.inner_data;
   jobject list_obj = env->NewObject(array_list_class_global_ref, its_init_method);

   for (auto it = v.begin(); it != v.end(); ++it) {
      jobject child_obj = env->NewObject(SomeSubType_class_global_ref, its_init_method);
      fill_obj(child_obj , *it);
      env->CallBooleanMethod(list_obj, add_method, child_obj);
   }

   env->SetObjectField(obj, field_id, list_obj);
}

According to our understanding on the JNI docs, that code should not have any memory leaks since local objects are destroyed when the native method ends. So, when the first fill_obj ends, at C++ side there's no more references to list_obj or any of its childs, and when some_fun ends, any reference of obj at C++ side is gone as well.

My understanding is that jobject contains some sort of reference counter and when that reference counter reachs 0, then there's no more references to the Java object at C++ side. If there's no more references to the object at Java side either, then the Java's garbage collector is free to release the resources occupied by the object.

We have a method that, when called, creates thousand of those objects, and each time that method is called, the memory that the process occupies in RAM (the resident memory) grows by more than 200 MiB, and that memory is never released.

We have added an explicit call to DeleteLocalRef but the result is the same.

What is going on? Are we doing something wrong?

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
ABu
  • 10,423
  • 6
  • 52
  • 103
  • *that code should not have any memory leaks* -- How do you know the issue isn't with the C++ code itself, and has nothing to do with Java? For example, a `std::vector`, depending on the type you store in it, can leak memory if the type does not respect the [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). Actually, not vector, but placing faulty objects in the vector can leak memory. – PaulMcKenzie Feb 01 '22 at 17:29
  • @PaulMcKenzie Inside the `std::vector` we have C++ objects, no pointers to objects or references to Java objects. But yes, there could be leaks at C++ side, not related to JNI, but that's hard to be true because we have literally not a single manual call to `malloc` or `new`, excepts a few (three or four) `new` calls for some objects that we store on `shared_ptr`s in the same sentence. Anything else use strict scoping rules. – ABu Feb 01 '22 at 17:42
  • @Holger fixed. I meant `list_obj`. – ABu Feb 01 '22 at 17:45
  • *and that memory is never released.* -- That is not the determining factor that this is a memory leak. If you called this function 10 times, does the memory increase 200MB each time, thus totaling 2 MB or more of memory being used? If not, then what you are seeing is how the heap manager works, and that is by reusing the same allocated space for new allocations, instead of releasing it back to the OS. – PaulMcKenzie Feb 01 '22 at 17:52
  • @PaulMcKenzie Yes, it grows by 200MiB each time. – ABu Feb 01 '22 at 17:59
  • 1
    Also, your line of "desperation" should have also been placed within the `for` loop for the `child_obj` if you want to test if that works. – PaulMcKenzie Feb 01 '22 at 18:10
  • @PaulMcKenzie right we are revisiting if we missed some call on any of our hundreds of `fill_obj` overloads. I hope that's the reason. It's also true that in some causes we are using inline functions that returns copies of `jobject` in factory-like functions that creates and returns the object, under our assumption that jobjects work like shared_ptrs. – ABu Feb 01 '22 at 18:17
  • 1
    `jobject` does not work like `shared_ptr`. Mind that the JNI API does work equally with C; there’s no smartness in the `jobject`, especially none that would require C++. When [the JNI spec says](https://docs.oracle.com/en/java/javase/17/docs/specs/jni/design.html#global-and-local-references) that “*local references are valid for the duration of a native method call, and are automatically freed after the native method returns*”, it means “when the Java call to a method declared `native` returns to the Java caller”. Then, all local references are freed at once. JNI has no notion of C++ scopes. – Holger Feb 01 '22 at 19:18
  • @Holger the point is we are never calling C++ inside Java. We are mostly C++ developers, but need Java for a library not available for C++. So all of our Java/C++ interaction is from C++ to Java. Java classes never call native methods in our case. – ABu Feb 01 '22 at 19:38
  • 3
    Then, there is no way around calling `DeleteLocalRef` for *every* local reference you created. – Holger Feb 01 '22 at 19:40
  • 1
    @Holger or, you can use `PushLocalFrame()` and `PopLocalFrame()` to free all local references at one time. – Remy Lebeau Feb 01 '22 at 19:43
  • 2
    @Holger I think you could promote your comment as an answer. Most of our confusion comes from the fact that the docs are mostly oriented from the point of view of Java calling native functions. – ABu Feb 01 '22 at 19:44
  • @RemyLebeau That sounds very interesting, and sounds like very easy to integrate with our existing code. Could you elaborate about that, maybe as an answer? I would love to see an usage example with my `some_fun`. – ABu Feb 01 '22 at 19:48
  • 1
    @Peregring-lk -- So I guess my wild guess basically worked? Actually, it is a leak in terms of C++, since `jobject` has no idea of RAII. As a matter of fact, I highly suggest you create a class that wraps the jobject and automatically on the destructor, removes the object (of course you use this in cases like this, and not ones where Java will do the delete). Your current code basically remains the same, only `jobject` would be some sort of RAII object. When writing JNI code, you have to use the RAII paradigm as much as possible, at least that's my experience. – PaulMcKenzie Feb 01 '22 at 20:13
  • @PaulMcKenzie Haven't test it yet; will try tomorrow at work. About RAII, well, our jobjects are actually encapsulated, but our wrappers act more like reference_wrappers than RAII objects, and its a bit late for us to change that. We also have a wrapper for jclass, acting like a factory: we request an object to the class and we return it back for release, but we let tons of objects without releasing because of a mix of bad understanding of JNI and Java. We assumed somehow that releasing a "parent object", automagically released the full tree of child objects (like releasing a widget). – ABu Feb 01 '22 at 21:12

2 Answers2

5

Since you say your some_fun() function is not being called by Java code, it is just pure C++ code that is internally calling into JNI, then yes, you will need to call DeleteLocalRef() on any local references you hold to Java objects that you don't pass back to Java. You will need to do this inside of your for loop as well.

Try this:

void some_fun()
{
    // env is a JNIEnv*, cls (and cls2) a jclass, and init (and mid) a jmethodID
    jobject obj = env->NewObject(cls, init); // <-- local ref created
    fill_obj(obj, cpp_data);
    env->callStaticVoidMethod(cls2, mid, obj);
    env->DeleteLocalRef(obj); // <-- local ref released
}

void fill_obj(jobject obj, SomeType const& cpp_data)
{
   std::vector<SomeSubType> const& v = cpp_data.inner_data;

   jobject list_obj = env->NewObject(array_list_class_global_ref, its_init_method); // <-- local ref created

   for (auto it = v.begin(); it != v.end(); ++it) {
      jobject child_obj = env->NewObject(SomeSubType_class_global_ref, its_init_method); // <-- local ref created
      fill_obj(child_obj, *it);
      env->CallBooleanMethod(list_obj, add_method, child_obj);
      env->DeleteLocalRef(child_obj); // <-- local ref released
   }

   env->SetObjectField(obj, field_id, list_obj);
   env->DeleteLocalRef(list_obj); // <-- local ref released
}

Alternatively, you can use (Push|Pop)LocalFrame() instead, so that JNI can keep track of all the local references you create and then release them for you in one go, eg:

void some_fun()
{
    // env is a JNIEnv*, cls (and cls2) a jclass, and init (and mid) a jmethodID

    env->PushLocalFrame(1);

    jobject obj = env->NewObject(cls, init);
    fill_obj(obj, cpp_data);
    env->callStaticVoidMethod(cls2, mid, obj);

    env->PopLocalFrame(NULL);
}

void fill_obj(jobject obj, SomeType const& cpp_data)
{
   std::vector<SomeSubType> const& v = cpp_data.inner_data;

   env->PushLocalFrame(1+v.size());

   jobject list_obj = env->NewObject(array_list_class_global_ref, its_init_method);

   for (auto it = v.begin(); it != v.end(); ++it) {
      jobject child_obj = env->NewObject(SomeSubType_class_global_ref, its_init_method);
      fill_obj(child_obj, *it);
      env->CallBooleanMethod(list_obj, add_method, child_obj);
   }

   env->SetObjectField(obj, field_id, list_obj);

   env->PopLocalFrame(NULL);
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • I marked your answer as the right one originally because the push/pop local frame approach is the one that we finally used to avoid leaks, but I changed my mind because the @Holger answer goes more in deepth with my original question (are we releasing objects properly **with our current approach**"), even if we finally moved to use yours. – ABu Feb 07 '22 at 18:59
4

Note the section Implementing Local References from the JNI specification:

To implement local references, the Java VM creates a registry for each transition of control from Java to a native method. A registry maps nonmovable local references to Java objects, and keeps the objects from being garbage collected. All Java objects passed to the native method (including those that are returned as the results of JNI function calls) are automatically added to the registry. The registry is deleted after the native method returns, allowing all of its entries to be garbage collected.

The term “after the native method returns” means the reversal of the “transition of control from Java to a native method”, in other words, the method declared inside a Java class using the keyword native has been called and returns to the caller.

Once this is understood, it’s also clear how to read the Global and Local References section

Local references are valid for the duration of a native method call, and are automatically freed after the native method returns.

So when you have code that runs for a long time before returning to the Java caller or has no Java caller at all, there is no way around deleting references manually using DeleteLocalRef.

As Remy Lebeau mentioned, you can also use PushLocalFrame and PopLocalFrame to perform bulk destruction of references created between two points.

Mind that the destruction of the reference does not imply the destruction of the referenced object, but just not to prevent the garbage collection of the object. So after calling env -> CallBooleanMethod(list_obj, add_method, child_obj);, you can destroy the child_obj reference, because the object is still referenced by the list you’re referencing via list_obj, so the added object will be kept as long as it is still in use.

Holger
  • 285,553
  • 42
  • 434
  • 765
  • 2
    "*[`PushLocalFrame` and `PopLocalFrame`] ... requires to know beforehand the **maximum** number of references you’ll create between these points*" - no, it doesn't. You specify the *minimum* you think you will need, but you can create more, they will still be added to the current frame. The capacity is dynamic, similar to `std::vector` – Remy Lebeau Feb 01 '22 at 20:06
  • @RemyLebeau thanks for the correction. – Holger Feb 01 '22 at 20:07