Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The free_callback function has a JNI weak global reference leak problem #1586

Closed
xiezhaokun opened this issue Jan 9, 2024 · 6 comments
Closed

Comments

@xiezhaokun
Copy link
Contributor

xiezhaokun commented Jan 9, 2024

Version of JNA and related jars

all version

Version and vendor of the java virtual machine

all

Operating system

all

System architecture (CPU type, bitness of the JVM)

Complete description of the problem

The number of arg_classes created by create_callback and the number of arg_classes released by create_callback are inconsistent.

for (i=0;i < argc;i++) {
    int jtype;
    jclass cls = (*env)->GetObjectArrayElement(env, arg_classes, i);
    if ((cb->conversion_flags[i] = get_conversion_flag(env, cls)) != CVT_DEFAULT) {
      cb->arg_classes[i] = (*env)->NewWeakGlobalRef(env, cls);
      cvt = 1;
    }
    else {
      cb->arg_classes[i] = NULL;
    }
...
}

When direct is false , free the cb->arg_classes , but not free JNI weak reference.
https://github.com/java-native-access/jna/blob/master/native/callback.c#L219C3-L224

  if (!direct || !cvt) {
    free(cb->conversion_flags);
    cb->conversion_flags = NULL;
    free(cb->arg_classes);
    cb->arg_classes = NULL;
  }
void 
free_callback(JNIEnv* env, callback *cb) {
  (*env)->DeleteWeakGlobalRef(env, cb->object);
  ffi_closure_free(cb->closure);
  free(cb->arg_types);
  if (cb->arg_classes) {
    unsigned i;
    for (i=0;i < cb->cif.nargs;i++) {
      if (cb->arg_classes[i]) {
        (*env)->DeleteWeakGlobalRef(env, cb->arg_classes[i]);
      }
    }
    free(cb->arg_classes);
  }
...
}

Steps to reproduce

native code
#include <stdio.h>

typedef void(*callback_function)(int, void *);

void set_callback(callback_function callback) {
    // Call Java callback function with parameter 42
    callback(42, NULL);
}
gcc -shared -fPIC -o libjnatest.so jna_test.c
java code
  • MyCallback
package com.sun.jna;

public interface MyCallback extends Callback {
    void callback(int id, Pointer pointer);
}
  • MyCallbackImpl
package com.sun.jna;

public class MyCallbackImpl implements MyCallback {
    @Override
    public void callback(int id, Pointer pointer) {
//        System.out.println(pointer);
    }
}
  • Example
package com.sun.jna;

public class Example {

   // static MyCallback myCallback = new MyCallbackImpl();
    public interface MyLibrary extends Library {
        MyLibrary INSTANCE = Native.loadLibrary(System.getProperty("user.dir") + "/libjnatest.so", MyLibrary.class);

        void set_callback(Callback myCallback);
    }

    public static void main(String[] args) throws InterruptedException {
        System.out.println();
        System.out.println("--------------------start--------------------");
        test(1000000);
        System.out.println("--------------------end--------------------");
        Thread.sleep(1000000L);
    }

    static void test(int loop) {

        for (int i = 0; i < loop; i++) {
            MyCallback myCallback = new MyCallbackImpl();
            MyLibrary.INSTANCE.set_callback(myCallback);
            CallbackReference callbackReference = CallbackReference.callbackMap.get(myCallback);
            // free reference
            callbackReference.dispose();
        }
    }
}
@matthiasblaesing
Copy link
Member

Thank you. I think I see the reasoning in the structure. However, could you pleas check the example again? When I run it, I immediately get a segfauls after the first iteration. This happens for JDKs 11, 17 and 21.

@matthiasblaesing
Copy link
Member

With this additional change:

--- a/src/com/sun/jna/CallbackReference.java
+++ b/src/com/sun/jna/CallbackReference.java
@@ -508,7 +508,7 @@
         Map<Callback, CallbackReference> map = direct ? directCallbackMap : callbackMap;
         synchronized(pointerCallbackMap) {
             CallbackReference cbref = map.get(cb);
-            if (cbref == null) {
+            if (cbref == null || cbref.cbstruct == null) {
                 cbref = new CallbackReference(cb, callingConvention, direct);
                 map.put(cb, cbref);
                 pointerCallbackMap.put(cbref.getTrampoline(),

I can run the sample in a stable manner and reproduce the problem. In my test I see 700MB RSS vs. 385MB RSS with 10.000.000 iterations.

One final question @xiezhaokun: Could you check your author information: xiezhaokun <1016340276@qq.com>. Is xiezhaokun really your full name (given and familyname) and is the email a valid email address?

@xiezhaokun
Copy link
Contributor Author

With this additional change:

--- a/src/com/sun/jna/CallbackReference.java
+++ b/src/com/sun/jna/CallbackReference.java
@@ -508,7 +508,7 @@
         Map<Callback, CallbackReference> map = direct ? directCallbackMap : callbackMap;
         synchronized(pointerCallbackMap) {
             CallbackReference cbref = map.get(cb);
-            if (cbref == null) {
+            if (cbref == null || cbref.cbstruct == null) {
                 cbref = new CallbackReference(cb, callingConvention, direct);
                 map.put(cb, cbref);
                 pointerCallbackMap.put(cbref.getTrampoline(),

I can run the sample in a stable manner and reproduce the problem. In my test I see 700MB RSS vs. 385MB RSS with 10.000.000 iterations.

One final question @xiezhaokun: Could you check your author information: xiezhaokun <1016340276@qq.com>. Is xiezhaokun really your full name (given and familyname) and is the email a valid email address?

Yes, my user information is all valid.

@xiezhaokun
Copy link
Contributor Author

Thank you. I think I see the reasoning in the structure. However, could you pleas check the example again? When I run it, I immediately get a segfauls after the first iteration. This happens for JDKs 11, 17 and 21.

The test case I used jna 5.14.0 version to test

@xiezhaokun
Copy link
Contributor Author

Thank you. I think I see the reasoning in the structure. However, could you pleas check the example again? When I run it, I immediately get a segfauls after the first iteration. This happens for JDKs 11, 17 and 21.

I updated the example.

@matthiasblaesing
Copy link
Member

Fixed via 4bf9f92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants