From 292f89662917f5387d920a93f5b05d84175f3efb Mon Sep 17 00:00:00 2001 From: Hossein Yousefi Date: Tue, 19 Sep 2023 15:59:22 +0200 Subject: [PATCH] Convert incorrect exceptions into errors (#397) Closes #394. --- jni/CHANGELOG.md | 17 ++++ jni/lib/jni.dart | 8 +- jni/lib/src/accessors.dart | 2 +- jni/lib/src/errors.dart | 144 ++++++++++++++++++++++++++++++++ jni/lib/src/jexceptions.dart | 123 --------------------------- jni/lib/src/jni.dart | 20 ++--- jni/lib/src/jobject.dart | 8 +- jni/lib/src/jreference.dart | 24 +++--- jni/pubspec.yaml | 2 +- jni/test/exception_test.dart | 20 ++--- jni/test/jbyte_buffer_test.dart | 2 +- jni/test/jobject_test.dart | 50 ++++++++--- 12 files changed, 244 insertions(+), 176 deletions(-) create mode 100644 jni/lib/src/errors.dart delete mode 100644 jni/lib/src/jexceptions.dart diff --git a/jni/CHANGELOG.md b/jni/CHANGELOG.md index 64ddb4d7..15e7e9ec 100644 --- a/jni/CHANGELOG.md +++ b/jni/CHANGELOG.md @@ -1,3 +1,20 @@ +## 0.8.0-wip + +- **Breaking Change** ([#394](https://github.com/dart-lang/jnigen/issues/394)): + Converted various `Exception`s into `Error`s: + - `UseAfterReleaseException` -> `UseAfterReleaseError` + - `DoubleReleaseException` -> `DoubleReleaseError` + - `SpawnException` -> `JniError` (It's now a `sealed class`) + - `JNullException` -> `JNullError` + - `InvalidCallTypeException` -> `InvalidCallTypeError` + - `HelperNotFoundException` -> `HelperNotFoundError` + - `JvmExistsException` -> `JniVmExistsError` + - `NoJvmInstanceException` -> `NoJvmInstanceError` +- **Breaking Change**: Removed `InvalidJStringException`. +- **Breaking Change**: The default return `callType` of type parameter `int` for + methods such as `JObject.callMethodByName` is now Java's `long` instead + of `int` to be consistent with the way arguments work. + ## 0.7.0 - **Breaking Change** ([#387](https://github.com/dart-lang/jnigen/issues/387)): diff --git a/jni/lib/jni.dart b/jni/lib/jni.dart index 85f99fa3..ce372fff 100644 --- a/jni/lib/jni.dart +++ b/jni/lib/jni.dart @@ -60,19 +60,21 @@ /// This library provides classes and functions for JNI interop from Dart. library jni; -export 'src/third_party/generated_bindings.dart' - hide JniBindings, JniEnv, JniEnv1, JniExceptionDetails; +export 'src/errors.dart'; export 'src/jni.dart' hide ProtectedJniExtensions; export 'src/jvalues.dart' hide JValueArgs, toJValues; export 'src/types.dart'; export 'src/jarray.dart'; -export 'src/jexceptions.dart'; export 'src/jobject.dart'; export 'src/jprimitives.dart'; export 'src/jreference.dart' show JReferenceUseExtension; + export 'src/lang/lang.dart'; export 'src/nio/nio.dart'; export 'src/util/util.dart'; +export 'src/third_party/generated_bindings.dart' + hide JniBindings, JniEnv, JniEnv1, JniExceptionDetails; + export 'package:ffi/ffi.dart' show using, Arena; export 'dart:ffi' show nullptr; diff --git a/jni/lib/src/accessors.dart b/jni/lib/src/accessors.dart index b5e843c2..e505b947 100644 --- a/jni/lib/src/accessors.dart +++ b/jni/lib/src/accessors.dart @@ -7,7 +7,7 @@ import 'package:ffi/ffi.dart' show using; import 'package:jni/src/jvalues.dart'; -import 'jexceptions.dart'; +import 'errors.dart'; import 'third_party/generated_bindings.dart'; import 'jni.dart'; diff --git a/jni/lib/src/errors.dart b/jni/lib/src/errors.dart new file mode 100644 index 00000000..848d93ea --- /dev/null +++ b/jni/lib/src/errors.dart @@ -0,0 +1,144 @@ +// 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. + +import 'package:jni/src/third_party/generated_bindings.dart'; + +// TODO(#393): Add the fact that [JException] is now a [JObject] to the +// CHANGELOG. + +final class UseAfterReleaseError extends Error { + @override + String toString() { + return 'Use after release error'; + } +} + +// TODO(#393): Use NullPointerError once it's available. +final class JNullError extends Error { + @override + String toString() => 'The reference was null'; +} + +final class DoubleReleaseError extends Error { + @override + String toString() { + return 'Double release error'; + } +} + +/// Represents JNI errors that might be returned by methods like +/// `JNI_CreateJavaVM`. +sealed class JniError extends Error { + static const _errors = { + JniErrorCode.JNI_ERR: JniGenericError.new, + JniErrorCode.JNI_EDETACHED: JniThreadDetachedError.new, + JniErrorCode.JNI_EVERSION: JniVersionError.new, + JniErrorCode.JNI_ENOMEM: JniOutOfMemoryError.new, + JniErrorCode.JNI_EEXIST: JniVmExistsError.new, + JniErrorCode.JNI_EINVAL: JniArgumentError.new, + }; + + final String message; + + JniError(this.message); + + factory JniError.of(int status) { + if (!_errors.containsKey(status)) { + status = JniErrorCode.JNI_ERR; + } + return _errors[status]!(); + } + + @override + String toString() { + return 'JniError: $message'; + } +} + +final class JniGenericError extends JniError { + JniGenericError() : super('Generic JNI error'); +} + +final class JniThreadDetachedError extends JniError { + JniThreadDetachedError() : super('Thread detached from VM'); +} + +final class JniVersionError extends JniError { + JniVersionError() : super('JNI version error'); +} + +final class JniOutOfMemoryError extends JniError { + JniOutOfMemoryError() : super('Out of memory'); +} + +final class JniVmExistsError extends JniError { + JniVmExistsError() : super('VM Already created'); +} + +final class JniArgumentError extends JniError { + JniArgumentError() : super('Invalid arguments'); +} + +final class NoJvmInstanceError extends Error { + @override + String toString() => 'No JNI instance is available'; +} + +// TODO(#395): Remove this when calltypes are removed. +extension on int { + static const _names = { + JniCallType.booleanType: 'bool', + JniCallType.byteType: 'byte', + JniCallType.shortType: 'short', + JniCallType.charType: 'char', + JniCallType.intType: 'int', + JniCallType.longType: 'long', + JniCallType.floatType: 'float', + JniCallType.doubleType: 'double', + JniCallType.objectType: 'object', + JniCallType.voidType: 'void', + }; + String str() => _names[this]!; +} + +// TODO(#395): Remove this when `JniCallType`s are removed. +final class InvalidCallTypeError extends Error { + final int type; + final Set allowed; + + InvalidCallTypeError(this.type, this.allowed); + + @override + String toString() => 'Invalid type for call ${type.str()}. ' + 'Allowed types are ${allowed.map((t) => t.str()).toSet()}'; +} + +// TODO(#393): Remove this class in favor of `JThrowable`. +class JniException implements Exception { + /// Error message from Java exception. + final String message; + + /// Stack trace from Java. + final String stackTrace; + + JniException(this.message, this.stackTrace); + + @override + String toString() => 'Exception in Java code called through JNI: ' + '$message\n\n$stackTrace\n'; +} + +final class HelperNotFoundError extends Error { + final String path; + + HelperNotFoundError(this.path); + + @override + String toString() => ''' +Lookup for helper library $path failed. +Please ensure that `dartjni` shared library is built. +Provided jni:setup script can be used to build the shared library. +If the library is already built, ensure that the JVM libraries can be +loaded from Dart.'''; +} diff --git a/jni/lib/src/jexceptions.dart b/jni/lib/src/jexceptions.dart deleted file mode 100644 index 5a0d59e9..00000000 --- a/jni/lib/src/jexceptions.dart +++ /dev/null @@ -1,123 +0,0 @@ -// Copyright (c) 2022, 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. - -import 'dart:ffi'; - -import 'package:jni/src/third_party/generated_bindings.dart'; - -abstract class JException implements Exception {} - -class UseAfterReleaseException implements JException { - final Pointer ptr; - UseAfterReleaseException(this.ptr); - - @override - String toString() { - return 'Use after release on $ptr.'; - } -} - -class JNullException implements JException { - const JNullException(); - - @override - String toString() => 'The reference was null.'; -} - -class InvalidJStringException implements JException { - final Pointer reference; - InvalidJStringException(this.reference); - - @override - String toString() => 'Not a valid Java String: ' - '0x${reference.address.toRadixString(16)}.'; -} - -class DoubleReleaseException implements JException { - final Pointer ptr; - DoubleReleaseException(this.ptr); - - @override - String toString() { - return 'Double release on $ptr.'; - } -} - -class JvmExistsException implements JException { - @override - String toString() => 'A JVM is already spawned'; -} - -/// Represents spawn errors that might be returned by JNI_CreateJavaVM -class SpawnException implements JException { - static const _errors = { - JniErrorCode.JNI_ERR: 'Generic JNI error', - JniErrorCode.JNI_EDETACHED: 'Thread detached from VM', - JniErrorCode.JNI_EVERSION: 'JNI version error', - JniErrorCode.JNI_ENOMEM: 'Out of memory', - JniErrorCode.JNI_EEXIST: 'VM Already created', - JniErrorCode.JNI_EINVAL: 'Invalid arguments', - }; - int status; - SpawnException.of(this.status); - - @override - String toString() => _errors[status] ?? 'Unknown status code: $status'; -} - -class NoJvmInstanceException implements JException { - @override - String toString() => 'No JNI instance is available'; -} - -extension JniTypeNames on int { - static const _names = { - JniCallType.booleanType: 'bool', - JniCallType.byteType: 'byte', - JniCallType.shortType: 'short', - JniCallType.charType: 'char', - JniCallType.intType: 'int', - JniCallType.longType: 'long', - JniCallType.floatType: 'float', - JniCallType.doubleType: 'double', - JniCallType.objectType: 'object', - JniCallType.voidType: 'void', - }; - String str() => _names[this]!; -} - -class InvalidCallTypeException implements JException { - int type; - Set allowed; - InvalidCallTypeException(this.type, this.allowed); - @override - String toString() => 'Invalid type for call ${type.str()}. ' - 'Allowed types are ${allowed.map((t) => t.str()).toSet()}'; -} - -class JniException implements JException { - /// Error message from Java exception. - final String message; - - /// Stack trace from Java. - final String stackTrace; - JniException(this.message, this.stackTrace); - - @override - String toString() => 'Exception in Java code called through JNI: ' - '$message\n\n$stackTrace\n'; -} - -class HelperNotFoundException implements JException { - HelperNotFoundException(this.path); - final String path; - - @override - String toString() => ''' -Lookup for helper library $path failed. -Please ensure that `dartjni` shared library is built. -Provided jni:setup script can be used to build the shared library. -If the library is already built, ensure that the JVM libraries can be -loaded from Dart.'''; -} diff --git a/jni/lib/src/jni.dart b/jni/lib/src/jni.dart index 1e25151f..0bc154b9 100644 --- a/jni/lib/src/jni.dart +++ b/jni/lib/src/jni.dart @@ -9,7 +9,7 @@ import 'dart:isolate'; import 'package:ffi/ffi.dart'; import 'package:path/path.dart'; -import 'jexceptions.dart'; +import 'errors.dart'; import 'jobject.dart'; import 'third_party/generated_bindings.dart'; import 'jvalues.dart'; @@ -38,7 +38,7 @@ DynamicLibrary _loadDartJniLibrary({String? dir, String baseName = "dartjni"}) { final dylib = DynamicLibrary.open(libPath); return dylib; } on Error { - throw HelperNotFoundException(libPath); + throw HelperNotFoundError(libPath); } } @@ -97,12 +97,12 @@ abstract class Jni { jniVersion: jniVersion, ); if (status == false) { - throw JvmExistsException(); + throw JniVmExistsError(); } } /// Same as [spawn] but if a JVM exists, returns silently instead of - /// throwing [JvmExistsException]. + /// throwing [JvmExistsError]. /// /// If the options are different than that of existing VM, the existing VM's /// options will remain in effect. @@ -129,7 +129,7 @@ abstract class Jni { } else if (status == DART_JNI_SINGLETON_EXISTS) { return false; } else { - throw SpawnException.of(status); + throw JniError.of(status); } }); @@ -176,12 +176,12 @@ abstract class Jni { return _bindings.GetJavaVM(); } - /// Returns the instance of [GlobalJniEnvStruct], which is an abstraction over JNIEnv - /// without the same-thread restriction. + /// Returns the instance of [GlobalJniEnvStruct], which is an abstraction over + /// JNIEnv without the same-thread restriction. static Pointer _fetchGlobalEnv() { final env = _bindings.GetGlobalEnv(); if (env == nullptr) { - throw NoJvmInstanceException(); + throw NoJvmInstanceError(); } return env; } @@ -336,11 +336,11 @@ extension AdditionalEnvMethods on GlobalJniEnv { /// DeleteGlobalRef. String toDartString(JStringPtr jstringPtr, {bool releaseOriginal = false}) { if (jstringPtr == nullptr) { - throw const JNullException(); + throw JNullError(); } final chars = GetStringChars(jstringPtr, nullptr); if (chars == nullptr) { - throw InvalidJStringException(jstringPtr); + throw ArgumentError('Not a valid jstring pointer.'); } final result = chars.cast().toDartString(); ReleaseStringChars(jstringPtr, chars); diff --git a/jni/lib/src/jobject.dart b/jni/lib/src/jobject.dart index a7e659ce..15c69727 100644 --- a/jni/lib/src/jobject.dart +++ b/jni/lib/src/jobject.dart @@ -7,7 +7,7 @@ import 'dart:ffi'; import 'package:ffi/ffi.dart'; import 'package:jni/src/accessors.dart'; -import 'jexceptions.dart'; +import 'errors.dart'; import 'jni.dart'; import 'jreference.dart'; import 'lang/jstring.dart'; @@ -65,7 +65,7 @@ Pointer _getID( int _getCallType(int? callType, int defaultType, Set allowed) { if (callType == null) return defaultType; if (allowed.contains(callType)) return callType; - throw InvalidCallTypeException(callType, allowed); + throw InvalidCallTypeError(callType, allowed); } T _callOrGet(int? callType, JniResult Function(int) function) { @@ -78,7 +78,7 @@ T _callOrGet(int? callType, JniResult Function(int) function) { result = function(finalCallType).boolean as T; break; case int: - finalCallType = _getCallType(callType, JniCallType.intType, { + finalCallType = _getCallType(callType, JniCallType.longType, { JniCallType.byteType, JniCallType.charType, JniCallType.shortType, @@ -177,8 +177,6 @@ class JObject extends JReference { return _jClass ??= getClass(); } - /// Deletes the JNI reference and marks this object as released. Any further - /// uses will throw [UseAfterReleaseException]. @override void release() { _jClass?.release(); diff --git a/jni/lib/src/jreference.dart b/jni/lib/src/jreference.dart index 68c0adaf..8d8b71dc 100644 --- a/jni/lib/src/jreference.dart +++ b/jni/lib/src/jreference.dart @@ -7,13 +7,13 @@ import 'dart:ffi'; import 'package:ffi/ffi.dart'; import 'package:jni/src/third_party/generated_bindings.dart'; -import 'jexceptions.dart'; +import 'errors.dart'; import 'jni.dart'; extension ProtectedJReference on JReference { void setAsReleased() { if (_released) { - throw DoubleReleaseException(_reference); + throw DoubleReleaseError(); } _released = true; JReference._finalizer.detach(this); @@ -21,7 +21,7 @@ extension ProtectedJReference on JReference { void ensureNotNull() { if (isNull) { - throw const JNullException(); + throw JNullError(); } } @@ -47,15 +47,17 @@ abstract class JReference implements Finalizable { bool _released = false; - /// Check whether the underlying JNI reference is `null`. + /// Whether the underlying JNI reference is `null` or not. bool get isNull => reference == nullptr; - /// Returns `true` if the underlying JNI reference is deleted. + /// Whether the underlying JNI reference is deleted or not. bool get isReleased => _released; - /// Deletes the underlying JNI reference. + /// Deletes the underlying JNI reference and marks this as released. /// - /// Further uses will throw [UseAfterReleaseException]. + /// Throws [DoubleReleaseError] if this is already released. + /// + /// Further uses of this object will throw [UseAfterReleaseError]. void release() { setAsReleased(); Jni.env.DeleteGlobalRef(_reference); @@ -63,12 +65,12 @@ abstract class JReference implements Finalizable { /// The underlying JNI global object reference. /// - /// Throws [UseAfterReleaseException] if the object is previously released. + /// Throws [UseAfterReleaseError] if the object is previously released. /// /// Be careful when storing this in a variable since it might have gotten /// released upon use. JObjectPtr get reference { - if (_released) throw UseAfterReleaseException(_reference); + if (_released) throw UseAfterReleaseError(); return _reference; } @@ -84,11 +86,9 @@ extension JReferenceUseExtension on T { R use(R Function(T) callback) { try { final result = callback(this); - release(); return result; - } catch (e) { + } finally { release(); - rethrow; } } } diff --git a/jni/pubspec.yaml b/jni/pubspec.yaml index 46bedce2..1d99a401 100644 --- a/jni/pubspec.yaml +++ b/jni/pubspec.yaml @@ -4,7 +4,7 @@ name: jni description: A library to access JNI from Dart and Flutter that acts as a support library for package:jnigen. -version: 0.7.0 +version: 0.8.0-wip repository: https://github.com/dart-lang/jnigen/tree/main/jni topics: diff --git a/jni/test/exception_test.dart b/jni/test/exception_test.dart index d3c806dc..42d22976 100644 --- a/jni/test/exception_test.dart +++ b/jni/test/exception_test.dart @@ -14,17 +14,17 @@ void main() { checkDylibIsUpToDate(); bool caught = false; try { - // If library does not exist, a helpful exception should be thrown. - // we can't test this directly because - // `test` schedules functions asynchronously + // If library does not exist, a helpful error should be thrown. + // we can't test this directly because `test` schedules functions + // asynchronously. Jni.spawn(dylibDir: "wrong_dir"); - } on HelperNotFoundException catch (_) { + } on HelperNotFoundError catch (_) { // stderr.write("\n$_\n"); Jni.spawnIfNotExists( dylibDir: "build/jni_libs", jvmOptions: ["-Xmx128m"]); caught = true; - } on JvmExistsException { - stderr.writeln('cannot verify: HelperNotFoundException thrown'); + } on JniVmExistsError { + stderr.writeln('cannot verify: HelperNotFoundError thrown'); } if (!caught) { throw "Expected HelperNotFoundException\n" @@ -38,14 +38,14 @@ void run({required TestRunnerCallback testRunner}) { testRunner("double free throws exception", () { final r = Jni.newInstance("java/util/Random", "()V", []); r.release(); - expect(r.release, throwsA(isA())); + expect(r.release, throwsA(isA())); }); testRunner("Use after free throws exception", () { final r = Jni.newInstance("java/util/Random", "()V", []); r.release(); expect(() => r.callMethodByName("nextInt", "(I)I", [JValueInt(256)]), - throwsA(isA())); + throwsA(isA())); }); testRunner("void fieldType throws exception", () { @@ -56,12 +56,12 @@ void run({required TestRunnerCallback testRunner}) { throwsArgumentError); }); - testRunner("Wrong callType throws exception", () { + testRunner("Wrong callType throws error", () { final r = Jni.newInstance("java/util/Random", "()V", []); expect( () => r.callMethodByName( "nextInt", "(I)I", [JValueInt(256)], JniCallType.doubleType), - throwsA(isA())); + throwsA(isA())); }); testRunner("An exception in JNI throws JniException in Dart", () { diff --git a/jni/test/jbyte_buffer_test.dart b/jni/test/jbyte_buffer_test.dart index f68c47b0..48f9eaf4 100644 --- a/jni/test/jbyte_buffer_test.dart +++ b/jni/test/jbyte_buffer_test.dart @@ -221,7 +221,7 @@ void run({required TestRunnerCallback testRunner}) { final data2 = directBuffer.asUint8List(releaseOriginal: true); expect( () => directBuffer.nextByte = 42, - throwsA(isA()), + throwsA(isA()), ); expect(data2[0], 42); }); diff --git a/jni/test/jobject_test.dart b/jni/test/jobject_test.dart index 883913cd..a63828cf 100644 --- a/jni/test/jobject_test.dart +++ b/jni/test/jobject_test.dart @@ -45,7 +45,12 @@ void run({required TestRunnerCallback testRunner}) { // to JNI strings. final long = longClass.newInstance(longCtor, [176]); - final intValue = long.callMethodByName("intValue", "()I", []); + final intValue = long.callMethodByName( + "intValue", + "()I", + [], + JniCallType.intType, + ); expect(intValue, equals(176)); // Release any JObject and JClass instances using `.release()` after use. @@ -103,9 +108,17 @@ void run({required TestRunnerCallback testRunner}) { final nextIntMethod = random.getMethodID("nextInt", "(I)I"); for (int i = 0; i < 100; i++) { - int r = random.callMethod(nextIntMethod, [JValueInt(256 * 256)]); + int r = random.callMethod( + nextIntMethod, + [JValueInt(256 * 256)], + JniCallType.intType, + ); int bits = 0; - final jbc = longClass.callStaticMethod(bitCountMethod, [r]); + final jbc = longClass.callStaticMethod( + bitCountMethod, + [r], + JniCallType.intType, + ); while (r != 0) { bits += r % 2; r = (r / 2).floor(); @@ -118,8 +131,13 @@ void run({required TestRunnerCallback testRunner}) { // One-off invocation of static method in single call. testRunner("invoke_", () { - final m = Jni.invokeStaticMethod("java/lang/Short", "compare", "(SS)I", - [JValueShort(1234), JValueShort(1324)]); + final m = Jni.invokeStaticMethod( + "java/lang/Short", + "compare", + "(SS)I", + [JValueShort(1234), JValueShort(1324)], + JniCallType.intType, + ); expect(m, equals(1234 - 1324)); }); @@ -172,8 +190,13 @@ void run({required TestRunnerCallback testRunner}) { // You can use() method on JObject for using once and deleting. testRunner("use() method", () { final randomInt = Jni.newInstance("java/util/Random", "()V", []).use( - (random) => - random.callMethodByName("nextInt", "(I)I", [JValueInt(15)])); + (random) => random.callMethodByName( + "nextInt", + "(I)I", + [JValueInt(15)], + JniCallType.intType, + ), + ); expect(randomInt, lessThan(15)); }); @@ -197,7 +220,14 @@ void run({required TestRunnerCallback testRunner}) { // Don't forget to escape $ in nested type names final ordinal = Jni.retrieveStaticField( "java/net/Proxy\$Type", "HTTP", "Ljava/net/Proxy\$Type;") - .use((f) => f.callMethodByName("ordinal", "()I", [])); + .use( + (f) => f.callMethodByName( + "ordinal", + "()I", + [], + JniCallType.intType, + ), + ); expect(ordinal, equals(1)); }); @@ -209,8 +239,8 @@ void run({required TestRunnerCallback testRunner}) { expect(backToStr.toDartString(), str.toDartString()); final _ = backToStr.castTo(JObject.type, releaseOriginal: true) ..releasedBy(arena); - expect(backToStr.toDartString, throwsA(isA())); - expect(backToStr.release, throwsA(isA())); + expect(backToStr.toDartString, throwsA(isA())); + expect(backToStr.release, throwsA(isA())); }); });