diff --git a/jni/java/src/main/java/com/github/dart_lang/jni/PortProxy.java b/jni/java/src/main/java/com/github/dart_lang/jni/PortProxy.java index a75a914f..bae1c325 100644 --- a/jni/java/src/main/java/com/github/dart_lang/jni/PortProxy.java +++ b/jni/java/src/main/java/com/github/dart_lang/jni/PortProxy.java @@ -62,21 +62,28 @@ private static void appendType(StringBuilder descriptor, Class type) { public static Object newInstance(String binaryName, long port, long threadId, long functionPtr) throws ClassNotFoundException { - Class clazz = Class.forName(binaryName); + Class clazz = Class.forName(binaryName); return Proxy.newProxyInstance( clazz.getClassLoader(), new Class[] {clazz}, new PortProxy(port, threadId, functionPtr)); } @Override - public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { - return _invoke(port, threadId, functionPtr, proxy, getDescriptor(method), args); + public Object invoke(Object proxy, Method method, Object[] args) { + Object[] result = _invoke(port, threadId, functionPtr, proxy, getDescriptor(method), args); + _cleanUp((Long) result[0]); + return result[1]; } - private native Object _invoke( + /// Returns an array with two objects: + /// [0]: The address of the result pointer used for the clean-up. + /// [1]: The result of the invocation. + private native Object[] _invoke( long port, long threadId, long functionPtr, Object proxy, String methodDescriptor, Object[] args); + + private native void _cleanUp(long resultPtr); } diff --git a/jni/src/dartjni.c b/jni/src/dartjni.c index 6f0fa086..df8feb78 100644 --- a/jni/src/dartjni.c +++ b/jni/src/dartjni.c @@ -636,7 +636,12 @@ void resultFor(CallbackResult* result, jobject object) { release_lock(&result->lock); } -JNIEXPORT jobject JNICALL +jclass _c_Object = NULL; +jclass _c_Long = NULL; + +jmethodID _m_Long_init = NULL; + +JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject thiz, jlong port, @@ -645,9 +650,8 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject proxy, jstring methodDescriptor, jobjectArray args) { - attach_thread(); + CallbackResult* result = (CallbackResult*)malloc(sizeof(CallbackResult)); if (threadId != thread_id()) { - CallbackResult* result = (CallbackResult*)malloc(sizeof(CallbackResult)); init_lock(&result->lock); init_cond(&result->cond); acquire_lock(&result->lock); @@ -674,18 +678,40 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, c_post.value.as_array.length = sizeof(c_post_arr) / sizeof(c_post_arr[0]); Dart_PostCObject_DL(port, &c_post); + while (!result->ready) { wait_for(&result->cond, &result->lock); } + release_lock(&result->lock); destroy_lock(&result->lock); destroy_cond(&result->cond); - jobject object = result->object; - free(result); - return object; } else { - return ((jobject(*)(uint64_t, jobject, jobject))functionPtr)( + result->object = ((jobject(*)(uint64_t, jobject, jobject))functionPtr)( port, (*env)->NewGlobalRef(env, methodDescriptor), (*env)->NewGlobalRef(env, args)); } -} \ No newline at end of file + // Returning an array of length 2. + // [0]: The result pointer, used for cleaning up the global reference, and + // freeing the memory since we passed the ownership to Java. + // [1]: The returned object. + attach_thread(); + load_class_global_ref(&_c_Object, "java/lang/Object"); + load_class_global_ref(&_c_Long, "java/lang/Long"); + load_method(_c_Long, &_m_Long_init, "", "(J)V"); + jobject first = (*env)->NewObject(env, _c_Long, _m_Long_init, (jlong)result); + jobject second = result->object; + jobjectArray arr = (*env)->NewObjectArray(env, 2, _c_Object, NULL); + (*env)->SetObjectArrayElement(env, arr, 0, first); + (*env)->SetObjectArrayElement(env, arr, 1, second); + return arr; +} + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, + jobject thiz, + jlong resultPtr) { + CallbackResult* result = (CallbackResult*)resultPtr; + (*env)->DeleteGlobalRef(env, result->object); + free(result); +} diff --git a/jni/src/dartjni.h b/jni/src/dartjni.h index cbb1f7f5..8a07d091 100644 --- a/jni/src/dartjni.h +++ b/jni/src/dartjni.h @@ -76,7 +76,7 @@ static inline void wait_for(ConditionVariable* cond, MutexLock* lock) { } static inline void destroy_cond(ConditionVariable* cond) { - DeleteCriticalSection(cond); + // Not available. } #elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \ @@ -435,7 +435,7 @@ JniResult PortProxy__newInstance(jobject binaryName, int64_t port, int64_t functionPtr); -JNIEXPORT jobject JNICALL +JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject thiz, jlong port, @@ -444,3 +444,8 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject proxy, jstring methodDescriptor, jobjectArray args); + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, + jobject thiz, + jlong resultPtr); diff --git a/jnigen/example/in_app_java/src/android_utils/dartjni.h b/jnigen/example/in_app_java/src/android_utils/dartjni.h index cbb1f7f5..8a07d091 100644 --- a/jnigen/example/in_app_java/src/android_utils/dartjni.h +++ b/jnigen/example/in_app_java/src/android_utils/dartjni.h @@ -76,7 +76,7 @@ static inline void wait_for(ConditionVariable* cond, MutexLock* lock) { } static inline void destroy_cond(ConditionVariable* cond) { - DeleteCriticalSection(cond); + // Not available. } #elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \ @@ -435,7 +435,7 @@ JniResult PortProxy__newInstance(jobject binaryName, int64_t port, int64_t functionPtr); -JNIEXPORT jobject JNICALL +JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject thiz, jlong port, @@ -444,3 +444,8 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject proxy, jstring methodDescriptor, jobjectArray args); + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, + jobject thiz, + jlong resultPtr); diff --git a/jnigen/example/kotlin_plugin/src/dartjni.h b/jnigen/example/kotlin_plugin/src/dartjni.h index cbb1f7f5..8a07d091 100644 --- a/jnigen/example/kotlin_plugin/src/dartjni.h +++ b/jnigen/example/kotlin_plugin/src/dartjni.h @@ -76,7 +76,7 @@ static inline void wait_for(ConditionVariable* cond, MutexLock* lock) { } static inline void destroy_cond(ConditionVariable* cond) { - DeleteCriticalSection(cond); + // Not available. } #elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \ @@ -435,7 +435,7 @@ JniResult PortProxy__newInstance(jobject binaryName, int64_t port, int64_t functionPtr); -JNIEXPORT jobject JNICALL +JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject thiz, jlong port, @@ -444,3 +444,8 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject proxy, jstring methodDescriptor, jobjectArray args); + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, + jobject thiz, + jlong resultPtr); diff --git a/jnigen/example/notification_plugin/src/dartjni.h b/jnigen/example/notification_plugin/src/dartjni.h index cbb1f7f5..8a07d091 100644 --- a/jnigen/example/notification_plugin/src/dartjni.h +++ b/jnigen/example/notification_plugin/src/dartjni.h @@ -76,7 +76,7 @@ static inline void wait_for(ConditionVariable* cond, MutexLock* lock) { } static inline void destroy_cond(ConditionVariable* cond) { - DeleteCriticalSection(cond); + // Not available. } #elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \ @@ -435,7 +435,7 @@ JniResult PortProxy__newInstance(jobject binaryName, int64_t port, int64_t functionPtr); -JNIEXPORT jobject JNICALL +JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject thiz, jlong port, @@ -444,3 +444,8 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject proxy, jstring methodDescriptor, jobjectArray args); + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, + jobject thiz, + jlong resultPtr); diff --git a/jnigen/example/pdfbox_plugin/src/third_party/dartjni.h b/jnigen/example/pdfbox_plugin/src/third_party/dartjni.h index cbb1f7f5..8a07d091 100644 --- a/jnigen/example/pdfbox_plugin/src/third_party/dartjni.h +++ b/jnigen/example/pdfbox_plugin/src/third_party/dartjni.h @@ -76,7 +76,7 @@ static inline void wait_for(ConditionVariable* cond, MutexLock* lock) { } static inline void destroy_cond(ConditionVariable* cond) { - DeleteCriticalSection(cond); + // Not available. } #elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \ @@ -435,7 +435,7 @@ JniResult PortProxy__newInstance(jobject binaryName, int64_t port, int64_t functionPtr); -JNIEXPORT jobject JNICALL +JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject thiz, jlong port, @@ -444,3 +444,8 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject proxy, jstring methodDescriptor, jobjectArray args); + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, + jobject thiz, + jlong resultPtr); diff --git a/jnigen/lib/src/bindings/dart_generator.dart b/jnigen/lib/src/bindings/dart_generator.dart index 3db7c0bc..c78a3b25 100644 --- a/jnigen/lib/src/bindings/dart_generator.dart +++ b/jnigen/lib/src/bindings/dart_generator.dart @@ -1534,8 +1534,9 @@ class _InterfaceMethodIf extends Visitor { @override void visit(Method node) { + final isVoid = node.returnType.name == 'void'; final signature = node.javaSig; - final saveResult = node.returnType.name == 'void' ? '' : 'final \$r = '; + final saveResult = isVoid ? '' : 'final \$r = '; s.write(''' if (\$d == r"$signature") { ${saveResult}_\$methods[\$p]![\$d]!( @@ -1588,13 +1589,25 @@ class _InterfaceParamCast extends Visitor { /// Only returns the reference for non primitive types. /// Returns null for void. /// -/// For example `$r.toJInteger().reference` when the return type is `integer`. +/// Since Dart doesn't know that this global reference is still used, it might +/// garbage collect it via [NativeFinalizer] thus making it invalid. +/// This passes the ownership to Java using [setAsDeleted]. +/// +/// `..setAsDeleted` detaches the object from the [NativeFinalizer] and Java +/// will clean up the global reference afterwards. +/// +/// For example `($r.toJInteger()..setAsDeleted()).reference` when the return +/// type is `integer`. class _InterfaceReturnBox extends TypeVisitor { const _InterfaceReturnBox(); @override String visitNonPrimitiveType(ReferredType node) { - return '\$r.reference'; + // Casting is done to create a new global reference. The user might + // use the original reference elsewhere and so the original object + // should not be [setAsDeleted]. + return '((\$r as $_jObject).castTo(const ${_jObject}Type())' + '..setAsDeleted()).reference'; } @override @@ -1602,7 +1615,7 @@ class _InterfaceReturnBox extends TypeVisitor { if (node.name == 'void') { return '$_jni.nullptr'; } - return '$_jni.J${node.boxedName}(\$r).reference'; + return '($_jni.J${node.boxedName}(\$r)..setAsDeleted()).reference'; } } diff --git a/jnigen/test/jackson_core_test/third_party/c_based/c_bindings/dartjni.h b/jnigen/test/jackson_core_test/third_party/c_based/c_bindings/dartjni.h index cbb1f7f5..8a07d091 100644 --- a/jnigen/test/jackson_core_test/third_party/c_based/c_bindings/dartjni.h +++ b/jnigen/test/jackson_core_test/third_party/c_based/c_bindings/dartjni.h @@ -76,7 +76,7 @@ static inline void wait_for(ConditionVariable* cond, MutexLock* lock) { } static inline void destroy_cond(ConditionVariable* cond) { - DeleteCriticalSection(cond); + // Not available. } #elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \ @@ -435,7 +435,7 @@ JniResult PortProxy__newInstance(jobject binaryName, int64_t port, int64_t functionPtr); -JNIEXPORT jobject JNICALL +JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject thiz, jlong port, @@ -444,3 +444,8 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject proxy, jstring methodDescriptor, jobjectArray args); + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, + jobject thiz, + jlong resultPtr); diff --git a/jnigen/test/kotlin_test/c_based/c_bindings/dartjni.h b/jnigen/test/kotlin_test/c_based/c_bindings/dartjni.h index cbb1f7f5..8a07d091 100644 --- a/jnigen/test/kotlin_test/c_based/c_bindings/dartjni.h +++ b/jnigen/test/kotlin_test/c_based/c_bindings/dartjni.h @@ -76,7 +76,7 @@ static inline void wait_for(ConditionVariable* cond, MutexLock* lock) { } static inline void destroy_cond(ConditionVariable* cond) { - DeleteCriticalSection(cond); + // Not available. } #elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \ @@ -435,7 +435,7 @@ JniResult PortProxy__newInstance(jobject binaryName, int64_t port, int64_t functionPtr); -JNIEXPORT jobject JNICALL +JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject thiz, jlong port, @@ -444,3 +444,8 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject proxy, jstring methodDescriptor, jobjectArray args); + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, + jobject thiz, + jlong resultPtr); diff --git a/jnigen/test/simple_package_test/c_based/c_bindings/dartjni.h b/jnigen/test/simple_package_test/c_based/c_bindings/dartjni.h index cbb1f7f5..8a07d091 100644 --- a/jnigen/test/simple_package_test/c_based/c_bindings/dartjni.h +++ b/jnigen/test/simple_package_test/c_based/c_bindings/dartjni.h @@ -76,7 +76,7 @@ static inline void wait_for(ConditionVariable* cond, MutexLock* lock) { } static inline void destroy_cond(ConditionVariable* cond) { - DeleteCriticalSection(cond); + // Not available. } #elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \ @@ -435,7 +435,7 @@ JniResult PortProxy__newInstance(jobject binaryName, int64_t port, int64_t functionPtr); -JNIEXPORT jobject JNICALL +JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject thiz, jlong port, @@ -444,3 +444,8 @@ Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, jobject proxy, jstring methodDescriptor, jobjectArray args); + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, + jobject thiz, + jlong resultPtr); diff --git a/jnigen/test/simple_package_test/c_based/dart_bindings/simple_package.dart b/jnigen/test/simple_package_test/c_based/dart_bindings/simple_package.dart index 7cb01108..11e873e5 100644 --- a/jnigen/test/simple_package_test/c_based/dart_bindings/simple_package.dart +++ b/jnigen/test/simple_package_test/c_based/dart_bindings/simple_package.dart @@ -2979,13 +2979,17 @@ class MyInterface<$T extends jni.JObject> extends jni.JObject { final $r = _$methods[$p]![$d]!( $a[0].castTo(const jni.JStringType(), deleteOriginal: true), ); - return $r.reference; + return (($r as jni.JObject).castTo(const jni.JObjectType()) + ..setAsDeleted()) + .reference; } if ($d == r"varCallback(Ljava/lang/Object;)Ljava/lang/Object;") { final $r = _$methods[$p]![$d]!( $a[0].castTo(_$types[$p]!["T"]!, deleteOriginal: true), ); - return $r.reference; + return (($r as jni.JObject).castTo(const jni.JObjectType()) + ..setAsDeleted()) + .reference; } if ($d == r"manyPrimitives(IZCD)J") { final $r = _$methods[$p]![$d]!( @@ -3002,7 +3006,7 @@ class MyInterface<$T extends jni.JObject> extends jni.JObject { .castTo(const jni.JDoubleType(), deleteOriginal: true) .doubleValue(deleteOriginal: true), ); - return jni.JLong($r).reference; + return (jni.JLong($r)..setAsDeleted()).reference; } return jni.nullptr; } diff --git a/jnigen/test/simple_package_test/dart_only/dart_bindings/simple_package.dart b/jnigen/test/simple_package_test/dart_only/dart_bindings/simple_package.dart index 7befe1b9..ef24ce3e 100644 --- a/jnigen/test/simple_package_test/dart_only/dart_bindings/simple_package.dart +++ b/jnigen/test/simple_package_test/dart_only/dart_bindings/simple_package.dart @@ -2818,13 +2818,17 @@ class MyInterface<$T extends jni.JObject> extends jni.JObject { final $r = _$methods[$p]![$d]!( $a[0].castTo(const jni.JStringType(), deleteOriginal: true), ); - return $r.reference; + return (($r as jni.JObject).castTo(const jni.JObjectType()) + ..setAsDeleted()) + .reference; } if ($d == r"varCallback(Ljava/lang/Object;)Ljava/lang/Object;") { final $r = _$methods[$p]![$d]!( $a[0].castTo(_$types[$p]!["T"]!, deleteOriginal: true), ); - return $r.reference; + return (($r as jni.JObject).castTo(const jni.JObjectType()) + ..setAsDeleted()) + .reference; } if ($d == r"manyPrimitives(IZCD)J") { final $r = _$methods[$p]![$d]!( @@ -2841,7 +2845,7 @@ class MyInterface<$T extends jni.JObject> extends jni.JObject { .castTo(const jni.JDoubleType(), deleteOriginal: true) .doubleValue(deleteOriginal: true), ); - return jni.JLong($r).reference; + return (jni.JLong($r)..setAsDeleted()).reference; } return jni.nullptr; }