[jnigen] Fix windows specifics and global reference ownership in interface implementation (https://github.com/dart-lang/jnigen/issues/335)
Since Dart doesn't know that this global reference is still used, it might garbage collect it via NativeFinalizer thus making it invalid. We're letting Java handle the clean up and use setAsDeleted to detach the native finalizer from the return value in Dart.
diff --git a/pkgs/jni/java/src/main/java/com/github/dart_lang/jni/PortProxy.java b/pkgs/jni/java/src/main/java/com/github/dart_lang/jni/PortProxy.java
index a75a914..bae1c32 100644
--- a/pkgs/jni/java/src/main/java/com/github/dart_lang/jni/PortProxy.java
+++ b/pkgs/jni/java/src/main/java/com/github/dart_lang/jni/PortProxy.java
@@ -62,21 +62,28 @@
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/pkgs/jni/src/dartjni.c b/pkgs/jni/src/dartjni.c
index 6f0fa08..df8feb7 100644
--- a/pkgs/jni/src/dartjni.c
+++ b/pkgs/jni/src/dartjni.c
@@ -636,7 +636,12 @@
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 @@
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 @@
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, "<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/pkgs/jni/src/dartjni.h b/pkgs/jni/src/dartjni.h
index cbb1f7f..8a07d09 100644
--- a/pkgs/jni/src/dartjni.h
+++ b/pkgs/jni/src/dartjni.h
@@ -76,7 +76,7 @@
}
static inline void destroy_cond(ConditionVariable* cond) {
- DeleteCriticalSection(cond);
+ // Not available.
}
#elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \
@@ -435,7 +435,7 @@
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 @@
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/pkgs/jnigen/example/in_app_java/src/android_utils/dartjni.h b/pkgs/jnigen/example/in_app_java/src/android_utils/dartjni.h
index cbb1f7f..8a07d09 100644
--- a/pkgs/jnigen/example/in_app_java/src/android_utils/dartjni.h
+++ b/pkgs/jnigen/example/in_app_java/src/android_utils/dartjni.h
@@ -76,7 +76,7 @@
}
static inline void destroy_cond(ConditionVariable* cond) {
- DeleteCriticalSection(cond);
+ // Not available.
}
#elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \
@@ -435,7 +435,7 @@
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 @@
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/pkgs/jnigen/example/kotlin_plugin/src/dartjni.h b/pkgs/jnigen/example/kotlin_plugin/src/dartjni.h
index cbb1f7f..8a07d09 100644
--- a/pkgs/jnigen/example/kotlin_plugin/src/dartjni.h
+++ b/pkgs/jnigen/example/kotlin_plugin/src/dartjni.h
@@ -76,7 +76,7 @@
}
static inline void destroy_cond(ConditionVariable* cond) {
- DeleteCriticalSection(cond);
+ // Not available.
}
#elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \
@@ -435,7 +435,7 @@
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 @@
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/pkgs/jnigen/example/notification_plugin/src/dartjni.h b/pkgs/jnigen/example/notification_plugin/src/dartjni.h
index cbb1f7f..8a07d09 100644
--- a/pkgs/jnigen/example/notification_plugin/src/dartjni.h
+++ b/pkgs/jnigen/example/notification_plugin/src/dartjni.h
@@ -76,7 +76,7 @@
}
static inline void destroy_cond(ConditionVariable* cond) {
- DeleteCriticalSection(cond);
+ // Not available.
}
#elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \
@@ -435,7 +435,7 @@
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 @@
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/pkgs/jnigen/example/pdfbox_plugin/src/third_party/dartjni.h b/pkgs/jnigen/example/pdfbox_plugin/src/third_party/dartjni.h
index cbb1f7f..8a07d09 100644
--- a/pkgs/jnigen/example/pdfbox_plugin/src/third_party/dartjni.h
+++ b/pkgs/jnigen/example/pdfbox_plugin/src/third_party/dartjni.h
@@ -76,7 +76,7 @@
}
static inline void destroy_cond(ConditionVariable* cond) {
- DeleteCriticalSection(cond);
+ // Not available.
}
#elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \
@@ -435,7 +435,7 @@
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 @@
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/pkgs/jnigen/lib/src/bindings/dart_generator.dart b/pkgs/jnigen/lib/src/bindings/dart_generator.dart
index 3db7c0b..c78a3b2 100644
--- a/pkgs/jnigen/lib/src/bindings/dart_generator.dart
+++ b/pkgs/jnigen/lib/src/bindings/dart_generator.dart
@@ -1534,8 +1534,9 @@
@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 @@
/// 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<String> {
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 @@
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/pkgs/jnigen/test/jackson_core_test/third_party/c_based/c_bindings/dartjni.h b/pkgs/jnigen/test/jackson_core_test/third_party/c_based/c_bindings/dartjni.h
index cbb1f7f..8a07d09 100644
--- a/pkgs/jnigen/test/jackson_core_test/third_party/c_based/c_bindings/dartjni.h
+++ b/pkgs/jnigen/test/jackson_core_test/third_party/c_based/c_bindings/dartjni.h
@@ -76,7 +76,7 @@
}
static inline void destroy_cond(ConditionVariable* cond) {
- DeleteCriticalSection(cond);
+ // Not available.
}
#elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \
@@ -435,7 +435,7 @@
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 @@
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/pkgs/jnigen/test/kotlin_test/c_based/c_bindings/dartjni.h b/pkgs/jnigen/test/kotlin_test/c_based/c_bindings/dartjni.h
index cbb1f7f..8a07d09 100644
--- a/pkgs/jnigen/test/kotlin_test/c_based/c_bindings/dartjni.h
+++ b/pkgs/jnigen/test/kotlin_test/c_based/c_bindings/dartjni.h
@@ -76,7 +76,7 @@
}
static inline void destroy_cond(ConditionVariable* cond) {
- DeleteCriticalSection(cond);
+ // Not available.
}
#elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \
@@ -435,7 +435,7 @@
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 @@
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/pkgs/jnigen/test/simple_package_test/c_based/c_bindings/dartjni.h b/pkgs/jnigen/test/simple_package_test/c_based/c_bindings/dartjni.h
index cbb1f7f..8a07d09 100644
--- a/pkgs/jnigen/test/simple_package_test/c_based/c_bindings/dartjni.h
+++ b/pkgs/jnigen/test/simple_package_test/c_based/c_bindings/dartjni.h
@@ -76,7 +76,7 @@
}
static inline void destroy_cond(ConditionVariable* cond) {
- DeleteCriticalSection(cond);
+ // Not available.
}
#elif defined __APPLE__ || defined __LINUX__ || defined __ANDROID__ || \
@@ -435,7 +435,7 @@
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 @@
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/pkgs/jnigen/test/simple_package_test/c_based/dart_bindings/simple_package.dart b/pkgs/jnigen/test/simple_package_test/c_based/dart_bindings/simple_package.dart
index 7cb0110..11e873e 100644
--- a/pkgs/jnigen/test/simple_package_test/c_based/dart_bindings/simple_package.dart
+++ b/pkgs/jnigen/test/simple_package_test/c_based/dart_bindings/simple_package.dart
@@ -2979,13 +2979,17 @@
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 @@
.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/pkgs/jnigen/test/simple_package_test/dart_only/dart_bindings/simple_package.dart b/pkgs/jnigen/test/simple_package_test/dart_only/dart_bindings/simple_package.dart
index 7befe1b..ef24ce3 100644
--- a/pkgs/jnigen/test/simple_package_test/dart_only/dart_bindings/simple_package.dart
+++ b/pkgs/jnigen/test/simple_package_test/dart_only/dart_bindings/simple_package.dart
@@ -2818,13 +2818,17 @@
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 @@
.castTo(const jni.JDoubleType(), deleteOriginal: true)
.doubleValue(deleteOriginal: true),
);
- return jni.JLong($r).reference;
+ return (jni.JLong($r)..setAsDeleted()).reference;
}
return jni.nullptr;
}
