From 9ec91f903d1e9dd1c0fac5fd550fa1c41c95dfa0 Mon Sep 17 00:00:00 2001 From: Uwe Trottmann Date: Tue, 12 Apr 2022 08:24:15 +0000 Subject: [PATCH 01/13] Add merge request template. --- .gitlab/merge_request_templates/Default.md | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .gitlab/merge_request_templates/Default.md diff --git a/.gitlab/merge_request_templates/Default.md b/.gitlab/merge_request_templates/Default.md new file mode 100644 index 000000000..57f092a39 --- /dev/null +++ b/.gitlab/merge_request_templates/Default.md @@ -0,0 +1,24 @@ +## What does this MR do? + + + +## Author's checklist + +- [ ] The MR fully addresses the requirements of the associated task. +- [ ] I did a self-review of the changes and did not spot any issues. Among others, this includes: + * I added unit tests for new/changed behavior; all test pass. + * My code conforms to our coding standards and guidelines. + * My changes are prepared in a way that makes the review straightforward for the reviewer. +- [ ] I amended [`CHANGELOG.md`](objectbox/CHANGELOG.md) if this affects users in any way. + +## Review checklist + +- [ ] I reviewed all changes line-by-line and addressed relevant issues +- [ ] The requirements of the associated task are fully met +- [ ] I can confirm that: + * CI passes + * Coverage percentages do not decrease + * New code conforms to standards and guidelines + * If applicable, additional checks were done for special code changes (e.g. core performance, binary size, OSS licenses) + +/assign me From 2a8b410413f733754cb0271a12d6fc87a6c73b6b Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Tue, 5 Apr 2022 14:39:20 +0200 Subject: [PATCH 02/13] bindings.dart: always try loading from 'lib' folder first. Also print message with potential solutions if loading fails. --- objectbox/CHANGELOG.md | 4 ++ .../lib/src/native/bindings/bindings.dart | 62 ++++++++++++++----- 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/objectbox/CHANGELOG.md b/objectbox/CHANGELOG.md index 7c0b9d1b7..89e53bea7 100644 --- a/objectbox/CHANGELOG.md +++ b/objectbox/CHANGELOG.md @@ -1,5 +1,9 @@ ## latest +* The native ObjectBox library is also searched for in the `lib` subfolder on desktop OS (macOS, + Linux, Windows). This is where the [`install.sh`](/install.sh) script downloads it by default. + E.g. it is no longer necessary to install the library globally to run `dart test` or `flutter test`. + ## 1.4.1 (2022-03-01) * Resolve "another store is still open" issue after Flutter hot restart (hot reload continues to work). #387 diff --git a/objectbox/lib/src/native/bindings/bindings.dart b/objectbox/lib/src/native/bindings/bindings.dart index b9e4694c8..1e03ed0c6 100644 --- a/objectbox/lib/src/native/bindings/bindings.dart +++ b/objectbox/lib/src/native/bindings/bindings.dart @@ -1,5 +1,7 @@ import 'dart:ffi'; -import 'dart:io' show Platform; +import 'dart:io' show Directory, Platform; + +import 'package:path/path.dart'; import 'helpers.dart'; import 'objectbox_c.dart'; @@ -31,29 +33,55 @@ ObjectBoxC? _tryObjectBoxLibProcess() { ObjectBoxC? _tryObjectBoxLibFile() { _lib = null; - var libName = 'objectbox'; + final String libName; if (Platform.isWindows) { - libName += '.dll'; - try { - _lib = DynamicLibrary.open(libName); - } on ArgumentError { - libName = 'lib/' + libName; - } + libName = 'objectbox.dll'; } else if (Platform.isMacOS) { - libName = 'lib' + libName + '.dylib'; - try { - _lib = DynamicLibrary.open(libName); - } on ArgumentError { - libName = '/usr/local/lib/' + libName; - } + libName = 'libobjectbox.dylib'; } else if (Platform.isAndroid) { - libName = 'lib' + libName + '-jni.so'; + libName = 'libobjectbox-jni.so'; } else if (Platform.isLinux) { - libName = 'lib' + libName + '.so'; + libName = 'libobjectbox.so'; } else { + // Other platforms not supported (for iOS see _tryObjectBoxLibProcess). return null; } - _lib ??= DynamicLibrary.open(libName); + // For desktop OS prefer version in 'lib' subfolder as this is where + // install.sh (which calls objectbox-c download.sh) puts the library. + if (Platform.isWindows || Platform.isMacOS || Platform.isLinux) { + // Must use absolute directory as relative directory fails on macOS + // due to security restrictions ("file system relative paths not allowed in + // hardened programs"). + String libPath = join(Directory.current.path, "lib", libName); + try { + _lib = DynamicLibrary.open(libPath); + } on ArgumentError { + // On macOS also try /usr/local/lib, this is where the objectbox-c + // download script installs to as well. + if (Platform.isMacOS) { + try { + _lib ??= DynamicLibrary.open('/usr/local/lib/' + libName); + } on ArgumentError { + // Ignore. + } + } + // Try default path, see below. + } + } + try { + // This will look in some standard places for shared libraries: + // - on Android in the JNI lib folder for the architecture + // - on Linux in /lib and /usr/lib + // - on macOS? + // - on Windows in the working directory and System32 + _lib ??= DynamicLibrary.open(libName); + } catch (e) { + print("Failed to load ObjectBox library. For Flutter apps, check if " + "objectbox_flutter_libs is added to dependencies. " + "For unit tests and Dart apps, check if the ObjectBox library was " + "downloaded (https://docs.objectbox.io/getting-started)."); + rethrow; + } return ObjectBoxC(_lib!); } From 68150e8b7d7840eb00b36aedc43160dd22577d67 Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Mon, 7 Mar 2022 14:56:20 +0100 Subject: [PATCH 03/13] Store run async: also close store on error. --- objectbox/lib/src/native/store.dart | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index bbf41b8a2..6c65c3da9 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -417,8 +417,13 @@ class Store { _IsoPass isoPass) async { final store = Store.attach(isoPass.model, isoPass.dbDirectoryPath, queriesCaseSensitiveDefault: isoPass.queriesCaseSensitiveDefault); - final result = await isoPass.runFn(store); - store.close(); + final R result; + try { + result = await isoPass.runFn(store); + } finally { + store.close(); + } + // Note: maybe replace with Isolate.exit (and remove kill call in // runIsolated) once min Dart SDK 2.15. isoPass.resultPort?.send(result); From fae86186c79dbf74847ee787499aaeef081747be Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Mon, 7 Mar 2022 14:57:33 +0100 Subject: [PATCH 04/13] Store run async: propagate errors, handle premature isolate exit. --- objectbox/lib/src/native/store.dart | 39 ++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index 6c65c3da9..3b4ed762d 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -440,20 +440,47 @@ class Store { Future runIsolated( TxMode mode, FutureOr Function(Store, P) callback, P param) async { final resultPort = ReceivePort(); + final exitPort = ReceivePort(); + final errorPort = ReceivePort(); // Await isolate spawn to avoid waiting forever if it fails to spawn. final isolate = await Isolate.spawn( _callFunctionWithStoreInIsolate, _IsoPass(_defs, directoryPath, _queriesCaseSensitiveDefault, - resultPort.sendPort, callback, param)); + resultPort.sendPort, callback, param), + errorsAreFatal: true, + onError: errorPort.sendPort, + onExit: exitPort.sendPort); // Use Completer to return result so type is not lost. - final result = Completer(); - resultPort.listen((dynamic message) { - result.complete(message as R); + final isolateResult = Completer(); + resultPort.listen((dynamic result) { + if (!isolateResult.isCompleted) { + isolateResult.complete(result as R); + } + }); + errorPort.listen((dynamic error) { + // See isolate.addErrorListener docs for message structure. + if (error is List) { + final Exception exception = Exception(error[0]); + final StackTrace stack = StackTrace.fromString(error[1] as String); + if (isolateResult.isCompleted) { + Zone.current.handleUncaughtError(exception, stack); + } else { + isolateResult.completeError(exception, stack); + } + } + }); + exitPort.listen((dynamic exit) { + if (!isolateResult.isCompleted) { + isolateResult.completeError( + Exception('Isolate exited without result or error.')); + } }); - await result.future; + await isolateResult.future; resultPort.close(); + exitPort.close(); + errorPort.close(); isolate.kill(); - return result.future; + return isolateResult.future; } /// Internal only - bypasses the main checks for async functions, you may From 1695a00fccb610577a0c32ca59bfe3a267618025 Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Mon, 7 Mar 2022 15:01:24 +0100 Subject: [PATCH 05/13] Store run async: rename, drop unused param, add code example. --- objectbox/CHANGELOG.md | 2 ++ objectbox/lib/src/native/store.dart | 29 +++++++++++++++++++++++++++-- objectbox/test/store_test.dart | 7 +++---- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/objectbox/CHANGELOG.md b/objectbox/CHANGELOG.md index 89e53bea7..142138106 100644 --- a/objectbox/CHANGELOG.md +++ b/objectbox/CHANGELOG.md @@ -1,4 +1,6 @@ ## latest +* Rename `Store.runIsolated` to `runAsync`, drop unused `mode` parameter, propagate errors and + handle premature isolate exit. * The native ObjectBox library is also searched for in the `lib` subfolder on desktop OS (macOS, Linux, Windows). This is where the [`install.sh`](/install.sh) script downloads it by default. diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index 3b4ed762d..992479c7b 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -432,13 +432,23 @@ class Store { /// Spawns an isolate, runs [callback] in that isolate passing it [param] with /// its own Store and returns the result of callback. /// + /// ```dart + /// String? readNameAndRemove(Store store, int objectId) { + /// var box = store.box(); + /// final nameOrNull = box.get(objectId)?.name; + /// box.remove(objectId); + /// return nameOrNull; + /// } + /// await store.runAsync(readNameAndRemove, objectId); + /// ``` + /// /// Instances of [callback] must be top-level functions or static methods /// of classes, not closures or instance methods of objects. /// /// Note: this requires Dart 2.15.0 or newer /// (shipped with Flutter 2.8.0 or newer). - Future runIsolated( - TxMode mode, FutureOr Function(Store, P) callback, P param) async { + Future runAsync( + FutureOr Function(Store, P) callback, P param) async { final resultPort = ReceivePort(); final exitPort = ReceivePort(); final errorPort = ReceivePort(); @@ -483,6 +493,21 @@ class Store { return isolateResult.future; } + /// Deprecated. Use [runAsync] instead. Will be removed in a future release. + /// + /// Spawns an isolate, runs [callback] in that isolate passing it [param] with + /// its own Store and returns the result of callback. + /// + /// Instances of [callback] must be top-level functions or static methods + /// of classes, not closures or instance methods of objects. + /// + /// Note: this requires Dart 2.15.0 or newer + /// (shipped with Flutter 2.8.0 or newer). + @Deprecated('Use `runAsync` instead. Will be removed in a future release.') + Future runIsolated(TxMode mode, + FutureOr Function(Store, P) callback, P param) async => + runAsync(callback, param); + /// Internal only - bypasses the main checks for async functions, you may /// only pass synchronous callbacks! R _runInTransaction(TxMode mode, R Function(Transaction) fn) { diff --git a/objectbox/test/store_test.dart b/objectbox/test/store_test.dart index 09ab2802e..dce08a286 100644 --- a/objectbox/test/store_test.dart +++ b/objectbox/test/store_test.dart @@ -191,11 +191,10 @@ void main() { Directory('store').deleteSync(recursive: true); }); - test('store_runInIsolatedTx', () async { + test('store run in isolate', () async { final env = TestEnv('store'); final id = env.box.put(TestEntity(tString: 'foo')); - final futureResult = - env.store.runIsolated(TxMode.write, readStringAndRemove, id); + final futureResult = env.store.runAsync(readStringAndRemove, id); print('Count in main isolate: ${env.box.count()}'); final String x; try { @@ -205,7 +204,7 @@ void main() { .firstMatch(Platform.version) ?.group(0); if (dartVersion != null && dartVersion.compareTo('2.15.0') < 0) { - print('runIsolated requires Dart 2.15, ignoring error.'); + print('API requires Dart 2.15, ignoring error.'); env.closeAndDelete(); return; } else { From d5ba111249141795bd199e377c43261cee64d842 Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Tue, 8 Mar 2022 15:44:27 +0100 Subject: [PATCH 06/13] Tests: add global notAtLeastDart2_15_0 function to skip tests. --- objectbox/test/store_test.dart | 18 ++---------------- objectbox/test/test_env.dart | 11 +++++++++++ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/objectbox/test/store_test.dart b/objectbox/test/store_test.dart index dce08a286..6aa5c748c 100644 --- a/objectbox/test/store_test.dart +++ b/objectbox/test/store_test.dart @@ -196,25 +196,11 @@ void main() { final id = env.box.put(TestEntity(tString: 'foo')); final futureResult = env.store.runAsync(readStringAndRemove, id); print('Count in main isolate: ${env.box.count()}'); - final String x; - try { - x = await futureResult; - } catch (e) { - final dartVersion = RegExp('([0-9]+).([0-9]+).([0-9]+)') - .firstMatch(Platform.version) - ?.group(0); - if (dartVersion != null && dartVersion.compareTo('2.15.0') < 0) { - print('API requires Dart 2.15, ignoring error.'); - env.closeAndDelete(); - return; - } else { - rethrow; - } - } + final String x = await futureResult; expect(x, 'foo!'); expect(env.box.count(), 0); // Must be removed once awaited env.closeAndDelete(); - }); + }, skip: notAtLeastDart2_15_0()); } Future readStringAndRemove(Store store, int id) async { diff --git a/objectbox/test/test_env.dart b/objectbox/test/test_env.dart index ebea8fc48..354614314 100644 --- a/objectbox/test/test_env.dart +++ b/objectbox/test/test_env.dart @@ -64,3 +64,14 @@ Matcher sameAsList(List list) => unorderedEquals(list); // We need to do this to receive an event in the stream before processing // the remainder of the test case. final yieldExecution = () async => await Future.delayed(Duration.zero); + +dynamic notAtLeastDart2_15_0() { + final dartVersion = RegExp('([0-9]+).([0-9]+).([0-9]+)') + .firstMatch(Platform.version) + ?.group(0); + if (dartVersion != null && dartVersion.compareTo('2.15.0') < 0) { + return 'Test requires Dart 2.15.0, skipping.'; + } else { + return false; + } +} From 7857ea8d06dcbd115e5d4dfa435ba6eb139dd95c Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Tue, 12 Apr 2022 12:33:31 +0200 Subject: [PATCH 07/13] Store run async: improve naming, documentation, IDE auto-complete. --- objectbox/lib/src/native/store.dart | 45 ++++++++++++++++++++--------- objectbox/test/store_test.dart | 4 +-- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index 992479c7b..049e15d19 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -412,14 +412,14 @@ class Store { return _runInTransaction(mode, (tx) => fn()); } - // Isolate entry point must be static or top-level. + // Isolate entry point must be able to be sent via SendPort.send. static Future _callFunctionWithStoreInIsolate( - _IsoPass isoPass) async { + _RunAsyncIsolateConfig isoPass) async { final store = Store.attach(isoPass.model, isoPass.dbDirectoryPath, queriesCaseSensitiveDefault: isoPass.queriesCaseSensitiveDefault); final R result; try { - result = await isoPass.runFn(store); + result = await isoPass.runCallback(store); } finally { store.close(); } @@ -432,6 +432,11 @@ class Store { /// Spawns an isolate, runs [callback] in that isolate passing it [param] with /// its own Store and returns the result of callback. /// + /// This is useful for ObjectBox operations that take longer than a few + /// milliseconds, e.g. putting many objects, which would cause frame drops. + /// + /// The following example gets the name of a User object, deletes the object + /// and returns the name: /// ```dart /// String? readNameAndRemove(Store store, int objectId) { /// var box = store.box(); @@ -442,21 +447,29 @@ class Store { /// await store.runAsync(readNameAndRemove, objectId); /// ``` /// - /// Instances of [callback] must be top-level functions or static methods - /// of classes, not closures or instance methods of objects. + /// The [callback] must be a function that can be sent to an isolate: either a + /// top-level function, static method or a closure that only captures objects + /// that can be sent to an isolate. + /// + /// The types `P` (type of the parameter to be passed to the callback) and + /// `R` (type of the result returned by the callback) must be able to be sent + /// to or received from an isolate. The same applies to errors originating + /// from the callback. + /// + /// See [SendPort.send] for a discussion on which values can be sent to and + /// received from isolates. /// /// Note: this requires Dart 2.15.0 or newer /// (shipped with Flutter 2.8.0 or newer). - Future runAsync( - FutureOr Function(Store, P) callback, P param) async { + Future runAsync(RunAsyncCallback callback, P param) async { final resultPort = ReceivePort(); final exitPort = ReceivePort(); final errorPort = ReceivePort(); // Await isolate spawn to avoid waiting forever if it fails to spawn. final isolate = await Isolate.spawn( _callFunctionWithStoreInIsolate, - _IsoPass(_defs, directoryPath, _queriesCaseSensitiveDefault, - resultPort.sendPort, callback, param), + _RunAsyncIsolateConfig(_defs, directoryPath, + _queriesCaseSensitiveDefault, resultPort.sendPort, callback, param), errorsAreFatal: true, onError: errorPort.sendPort, onExit: exitPort.sendPort); @@ -628,10 +641,16 @@ final _openStoreDirectories = HashSet(); final _nullSafetyEnabled = _nullReturningFn is! Future Function(); final _nullReturningFn = () => null; +// Define type so IDE generates named parameters. +/// Signature for the callback passed to [Store.runAsync]. +/// +/// Instances must be functions that can be sent to an isolate. +typedef RunAsyncCallback = FutureOr Function(Store store, P parameter); + /// Captures everything required to create a "copy" of a store in an isolate /// and run user code. @immutable -class _IsoPass { +class _RunAsyncIsolateConfig { final ModelDefinition model; /// Used to attach to store in separate isolate @@ -647,9 +666,9 @@ class _IsoPass { final P param; /// To be called in isolate. - final FutureOr Function(Store, P) callback; + final RunAsyncCallback callback; - const _IsoPass( + const _RunAsyncIsolateConfig( this.model, this.dbDirectoryPath, // ignore: avoid_positional_boolean_parameters @@ -660,5 +679,5 @@ class _IsoPass { /// Calls [callback] inside this class so types are not lost /// (if called in isolate types would be dynamic instead of P and R). - FutureOr runFn(Store store) => callback(store, param); + FutureOr runCallback(Store store) => callback(store, param); } diff --git a/objectbox/test/store_test.dart b/objectbox/test/store_test.dart index 6aa5c748c..6de955aa8 100644 --- a/objectbox/test/store_test.dart +++ b/objectbox/test/store_test.dart @@ -194,7 +194,7 @@ void main() { test('store run in isolate', () async { final env = TestEnv('store'); final id = env.box.put(TestEntity(tString: 'foo')); - final futureResult = env.store.runAsync(readStringAndRemove, id); + final futureResult = env.store.runAsync(_readStringAndRemove, id); print('Count in main isolate: ${env.box.count()}'); final String x = await futureResult; expect(x, 'foo!'); @@ -203,7 +203,7 @@ void main() { }, skip: notAtLeastDart2_15_0()); } -Future readStringAndRemove(Store store, int id) async { +Future _readStringAndRemove(Store store, int id) async { var box = store.box(); var testEntity = box.get(id); final result = testEntity!.tString! + '!'; From 40357245b20af4bb3ec33ebc63afc081782496cf Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Tue, 12 Apr 2022 13:25:56 +0200 Subject: [PATCH 08/13] Store run async: prevent messages racing each other. --- objectbox/lib/src/native/store.dart | 123 +++++++++++++++++----------- objectbox/test/store_test.dart | 40 +++++++++ 2 files changed, 116 insertions(+), 47 deletions(-) diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index 049e15d19..edc90e9d0 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -413,20 +413,31 @@ class Store { } // Isolate entry point must be able to be sent via SendPort.send. + // Must guarantee only a single result event is sent. + // runAsync only handles a single event, any sent afterwards are ignored. E.g. + // in case [Error] or [Exception] are thrown after the result is sent. static Future _callFunctionWithStoreInIsolate( _RunAsyncIsolateConfig isoPass) async { final store = Store.attach(isoPass.model, isoPass.dbDirectoryPath, queriesCaseSensitiveDefault: isoPass.queriesCaseSensitiveDefault); - final R result; + List result; try { - result = await isoPass.runCallback(store); + final callbackResult = await isoPass.runCallback(store); + // Use a one element list to signal a result was sent. + result = List.filled(1, callbackResult); + } catch (error, stack) { + // Use a three element list to signal an error was caught. + // A two element list is already sent by isolate onError handler. + result = List.filled(3, null) + ..[0] = error + ..[1] = stack; } finally { store.close(); } - // Note: maybe replace with Isolate.exit (and remove kill call in - // runIsolated) once min Dart SDK 2.15. - isoPass.resultPort?.send(result); + // Note: maybe replace with Isolate.exit (and remove kill() call in caller) + // once min Dart SDK 2.15. + isoPass.resultPort.send(result); } /// Spawns an isolate, runs [callback] in that isolate passing it [param] with @@ -462,48 +473,66 @@ class Store { /// Note: this requires Dart 2.15.0 or newer /// (shipped with Flutter 2.8.0 or newer). Future runAsync(RunAsyncCallback callback, P param) async { - final resultPort = ReceivePort(); - final exitPort = ReceivePort(); - final errorPort = ReceivePort(); - // Await isolate spawn to avoid waiting forever if it fails to spawn. - final isolate = await Isolate.spawn( - _callFunctionWithStoreInIsolate, - _RunAsyncIsolateConfig(_defs, directoryPath, - _queriesCaseSensitiveDefault, resultPort.sendPort, callback, param), - errorsAreFatal: true, - onError: errorPort.sendPort, - onExit: exitPort.sendPort); - // Use Completer to return result so type is not lost. - final isolateResult = Completer(); - resultPort.listen((dynamic result) { - if (!isolateResult.isCompleted) { - isolateResult.complete(result as R); - } - }); - errorPort.listen((dynamic error) { - // See isolate.addErrorListener docs for message structure. - if (error is List) { - final Exception exception = Exception(error[0]); - final StackTrace stack = StackTrace.fromString(error[1] as String); - if (isolateResult.isCompleted) { - Zone.current.handleUncaughtError(exception, stack); - } else { - isolateResult.completeError(exception, stack); - } - } - }); - exitPort.listen((dynamic exit) { - if (!isolateResult.isCompleted) { - isolateResult.completeError( - Exception('Isolate exited without result or error.')); - } - }); - await isolateResult.future; - resultPort.close(); - exitPort.close(); - errorPort.close(); + final port = RawReceivePort(); + final completer = Completer(); + + void _cleanup() { + port.close(); + } + + port.handler = (dynamic message) { + _cleanup(); + completer.complete(message); + }; + + final Isolate isolate; + try { + // Await isolate spawn to avoid waiting forever if it fails to spawn. + isolate = await Isolate.spawn( + _callFunctionWithStoreInIsolate, + _RunAsyncIsolateConfig(_defs, directoryPath, + _queriesCaseSensitiveDefault, port.sendPort, callback, param), + errorsAreFatal: true, + onError: port.sendPort, + onExit: port.sendPort); + } on Object { + _cleanup(); + rethrow; + } + + final dynamic response = await completer.future; + // Replace with Isolate.exit in _callFunctionWithStoreInIsolate + // once min SDK 2.15. isolate.kill(); - return isolateResult.future; + + if (response == null) { + throw RemoteError('Isolate exited without result or error.', ''); + } + + assert(response is List); + response as List; + + final respLength = response.length; + assert(1 <= respLength && respLength <= 3); + switch (respLength) { + case 1: + // Success, return result. + return response[0] as R; + case 2: + // See isolate.addErrorListener docs for message structure. + await Future.error(RemoteError( + response[0] as String, + response[1] as String, + )); + case 3: + default: + // Error thrown by callback. + assert(respLength == 3 && response[2] == null); + await Future.error( + response[0] as Object, + response[1] as StackTrace, + ); + } } /// Deprecated. Use [runAsync] instead. Will be removed in a future release. @@ -660,7 +689,7 @@ class _RunAsyncIsolateConfig { final bool queriesCaseSensitiveDefault; /// Non-void functions can use this port to receive the result. - final SendPort? resultPort; + final SendPort resultPort; /// Parameter passed to [callback]. final P param; diff --git a/objectbox/test/store_test.dart b/objectbox/test/store_test.dart index 6de955aa8..e7a0cb1a0 100644 --- a/objectbox/test/store_test.dart +++ b/objectbox/test/store_test.dart @@ -201,6 +201,29 @@ void main() { expect(env.box.count(), 0); // Must be removed once awaited env.closeAndDelete(); }, skip: notAtLeastDart2_15_0()); + + test('store runAsync returns isolate error', () async { + final env = TestEnv('store'); + try { + await env.store.runAsync(_producesIsolateError, 'nothing'); + fail("Should throw RemoteError"); + } on RemoteError { + // expected + } + env.closeAndDelete(); + }, skip: notAtLeastDart2_15_0()); + + test('store runAsync returns callback error', () async { + final env = TestEnv('store'); + try { + await env.store.runAsync(_producesCallbackError, 'nothing'); + fail("Should throw error produced by callback"); + } catch (e) { + expect(e, isA()); + expect(e, predicate((ArgumentError e) => e.message == 'Return me')); + } + env.closeAndDelete(); + }, skip: notAtLeastDart2_15_0()); } Future _readStringAndRemove(Store store, int id) async { @@ -215,6 +238,23 @@ Future _readStringAndRemove(Store store, int id) async { return await Future.delayed(const Duration(milliseconds: 10), () => result); } +// Produce an error within the isolate that triggers the onError handler case. +// Errors because ReceivePort can not be sent via SendPort. +int _producesIsolateError(Store store, String param) { + final port = ReceivePort(); + try { + throw port; + } finally { + port.close(); + } +} + +// Produce an error that is caught and sent, triggering the error thrown +// by callable case. +int _producesCallbackError(Store store, String param) { + throw ArgumentError('Return me'); +} + class StoreAttachIsolateInit { SendPort sendPort; String path; From 73743214486213b7a826b82c9d2acfe8ffc1ae6c Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Tue, 12 Apr 2022 14:17:17 +0200 Subject: [PATCH 09/13] Store run async: use message types instead of List. --- objectbox/lib/src/native/store.dart | 68 ++++++++++++++++------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index edc90e9d0..675d433a7 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -420,17 +420,12 @@ class Store { _RunAsyncIsolateConfig isoPass) async { final store = Store.attach(isoPass.model, isoPass.dbDirectoryPath, queriesCaseSensitiveDefault: isoPass.queriesCaseSensitiveDefault); - List result; + dynamic result; try { final callbackResult = await isoPass.runCallback(store); - // Use a one element list to signal a result was sent. - result = List.filled(1, callbackResult); + result = _RunAsyncResult(callbackResult); } catch (error, stack) { - // Use a three element list to signal an error was caught. - // A two element list is already sent by isolate onError handler. - result = List.filled(3, null) - ..[0] = error - ..[1] = stack; + result = _RunAsyncError(error, stack); } finally { store.close(); } @@ -509,29 +504,25 @@ class Store { throw RemoteError('Isolate exited without result or error.', ''); } - assert(response is List); - response as List; - - final respLength = response.length; - assert(1 <= respLength && respLength <= 3); - switch (respLength) { - case 1: - // Success, return result. - return response[0] as R; - case 2: - // See isolate.addErrorListener docs for message structure. - await Future.error(RemoteError( - response[0] as String, - response[1] as String, - )); - case 3: - default: - // Error thrown by callback. - assert(respLength == 3 && response[2] == null); - await Future.error( - response[0] as Object, - response[1] as StackTrace, - ); + if (response is _RunAsyncResult) { + // Success, return result. + return response.result as R; + } else if (response is List) { + // See isolate.addErrorListener docs for message structure. + assert(response.length == 2); + await Future.error(RemoteError( + response[0] as String, + response[1] as String, + )); + } else { + // Error thrown by callback. + assert(response is _RunAsyncError); + response as _RunAsyncError; + + await Future.error( + response.error, + response.stack, + ); } } @@ -710,3 +701,18 @@ class _RunAsyncIsolateConfig { /// (if called in isolate types would be dynamic instead of P and R). FutureOr runCallback(Store store) => callback(store, param); } + +@immutable +class _RunAsyncResult { + final R result; + + const _RunAsyncResult(this.result); +} + +@immutable +class _RunAsyncError { + final Object error; + final StackTrace stack; + + const _RunAsyncError(this.error, this.stack); +} From 2506ec0fd881de951027aaf4448253c8473374cd Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Mon, 7 Mar 2022 15:16:35 +0100 Subject: [PATCH 10/13] Add Store.runInTransactionAsync and tests. --- objectbox/CHANGELOG.md | 2 + objectbox/lib/src/native/store.dart | 31 ++++++++++++++ objectbox/test/box_test.dart | 66 +++++++++++++++++++++++++++++ objectbox/test/store_test.dart | 39 +++++++++++++++++ 4 files changed, 138 insertions(+) diff --git a/objectbox/CHANGELOG.md b/objectbox/CHANGELOG.md index 142138106..7f3565f66 100644 --- a/objectbox/CHANGELOG.md +++ b/objectbox/CHANGELOG.md @@ -1,4 +1,6 @@ ## latest +* Add `Store.runInTransactionAsync` to run database operations asynchronously in the background + (requires Flutter 2.8.0/Dart 2.15.0 or newer). * Rename `Store.runIsolated` to `runAsync`, drop unused `mode` parameter, propagate errors and handle premature isolate exit. diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index 675d433a7..f78602be4 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -412,6 +412,29 @@ class Store { return _runInTransaction(mode, (tx) => fn()); } + /// Like [runAsync], but executes [callback] within a read or write + /// transaction depending on [mode]. + /// + /// See the documentation on [runAsync] for important usage details. + /// + /// The following example gets the name of a User object, deletes the object + /// and returns the name within a write transaction: + /// ```dart + /// String? readNameAndRemove(Store store, int objectId) { + /// var box = store.box(); + /// final nameOrNull = box.get(objectId)?.name; + /// box.remove(objectId); + /// return nameOrNull; + /// } + /// await store.runInTransactionAsync(TxMode.write, readNameAndRemove, objectId); + /// ``` + Future runInTransactionAsync( + TxMode mode, TxAsyncCallback callback, P param) => + runAsync( + (Store store, P p) => + store.runInTransaction(mode, () => callback(store, p)), + param); + // Isolate entry point must be able to be sent via SendPort.send. // Must guarantee only a single result event is sent. // runAsync only handles a single event, any sent afterwards are ignored. E.g. @@ -440,6 +463,8 @@ class Store { /// /// This is useful for ObjectBox operations that take longer than a few /// milliseconds, e.g. putting many objects, which would cause frame drops. + /// If all operations can execute within a single transaction, prefer to use + /// [runInTransactionAsync]. /// /// The following example gets the name of a User object, deletes the object /// and returns the name: @@ -716,3 +741,9 @@ class _RunAsyncError { const _RunAsyncError(this.error, this.stack); } + +// Specify so IDE generates named parameters. +/// Signature for callback passed to [Store.runInTransactionAsync]. +/// +/// Instances must be functions that can be sent to an isolate. +typedef TxAsyncCallback = R Function(Store store, P parameter); diff --git a/objectbox/test/box_test.dart b/objectbox/test/box_test.dart index ed1297a8d..94b7012f3 100644 --- a/objectbox/test/box_test.dart +++ b/objectbox/test/box_test.dart @@ -553,6 +553,17 @@ void main() { expect(count, equals(6)); }); + test('simple write in txn works - async', () async { + int count; + void callback(Store store, List param) { + store.box().putMany(param); + } + + await store.runInTransactionAsync(TxMode.write, callback, simpleItems()); + count = box.count(); + expect(count, equals(6)); + }, skip: notAtLeastDart2_15_0()); + test('failing transactions', () { expect( () => store.runInTransaction(TxMode.write, () { @@ -568,6 +579,22 @@ void main() { expect(box.count(), equals(0)); }); + test('failing transactions - async', () async { + expect( + () async => await store.runInTransactionAsync(TxMode.write, + (Store store, List param) { + store.box().putMany(param); + // note: we're throwing conditionally (but always true) so that + // the return type is not [Never]. See [Transaction.execute()] + // testing for the return type to be a [Future]. [Never] is a + // base class to everything, so a [Future] is also a [Never]. + if (1 + 1 == 2) throw 'test-exception'; + return 1; + }, simpleItems()), + throwsA('test-exception')); + expect(box.count(), equals(0)); + }, skip: notAtLeastDart2_15_0()); + test('recursive write in write transaction', () { store.runInTransaction(TxMode.write, () { box.putMany(simpleItems()); @@ -578,6 +605,22 @@ void main() { expect(box.count(), equals(12)); }); + test('recursive write in write transaction - async', () async { + await store.runInTransactionAsync(TxMode.write, + (Store store, List param) { + final box = store.box(); + box.putMany(param); + store.runInTransaction(TxMode.write, () { + // Re-set IDs to re-insert. + for (var element in param) { + element.id = 0; + } + box.putMany(param); + }); + }, simpleItems()); + expect(box.count(), equals(12)); + }, skip: notAtLeastDart2_15_0()); + test('recursive read in write transaction', () { int count = store.runInTransaction(TxMode.write, () { box.putMany(simpleItems()); @@ -586,6 +629,16 @@ void main() { expect(count, equals(6)); }); + test('recursive read in write transaction - async', () async { + int count = await store.runInTransactionAsync(TxMode.write, + (Store store, List param) { + final box = store.box(); + box.putMany(param); + return store.runInTransaction(TxMode.read, box.count); + }, simpleItems()); + expect(count, equals(6)); + }, skip: notAtLeastDart2_15_0()); + test('recursive write in read -> fails during creation', () { expect( () => store.runInTransaction(TxMode.read, () { @@ -597,6 +650,19 @@ void main() { e.toString().contains('failed to create transaction')))); }); + test('recursive write in async read -> fails during creation', () { + expect( + () => store.runInTransactionAsync(TxMode.read, + (Store store, List param) { + final box = store.box(); + box.count(); + return store.runInTransaction( + TxMode.write, () => box.putMany(param)); + }, simpleItems()), + throwsA(predicate((StateError e) => e.toString().contains( + 'Bad state: failed to create transaction: 10001 Cannot start a write transaction inside a read only transaction')))); + }, skip: notAtLeastDart2_15_0()); + test('failing in recursive txn', () { store.runInTransaction(TxMode.write, () { //should throw code10001 -> valid until fix diff --git a/objectbox/test/store_test.dart b/objectbox/test/store_test.dart index e7a0cb1a0..1122e56a1 100644 --- a/objectbox/test/store_test.dart +++ b/objectbox/test/store_test.dart @@ -111,6 +111,45 @@ void main() { env.closeAndDelete(); }); + test('async transactions', () async { + final env = TestEnv('store'); + expect(TxMode.values.length, 2); + for (var mode in TxMode.values) { + // Returned value falls through. + expect( + await env.store + .runInTransactionAsync(mode, (store, param) => 1, null), + 1); + + // Async callbacks are forbidden. + final asyncCallbacks = [ + (Store s, Object? p) async => null, + (Store s, Object? p) => + Future.delayed(const Duration(milliseconds: 1)), + (Store s, Object? p) => Future.value(), + ]; + for (var callback in asyncCallbacks) { + try { + await env.store.runInTransactionAsync(mode, callback, null); + fail("Should throw UnsupportedError"); + } on UnsupportedError catch (e) { + expect(e.message, + 'Executing an "async" function in a transaction is not allowed.'); + } + } + + // Functions that [Never] finish won't be executed at all. + try { + await env.store.runInTransactionAsync(mode, (store, param) { + throw 'Should never execute'; + }, null); + } on UnsupportedError catch (e) { + expect(e.message, 'Given transaction callback always fails.'); + } + } + env.closeAndDelete(); + }, skip: notAtLeastDart2_15_0()); + test('store multi-open', () { final stores = []; From 517e1363b8b2c43f46d0ced87d937e375776335e Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Tue, 12 Apr 2022 15:15:26 +0200 Subject: [PATCH 11/13] Demo runInTransactionAsync. --- .../flutter/objectbox_demo/lib/main.dart | 4 ++-- .../flutter/objectbox_demo/lib/objectbox.dart | 21 ++++++++++++++++++- .../flutter/objectbox_demo_sync/lib/main.dart | 4 ++-- .../objectbox_demo_sync/lib/objectbox.dart | 16 ++++++++++++++ 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/objectbox/example/flutter/objectbox_demo/lib/main.dart b/objectbox/example/flutter/objectbox_demo/lib/main.dart index 4dd18dbea..b95ccb402 100644 --- a/objectbox/example/flutter/objectbox_demo/lib/main.dart +++ b/objectbox/example/flutter/objectbox_demo/lib/main.dart @@ -44,9 +44,9 @@ class _MyHomePageState extends State { final _noteInputController = TextEditingController(); final _listController = StreamController>(sync: true); - void _addNote() { + Future _addNote() async { if (_noteInputController.text.isEmpty) return; - objectbox.noteBox.put(Note(_noteInputController.text)); + await objectbox.addNote(_noteInputController.text); _noteInputController.text = ''; } diff --git a/objectbox/example/flutter/objectbox_demo/lib/objectbox.dart b/objectbox/example/flutter/objectbox_demo/lib/objectbox.dart index 0eb25a480..1959148dc 100644 --- a/objectbox/example/flutter/objectbox_demo/lib/objectbox.dart +++ b/objectbox/example/flutter/objectbox_demo/lib/objectbox.dart @@ -40,6 +40,25 @@ class ObjectBox { Note('Delete notes by tapping on one'), Note('Write a demo app for ObjectBox') ]; - noteBox.putMany(demoNotes); + store.runInTransactionAsync(TxMode.write, _putNotesInTx, demoNotes); + } + + static void _putNotesInTx(Store store, List notes) => + store.box().putMany(notes); + + /// Add a note within a transaction. + /// + /// To avoid frame drops, run ObjectBox operations that take longer than a + /// few milliseconds, e.g. putting many objects, in an isolate with its + /// own Store instance. + /// For this example only a single object is put which would also be fine if + /// done here directly. + Future addNote(String text) => + store.runInTransactionAsync(TxMode.write, _addNoteInTx, text); + + static void _addNoteInTx(Store store, String text) { + // Perform ObjectBox operations that take longer than a few milliseconds + // here. To keep it simple, this example just puts a single object. + store.box().put(Note(text)); } } diff --git a/objectbox/example/flutter/objectbox_demo_sync/lib/main.dart b/objectbox/example/flutter/objectbox_demo_sync/lib/main.dart index 894347a8c..03ec20564 100644 --- a/objectbox/example/flutter/objectbox_demo_sync/lib/main.dart +++ b/objectbox/example/flutter/objectbox_demo_sync/lib/main.dart @@ -44,9 +44,9 @@ class _MyHomePageState extends State { final _noteInputController = TextEditingController(); final _listController = StreamController>(sync: true); - void _addNote() { + Future _addNote() async { if (_noteInputController.text.isEmpty) return; - objectbox.noteBox.put(Note(_noteInputController.text)); + await objectbox.addNote(_noteInputController.text); _noteInputController.text = ''; } diff --git a/objectbox/example/flutter/objectbox_demo_sync/lib/objectbox.dart b/objectbox/example/flutter/objectbox_demo_sync/lib/objectbox.dart index 0a40fc9f7..3e07d6634 100644 --- a/objectbox/example/flutter/objectbox_demo_sync/lib/objectbox.dart +++ b/objectbox/example/flutter/objectbox_demo_sync/lib/objectbox.dart @@ -42,4 +42,20 @@ class ObjectBox { ); return ObjectBox._create(store); } + + /// Add a note within a transaction. + /// + /// To avoid frame drops, run ObjectBox operations that take longer than a + /// few milliseconds, e.g. putting many objects, in an isolate with its + /// own Store instance. + /// For this example only a single object is put which would also be fine if + /// done here directly. + Future addNote(String text) => + store.runInTransactionAsync(TxMode.write, _addNoteInTx, text); + + static void _addNoteInTx(Store store, String text) { + // Perform ObjectBox operations that take longer than a few milliseconds + // here. To keep it simple, this example just puts a single object. + store.box().put(Note(text)); + } } From 73812abe6fc13e36956cac3a210a720c87ea5897 Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Tue, 12 Apr 2022 15:26:00 +0200 Subject: [PATCH 12/13] Store run async: directly reference closure over-capture bug. --- objectbox/lib/src/native/store.dart | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index f78602be4..954e45109 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -482,6 +482,11 @@ class Store { /// top-level function, static method or a closure that only captures objects /// that can be sent to an isolate. /// + /// Warning: Due to + /// [dart-lang/sdk#36983](https://github.com/dart-lang/sdk/issues/36983) a + /// closure may capture more objects than expected, even if they are not + /// directly used in the closure itself. + /// /// The types `P` (type of the parameter to be passed to the callback) and /// `R` (type of the result returned by the callback) must be able to be sent /// to or received from an isolate. The same applies to errors originating From 70cc9dd80a20e7a84b121316ff54eda5dbad92ea Mon Sep 17 00:00:00 2001 From: Uwe <13865709+greenrobot-team@users.noreply.github.com> Date: Tue, 3 May 2022 14:09:12 +0200 Subject: [PATCH 13/13] Demo: note why closure is not used for run async (#19) --- objectbox/example/flutter/objectbox_demo/lib/objectbox.dart | 3 +++ .../example/flutter/objectbox_demo_sync/lib/objectbox.dart | 3 +++ 2 files changed, 6 insertions(+) diff --git a/objectbox/example/flutter/objectbox_demo/lib/objectbox.dart b/objectbox/example/flutter/objectbox_demo/lib/objectbox.dart index 1959148dc..053ee201a 100644 --- a/objectbox/example/flutter/objectbox_demo/lib/objectbox.dart +++ b/objectbox/example/flutter/objectbox_demo/lib/objectbox.dart @@ -56,6 +56,9 @@ class ObjectBox { Future addNote(String text) => store.runInTransactionAsync(TxMode.write, _addNoteInTx, text); + /// Note: due to [dart-lang/sdk#36983](https://github.com/dart-lang/sdk/issues/36983) + /// not using a closure as it may capture more objects than expected. + /// These might not be send-able to an isolate. See Store.runAsync for details. static void _addNoteInTx(Store store, String text) { // Perform ObjectBox operations that take longer than a few milliseconds // here. To keep it simple, this example just puts a single object. diff --git a/objectbox/example/flutter/objectbox_demo_sync/lib/objectbox.dart b/objectbox/example/flutter/objectbox_demo_sync/lib/objectbox.dart index 3e07d6634..545daa935 100644 --- a/objectbox/example/flutter/objectbox_demo_sync/lib/objectbox.dart +++ b/objectbox/example/flutter/objectbox_demo_sync/lib/objectbox.dart @@ -53,6 +53,9 @@ class ObjectBox { Future addNote(String text) => store.runInTransactionAsync(TxMode.write, _addNoteInTx, text); + /// Note: due to [dart-lang/sdk#36983](https://github.com/dart-lang/sdk/issues/36983) + /// not using a closure as it may capture more objects than expected. + /// These might not be send-able to an isolate. See Store.runAsync for details. static void _addNoteInTx(Store store, String text) { // Perform ObjectBox operations that take longer than a few milliseconds // here. To keep it simple, this example just puts a single object.