Skip to content

Commit

Permalink
[jnigen] Fix windows specifics and global reference ownership in inte…
Browse files Browse the repository at this point in the history 
…rface implementation (dart-archive/jnigen#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.
  • Loading branch information
@HosseinYousefi
HosseinYousefi authored Jul 31, 2023
1 parent 7e690dd commit ef33349
Show file tree
Hide file tree
Showing 13 changed files with 132 additions and 38 deletions.
15 changes: 11 additions & 4 deletions pkgs/jni/java/src/main/java/com/github/dart_lang/jni/PortProxy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
42 changes: 34 additions & 8 deletions pkgs/jni/src/dartjni.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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));
}
}
// 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);
}
9 changes: 7 additions & 2 deletions pkgs/jni/src/dartjni.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ || \
Expand Down Expand Up @@ -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,
Expand All @@ -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);
9 changes: 7 additions & 2 deletions pkgs/jnigen/example/in_app_java/src/android_utils/dartjni.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ || \
Expand Down Expand Up @@ -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,
Expand All @@ -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);
9 changes: 7 additions & 2 deletions pkgs/jnigen/example/kotlin_plugin/src/dartjni.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ || \
Expand Down Expand Up @@ -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,
Expand All @@ -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);
9 changes: 7 additions & 2 deletions pkgs/jnigen/example/notification_plugin/src/dartjni.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ || \
Expand Down Expand Up @@ -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,
Expand All @@ -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);
9 changes: 7 additions & 2 deletions pkgs/jnigen/example/pdfbox_plugin/src/third_party/dartjni.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ || \
Expand Down Expand Up @@ -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,
Expand All @@ -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);
21 changes: 17 additions & 4 deletions pkgs/jnigen/lib/src/bindings/dart_generator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1534,8 +1534,9 @@ class _InterfaceMethodIf extends Visitor<Method, void> {

@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]!(
Expand Down Expand Up @@ -1588,21 +1589,33 @@ class _InterfaceParamCast extends Visitor<Param, void> {
/// 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
String visitPrimitiveType(PrimitiveType node) {
if (node.name == 'void') {
return '$_jni.nullptr';
}
return '$_jni.J${node.boxedName}(\$r).reference';
return '($_jni.J${node.boxedName}(\$r)..setAsDeleted()).reference';
}
}

Expand Down
9 changes: 7 additions & 2 deletions pkgs/jnigen/test/jackson_core_test/third_party/c_based/c_bindings/dartjni.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ || \
Expand Down Expand Up @@ -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,
Expand All @@ -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);
9 changes: 7 additions & 2 deletions pkgs/jnigen/test/kotlin_test/c_based/c_bindings/dartjni.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ || \
Expand Down Expand Up @@ -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,
Expand All @@ -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);
9 changes: 7 additions & 2 deletions pkgs/jnigen/test/simple_package_test/c_based/c_bindings/dartjni.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ || \
Expand Down Expand Up @@ -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,
Expand All @@ -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);
10 changes: 7 additions & 3 deletions pkgs/jnigen/test/simple_package_test/c_based/dart_bindings/simple_package.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]!(
Expand All @@ -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;
}
Expand Down
Loading

0 comments on commit ef33349

Please sign in to comment.