[jnigen] Finalize interfaces in Java (https://github.com/dart-lang/jnigen/issues/369)
diff --git a/pkgs/jni/java/src/main/java/com/github/dart_lang/jni/PortCleaner.java b/pkgs/jni/java/src/main/java/com/github/dart_lang/jni/PortCleaner.java new file mode 100644 index 0000000..161ef5e --- /dev/null +++ b/pkgs/jni/java/src/main/java/com/github/dart_lang/jni/PortCleaner.java
@@ -0,0 +1,88 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +package com.github.dart_lang.jni; + +import java.lang.ref.PhantomReference; +import java.lang.ref.ReferenceQueue; + +/// A registry of Java objects with associated Dart resources that cleans up the +/// resources after they get unreachable and collected by the garbage collector. +/// +/// A simple alternative to [java.lang.ref.Cleaner] which is only available in +/// Android API level 33+. +class PortCleaner { + static { + System.loadLibrary("dartjni"); + } + + private final ReferenceQueue<Object> queue = new ReferenceQueue<>(); + private final PortPhantom list = new PortPhantom(); + + private class PortPhantom extends PhantomReference<Object> { + final long port; + + /// Form a linked list. + PortPhantom prev = this, next = this; + + PortPhantom(Object referent, long port) { + super(referent, queue); + this.port = port; + insert(); + } + + /// Only used for the head of the list. + PortPhantom() { + super(null, null); + this.port = 0; + } + + void insert() { + synchronized (list) { + prev = list; + next = list.next; + next.prev = this; + list.next = this; + } + } + + private void remove() { + synchronized (list) { + next.prev = prev; + prev.next = next; + prev = this; + next = this; + } + } + } + + PortCleaner() { + // Only a single PortCleaner and therefore thread will be created. + Thread thread = + new Thread( + () -> { + while (true) { + try { + PortPhantom portPhantom = (PortPhantom) queue.remove(); + portPhantom.remove(); + if (portPhantom.port != 0) { + clean(portPhantom.port); + } + } catch (Throwable e) { + // Ignore. + } + } + }, + "PortCleaner"); + thread.setDaemon(true); + thread.start(); + } + + /// Registers [obj] to be cleaned up later by sending a signal through [port]. + void register(Object obj, long port) { + new PortPhantom(obj, port); + } + + private static native void clean(long port); +}
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 77cff13..ff9c076 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
@@ -11,6 +11,7 @@ System.loadLibrary("dartjni"); } + private static final PortCleaner cleaner = new PortCleaner(); private final long port; private final long isolateId; private final long functionPtr; @@ -23,48 +24,53 @@ private static String getDescriptor(Method method) { StringBuilder descriptor = new StringBuilder(); - descriptor.append(method.getName()).append("("); + descriptor.append(method.getName()).append('('); Class<?>[] parameterTypes = method.getParameterTypes(); for (Class<?> paramType : parameterTypes) { appendType(descriptor, paramType); } - descriptor.append(")"); + descriptor.append(')'); appendType(descriptor, method.getReturnType()); return descriptor.toString(); } private static void appendType(StringBuilder descriptor, Class<?> type) { if (type == void.class) { - descriptor.append("V"); + descriptor.append('V'); } else if (type == boolean.class) { - descriptor.append("Z"); + descriptor.append('Z'); } else if (type == byte.class) { - descriptor.append("B"); + descriptor.append('B'); } else if (type == char.class) { - descriptor.append("C"); + descriptor.append('C'); } else if (type == short.class) { - descriptor.append("S"); + descriptor.append('S'); } else if (type == int.class) { - descriptor.append("I"); + descriptor.append('I'); } else if (type == long.class) { - descriptor.append("J"); + descriptor.append('J'); } else if (type == float.class) { - descriptor.append("F"); + descriptor.append('F'); } else if (type == double.class) { - descriptor.append("D"); + descriptor.append('D'); } else if (type.isArray()) { descriptor.append('['); appendType(descriptor, type.getComponentType()); } else { - descriptor.append("L").append(type.getName().replace('.', '/')).append(";"); + descriptor.append('L').append(type.getName().replace('.', '/')).append(';'); } } public static Object newInstance(String binaryName, long port, long isolateId, long functionPtr) throws ClassNotFoundException { Class<?> clazz = Class.forName(binaryName); - return Proxy.newProxyInstance( - clazz.getClassLoader(), new Class[] {clazz}, new PortProxy(port, isolateId, functionPtr)); + Object obj = + Proxy.newProxyInstance( + clazz.getClassLoader(), + new Class[] {clazz}, + new PortProxy(port, isolateId, functionPtr)); + cleaner.register(obj, port); + return obj; } @Override @@ -77,7 +83,7 @@ /// 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( + private static native Object[] _invoke( long port, long isolateId, long functionPtr, @@ -85,5 +91,5 @@ String methodDescriptor, Object[] args); - private native void _cleanUp(long resultPtr); + private static native void _cleanUp(long resultPtr); }
diff --git a/pkgs/jni/src/dartjni.c b/pkgs/jni/src/dartjni.c index 5c34c36..dc595a9 100644 --- a/pkgs/jni/src/dartjni.c +++ b/pkgs/jni/src/dartjni.c
@@ -571,7 +571,7 @@ JNIEXPORT void JNICALL Java_com_github_dart_1lang_jni_PortContinuation__1resumeWith(JNIEnv* env, - jobject thiz, + jclass clazz, jlong port, jobject result) { attach_thread(); @@ -643,7 +643,7 @@ JNIEXPORT jobjectArray JNICALL Java_com_github_dart_1lang_jni_PortProxy__1invoke(JNIEnv* env, - jobject thiz, + jclass clazz, jlong port, jlong isolateId, jlong functionPtr, @@ -709,9 +709,18 @@ JNIEXPORT void JNICALL Java_com_github_dart_1lang_jni_PortProxy__1cleanUp(JNIEnv* env, - jobject thiz, + jclass clazz, jlong resultPtr) { CallbackResult* result = (CallbackResult*)resultPtr; (*env)->DeleteGlobalRef(env, result->object); free(result); } + +JNIEXPORT void JNICALL +Java_com_github_dart_1lang_jni_PortCleaner_clean(JNIEnv* env, + jclass clazz, + jlong port) { + Dart_CObject close_signal; + close_signal.type = Dart_CObject_kNull; + Dart_PostCObject_DL(port, &close_signal); +}
diff --git a/pkgs/jnigen/lib/src/bindings/dart_generator.dart b/pkgs/jnigen/lib/src/bindings/dart_generator.dart index 04d095b..3bc72cc 100644 --- a/pkgs/jnigen/lib/src/bindings/dart_generator.dart +++ b/pkgs/jnigen/lib/src/bindings/dart_generator.dart
@@ -410,30 +410,18 @@ s.write(''' /// Maps a specific port to the implemented interface. static final Map<int, $implClassName> _\$impls = {}; +'''); + s.write(r''' + ReceivePort? _$p; - ReceivePort? _\$p; - - static final Finalizer<ReceivePort> _\$finalizer = Finalizer((\$p) { - _\$impls.remove(\$p.sendPort.nativePort); - \$p.close(); - }); - - @override - void delete() { - _\$impls.remove(_\$p?.sendPort.nativePort); - _\$p?.close(); - _\$finalizer.detach(this); - super.delete(); - } - - static jni.JObjectPtr _\$invoke( + static jni.JObjectPtr _$invoke( int port, jni.JObjectPtr descriptor, jni.JObjectPtr args, ) { - return _\$invokeMethod( + return _$invokeMethod( port, - \$MethodInvocation.fromAddresses( + $MethodInvocation.fromAddresses( 0, descriptor.address, args.address, @@ -445,14 +433,14 @@ ffi.NativeFunction< jni.JObjectPtr Function( ffi.Uint64, jni.JObjectPtr, jni.JObjectPtr)>> - _\$invokePointer = ffi.Pointer.fromFunction(_\$invoke); + _$invokePointer = ffi.Pointer.fromFunction(_$invoke); - static ffi.Pointer<ffi.Void> _\$invokeMethod( - int \$p, - \$MethodInvocation \$i, + static ffi.Pointer<ffi.Void> _$invokeMethod( + int $p, + $MethodInvocation $i, ) { - final \$d = \$i.methodDescriptor.toDartString(deleteOriginal: true); - final \$a = \$i.args; + final $d = $i.methodDescriptor.toDartString(deleteOriginal: true); + final $a = $i.args; '''); final proxyMethodIf = _InterfaceMethodIf(resolver, s); for (final method in node.methods) { @@ -482,18 +470,27 @@ final \$a = \$p.sendPort.nativePort; _\$impls[\$a] = \$impl; '''); - s.write(''' - _\$finalizer.attach(\$x, \$p, detach: \$x); - \$p.listen((\$m) { - final \$i = \$MethodInvocation.fromMessage(\$m); - final \$r = _\$invokeMethod(\$p.sendPort.nativePort, \$i); - ProtectedJniExtensions.returnResult(\$i.result, \$r); + s.write(r''' + $p.listen(($m) { + if ($m == null) { + _$impls.remove($p.sendPort.nativePort); + $p.close(); + return; + } + final $i = $MethodInvocation.fromMessage($m); + final $r = _$invokeMethod($p.sendPort.nativePort, $i); + ProtectedJniExtensions.returnResult($i.result, $r); }); - return \$x; + return $x; } '''); } + // Writing any custom code provided for this class. + if (config.customClassBody?.containsKey(node.binaryName) ?? false) { + s.writeln(config.customClassBody![node.binaryName]); + } + // End of Class definition. s.writeln('}');
diff --git a/pkgs/jnigen/lib/src/config/config_types.dart b/pkgs/jnigen/lib/src/config/config_types.dart index 8004cee..02dcbc2 100644 --- a/pkgs/jnigen/lib/src/config/config_types.dart +++ b/pkgs/jnigen/lib/src/config/config_types.dart
@@ -325,6 +325,7 @@ this.sourcePath, this.classPath, this.preamble, + this.customClassBody, this.androidSdkConfig, this.mavenDownloads, this.summarizerOptions, @@ -386,6 +387,12 @@ /// Call [importClasses] before using this. late final Map<String, ClassDecl> importedClasses; + /// Custom code that is added to the end of the class body with the specified + /// binary name. + /// + /// Used for testing package:jnigen. + final Map<String, String>? customClassBody; + Future<void> importClasses() async { importedClasses = {}; for (final import in [
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 06112a1..cd47f44 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
@@ -3344,22 +3344,8 @@ /// Maps a specific port to the implemented interface. static final Map<int, $MyInterfaceImpl> _$impls = {}; - ReceivePort? _$p; - static final Finalizer<ReceivePort> _$finalizer = Finalizer(($p) { - _$impls.remove($p.sendPort.nativePort); - $p.close(); - }); - - @override - void delete() { - _$impls.remove(_$p?.sendPort.nativePort); - _$p?.close(); - _$finalizer.detach(this); - super.delete(); - } - static jni.JObjectPtr _$invoke( int port, jni.JObjectPtr descriptor, @@ -3439,14 +3425,19 @@ ).._$p = $p; final $a = $p.sendPort.nativePort; _$impls[$a] = $impl; - _$finalizer.attach($x, $p, detach: $x); $p.listen(($m) { + if ($m == null) { + _$impls.remove($p.sendPort.nativePort); + $p.close(); + return; + } final $i = $MethodInvocation.fromMessage($m); final $r = _$invokeMethod($p.sendPort.nativePort, $i); ProtectedJniExtensions.returnResult($i.result, $r); }); return $x; } + static Map<int, $MyInterfaceImpl> get $impls => _$impls; } abstract class $MyInterfaceImpl<$T extends jni.JObject> {
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 73d8b74..9ca0857 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
@@ -3153,22 +3153,8 @@ /// Maps a specific port to the implemented interface. static final Map<int, $MyInterfaceImpl> _$impls = {}; - ReceivePort? _$p; - static final Finalizer<ReceivePort> _$finalizer = Finalizer(($p) { - _$impls.remove($p.sendPort.nativePort); - $p.close(); - }); - - @override - void delete() { - _$impls.remove(_$p?.sendPort.nativePort); - _$p?.close(); - _$finalizer.detach(this); - super.delete(); - } - static jni.JObjectPtr _$invoke( int port, jni.JObjectPtr descriptor, @@ -3248,14 +3234,19 @@ ).._$p = $p; final $a = $p.sendPort.nativePort; _$impls[$a] = $impl; - _$finalizer.attach($x, $p, detach: $x); $p.listen(($m) { + if ($m == null) { + _$impls.remove($p.sendPort.nativePort); + $p.close(); + return; + } final $i = $MethodInvocation.fromMessage($m); final $r = _$invokeMethod($p.sendPort.nativePort, $i); ProtectedJniExtensions.returnResult($i.result, $r); }); return $x; } + static Map<int, $MyInterfaceImpl> get $impls => _$impls; } abstract class $MyInterfaceImpl<$T extends jni.JObject> {
diff --git a/pkgs/jnigen/test/simple_package_test/generate.dart b/pkgs/jnigen/test/simple_package_test/generate.dart index ce38009..446cfb2 100644 --- a/pkgs/jnigen/test/simple_package_test/generate.dart +++ b/pkgs/jnigen/test/simple_package_test/generate.dart
@@ -64,6 +64,11 @@ 'com.github.dart_lang.jnigen.annotations', ], logLevel: Level.INFO, + customClassBody: { + 'com.github.dart_lang.jnigen.interfaces.MyInterface': r''' + static Map<int, $MyInterfaceImpl> get $impls => _$impls; +''' + }, outputConfig: OutputConfig( bindingsType: bindingsType, cConfig: CCodeOutputConfig(
diff --git a/pkgs/jnigen/test/simple_package_test/runtime_test_registrant.dart b/pkgs/jnigen/test/simple_package_test/runtime_test_registrant.dart index 3e7d657..665a26d 100644 --- a/pkgs/jnigen/test/simple_package_test/runtime_test_registrant.dart +++ b/pkgs/jnigen/test/simple_package_test/runtime_test_registrant.dart
@@ -16,6 +16,11 @@ const fpDelta = 0.001; const trillion = 1024 * 1024 * 1024 * 1024; +void _runJavaGC() { + final system = Jni.findJClass('java/lang/System'); + system.callStaticMethodByName<void>('gc', '()V', []); +} + void registerTests(String groupName, TestRunnerCallback test) { group(groupName, () { test('static final fields - int', () { @@ -531,11 +536,11 @@ }); group('interface implementation', () { - for (final method in { - 'another thread': MyInterfaceConsumer.consumeOnAnotherThread, - 'the same thread': MyInterfaceConsumer.consumeOnSameThread, - }.entries) { - test('MyInterface.implement on ${method.key}', () async { + for (final (threading, consume) in [ + ('another thread', MyInterfaceConsumer.consumeOnAnotherThread), + ('the same thread', MyInterfaceConsumer.consumeOnSameThread), + ]) { + test('MyInterface.implement on $threading', () async { final voidCallbackResult = Completer<JString>(); final varCallbackResult = Completer<JInteger>(); final manyPrimitivesResult = Completer<int>(); @@ -574,7 +579,7 @@ // [voidCallback]. // The other two methods will be called individually using the passed // arguments afterwards. - method.value( + consume( myInterface, // For stringCallback: 'hello'.toJString(), @@ -595,7 +600,20 @@ final manyPrimitives = await manyPrimitivesResult.future; expect(manyPrimitives, -1 + 3 + 3.14.toInt() + 1); + // Currently we have one implementation of the interface. + expect(MyInterface.$impls, hasLength(1)); myInterface.delete(); + // Running System.gc() and waiting. + _runJavaGC(); + for (var i = 0; i < 8; ++i) { + await Future<void>.delayed(Duration(milliseconds: (1 << i) * 100)); + if (MyInterface.$impls.isEmpty) { + break; + } + } + // Since the interface is now deleted, the cleaner must signal to Dart + // to clean up. + expect(MyInterface.$impls, isEmpty); }); } }); @@ -603,7 +621,7 @@ group('$groupName (load tests)', () { const k4 = 4 * 1024; // This is a round number, unlike say 4000 const k256 = 256 * 1024; - test('create large number of JNI references without deleting', () { + test('Create large number of JNI references without deleting', () { for (int i = 0; i < k4; i++) { final e = Example.new1(i); expect(e.getNumber(), equals(i));