From 7e1e31970d0f8a9b6e576c3a4bad2a05fa463245 Mon Sep 17 00:00:00 2001 From: Uwe Trottmann <13865709+greenrobot-team@users.noreply.github.com> Date: Mon, 31 Jan 2022 14:03:58 +0100 Subject: [PATCH 1/2] Add Store.attach and Store.isOpen. --- objectbox/CHANGELOG.md | 3 + objectbox/lib/src/native/store.dart | 134 +++++++++++++++++++++------- objectbox/test/basics_test.dart | 82 +++++++++++++++++ objectbox/test/isolates_test.dart | 122 +++++++++++++++---------- 4 files changed, 261 insertions(+), 80 deletions(-) diff --git a/objectbox/CHANGELOG.md b/objectbox/CHANGELOG.md index 410025e2f..681913590 100644 --- a/objectbox/CHANGELOG.md +++ b/objectbox/CHANGELOG.md @@ -2,6 +2,9 @@ * Add an option to change code-generator's `output_dir` in `pubspec.yaml`. #341 * Support ObjectBox Admin for Android apps to browse the database, see our [docs](https://docs.objectbox.io/data-browser) to get started. #148 +* Add `Store.attach` to attach to a Store opened in a directory. This is an improved replacement for + `Store.fromReference` to share a Store across isolates. It is no longer required to pass a + Store reference and the underlying Store remains open until the last instance is closed. #376 * Update: [objectbox-c 0.15.2](https://github.com/objectbox/objectbox-c/releases/tag/v0.15.0). * Update: [objectbox-android 3.1.2](https://github.com/objectbox/objectbox-java/releases/tag/V3.1.0). * Update: [objectbox-swift 1.7.0](https://github.com/objectbox/objectbox-swift/releases/tag/v1.7.0). diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index 5bf42709a..1aa925048 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -27,6 +27,9 @@ part 'observable.dart'; /// Represents an ObjectBox database and works together with [Box] to allow /// getting and putting. class Store { + /// Path of the default directory, currently 'objectbox'. + static const String defaultDirectoryPath = 'objectbox'; + late final Pointer _cStore; HashMap? _entityTypeById; final _boxes = HashMap(); @@ -36,8 +39,8 @@ class Store { final _reader = ReaderWithCBuffer(); Transaction? _tx; - /// absolute path to the database directory - final String _dbDir; + /// Absolute path to the database directory, used for open check. + final String _absoluteDirectoryPath; late final ByteData _reference; @@ -51,8 +54,12 @@ class Store { /// Default value for string query conditions [caseSensitive] argument. final bool _queriesCaseSensitiveDefault; + static String _safeDirectoryPath(String? path) => + (path == null || path.isEmpty) ? defaultDirectoryPath : path; + /// Creates a BoxStore using the model definition from your - /// `objectbox.g.dart` file. + /// `objectbox.g.dart` file in the given [directory] path + /// (or if null the [defaultDirectoryPath]). /// /// For example in a Flutter app: /// ```dart @@ -76,10 +83,8 @@ class Store { String? macosApplicationGroup}) : _weak = false, _queriesCaseSensitiveDefault = queriesCaseSensitiveDefault, - _dbDir = path.context.canonicalize( - (directory == null || directory.isEmpty) - ? 'objectbox' - : directory) { + _absoluteDirectoryPath = + path.context.canonicalize(_safeDirectoryPath(directory)) { try { if (Platform.isMacOS && macosApplicationGroup != null) { if (!macosApplicationGroup.endsWith('/')) { @@ -96,12 +101,7 @@ class Store { malloc.free(cStr); } } - if (_openStoreDirectories.contains(_dbDir)) { - throw UnsupportedError( - 'Cannot create multiple Store instances for the same directory. ' - 'Please use a single Store or close() the previous instance before ' - 'opening another one.'); - } + _checkStoreDirectoryNotOpen(); final model = Model(_defs.model); final opt = C.opt(); @@ -132,23 +132,7 @@ class Store { } _cStore = C.store_open(opt); - try { - checkObxPtr(_cStore, 'failed to create store'); - } on ObjectBoxException catch (e) { - // Recognize common problems when trying to open/create a database - // 10199 = OBX_ERROR_STORAGE_GENERAL - // 13 = permissions denied, 30 = read-only filesystem - if (e.message.contains(OBX_ERROR_STORAGE_GENERAL.toString()) && - e.message.contains('Dir does not exist') && - (e.message.endsWith(' (13)') || e.message.endsWith(' (30)'))) { - throw ObjectBoxException(e.message + - ' - this usually indicates a problem with permissions; ' - "if you're using Flutter you may need to use " - 'getApplicationDocumentsDirectory() from the path_provider ' - 'package, see example/README.md'); - } - rethrow; - } + _checkStorePointer(_cStore); // Always create _reference, so it can be non-nullable. // Ensure we only try to access the store created in the same process. @@ -157,7 +141,7 @@ class Store { _reference.setUint64(0 * _int64Size, pid); _reference.setUint64(1 * _int64Size, _ptr.address); - _openStoreDirectories.add(_dbDir); + _openStoreDirectories.add(_absoluteDirectoryPath); } catch (e) { _reader.clear(); rethrow; @@ -205,7 +189,7 @@ class Store { {bool queriesCaseSensitiveDefault = true}) // must not close the same native store twice so [_weak]=true : _weak = true, - _dbDir = '', + _absoluteDirectoryPath = '', _queriesCaseSensitiveDefault = queriesCaseSensitiveDefault { // see [reference] for serialization order final readPid = _reference.getUint64(0 * _int64Size); @@ -221,6 +205,90 @@ class Store { } } + /// Attach to a store opened in the [directoryPath] + /// (or if null the [defaultDirectoryPath]). + /// + /// Use this to access an open store from other isolates. + /// This results in each isolate having access to the same underlying native + /// store. + /// + /// The returned store is a new instance (e.g. different pointer value) with + /// its own lifetime and must also be closed (e.g. before an isolate exits). + /// The actual underlying store is only closed when the last store instance + /// is closed (e.g. when the app exits). + Store.attach(this._defs, String? directoryPath, + {bool queriesCaseSensitiveDefault = true}) + // _weak = false so store can be closed. + : _weak = false, + _queriesCaseSensitiveDefault = queriesCaseSensitiveDefault, + _absoluteDirectoryPath = + path.context.canonicalize(_safeDirectoryPath(directoryPath)) { + try { + // Do not allow attaching to a store that is already open in the current + // isolate. While technically possible this is not the intended usage + // and e.g. transactions would have to be carefully managed to not + // overlap. + _checkStoreDirectoryNotOpen(); + + final path = _safeDirectoryPath(directoryPath); + final pathCStr = path.toNativeUtf8(); + try { + _cStore = C.store_attach(pathCStr.cast()); + } finally { + malloc.free(pathCStr); + } + + _checkStorePointer(_cStore); + + // Not setting _reference as this is a replacement for obtaining a store + // via reference. + } catch (e) { + _reader.clear(); + rethrow; + } + } + + void _checkStoreDirectoryNotOpen() { + if (_openStoreDirectories.contains(_absoluteDirectoryPath)) { + throw UnsupportedError( + 'Cannot create multiple Store instances for the same directory in the same isolate. ' + 'Please use a single Store, close() the previous instance before ' + 'opening another one or attach to it in another isolate.'); + } + } + + void _checkStorePointer(Pointer cStore) { + try { + checkObxPtr(cStore, 'failed to create store'); + } on ObjectBoxException catch (e) { + // Recognize common problems when trying to open/create a database + // 10199 = OBX_ERROR_STORAGE_GENERAL + // 13 = permissions denied, 30 = read-only filesystem + if (e.message.contains(OBX_ERROR_STORAGE_GENERAL.toString()) && + e.message.contains('Dir does not exist') && + (e.message.endsWith(' (13)') || e.message.endsWith(' (30)'))) { + throw ObjectBoxException(e.message + + ' - this usually indicates a problem with permissions; ' + "if you're using Flutter you may need to use " + 'getApplicationDocumentsDirectory() from the path_provider ' + 'package, see example/README.md'); + } + rethrow; + } + } + + /// Returns if an open store (i.e. opened before and not yet closed) was found + /// for the given [directoryPath] (or if null the [defaultDirectoryPath]). + static bool isOpen(String? directoryPath) { + final path = _safeDirectoryPath(directoryPath); + final cStr = path.toNativeUtf8(); + try { + return C.store_is_open(cStr.cast()); + } finally { + malloc.free(cStr); + } + } + /// Returns a store reference you can use to create a new store instance with /// a single underlying native store. See [Store.fromReference] for more details. ByteData get reference => _reference; @@ -243,7 +311,7 @@ class Store { _reader.clear(); if (!_weak) { - _openStoreDirectories.remove(_dbDir); + _openStoreDirectories.remove(_absoluteDirectoryPath); checkObx(C.store_close(_cStore)); } } diff --git a/objectbox/test/basics_test.dart b/objectbox/test/basics_test.dart index e919703bb..c2b2c469c 100644 --- a/objectbox/test/basics_test.dart +++ b/objectbox/test/basics_test.dart @@ -1,6 +1,8 @@ import 'dart:ffi' as ffi; import 'dart:io'; +import 'dart:isolate'; +import 'package:async/async.dart'; import 'package:objectbox/internal.dart'; import 'package:objectbox/src/native/bindings/bindings.dart'; import 'package:objectbox/src/native/bindings/helpers.dart'; @@ -63,6 +65,58 @@ void main() { env.closeAndDelete(); }); + test('store attach fails if same isolate', () { + final env = TestEnv('basics'); + expect( + () => Store.attach(getObjectBoxModel(), env.dir.path), + throwsA(predicate((UnsupportedError e) => + e.message!.contains('Cannot create multiple Store instances')))); + env.closeAndDelete(); + }); + + test('store attach remains open if main store closed', () async { + final env = TestEnv('basics'); + final store1 = env.store; + final receivePort = ReceivePort(); + final received = StreamQueue(receivePort); + await Isolate.spawn(storeAttachIsolate, + StoreAttachIsolateInit(receivePort.sendPort, env.dir.path)); + final commandPort = await received.next as SendPort; + + // Check native instance pointer is different. + final store2Address = await received.next as int; + expect(InternalStoreAccess.ptr(store1).address, isNot(store2Address)); + + final id = store1.box().put(TestEntity(tString: 'foo')); + expect(id, 1); + // Close original store to test store remains open until all refs closed. + store1.close(); + expect(true, Store.isOpen('testdata-basics')); + + // Read data with attached store. + commandPort.send(id); + final readtString = await received.next as String?; + expect(readtString, isNotNull); + expect(readtString, 'foo'); + + // Close attached store, should close store completely. + commandPort.send(null); + await received.next; + expect(false, Store.isOpen('testdata-basics')); + + // Dispose StreamQueue. + await received.cancel(); + }); + + test('store is open', () { + expect(false, Store.isOpen('')); + expect(false, Store.isOpen('testdata-basics')); + final env = TestEnv('basics'); + expect(true, Store.isOpen('testdata-basics')); + env.closeAndDelete(); + expect(false, Store.isOpen('testdata-basics')); + }); + test('transactions', () { final env = TestEnv('basics'); expect(TxMode.values.length, 2); @@ -139,3 +193,31 @@ void main() { Directory('basics').deleteSync(recursive: true); }); } + +class StoreAttachIsolateInit { + SendPort sendPort; + String path; + + StoreAttachIsolateInit(this.sendPort, this.path); +} + +void storeAttachIsolate(StoreAttachIsolateInit init) async { + final store2 = Store.attach(getObjectBoxModel(), init.path); + + final commandPort = ReceivePort(); + init.sendPort.send(commandPort.sendPort); + init.sendPort.send(InternalStoreAccess.ptr(store2).address); + + await for (final message in commandPort) { + if (message is int) { + final read = store2.box().get(message); + init.sendPort.send(read?.tString); + } else if (message == null) { + store2.close(); + init.sendPort.send(null); + break; + } + } + + print('Store attach isolate finished'); +} diff --git a/objectbox/test/isolates_test.dart b/objectbox/test/isolates_test.dart index 3578016c3..0ee187f56 100644 --- a/objectbox/test/isolates_test.dart +++ b/objectbox/test/isolates_test.dart @@ -45,60 +45,87 @@ void main() { receivePort.close(); }); - /// Work with a single store across multiple isolates. - test('single store in multiple isolates', () async { - final receivePort = ReceivePort(); - final isolate = - await Isolate.spawn(createDataIsolate, receivePort.sendPort); + /// Work with a single store across multiple isolates using + /// the legacy way of passing a pointer reference to the isolate. + test('single store using reference', () async { + await testUsingStoreFromIsolate( + storeCreatorFromRef, (env) => env.store.reference); + }); - final sendPortCompleter = Completer(); - late Completer responseCompleter; - receivePort.listen((dynamic data) { - if (data is SendPort) { - sendPortCompleter.complete(data); - } else { - print('Main received: $data'); - responseCompleter.complete(data); - } - }); + /// Work with a single store across multiple isolates using + /// the directory path to attach to an existing store. + test('single store using attach', () async { + await testUsingStoreFromIsolate(storeCreatorAttach, (env) => env.dir.path); + }); +} - // Receive the SendPort from the Isolate - SendPort sendPort = await sendPortCompleter.future; +// Note: can't use closures, are only supported from Dart SDK 2.15. +Store storeCreatorFromRef(dynamic msg) => + Store.fromReference(getObjectBoxModel(), msg as ByteData); - final call = (dynamic message) { - responseCompleter = Completer(); - sendPort.send(message); - return responseCompleter.future; - }; +Store storeCreatorAttach(dynamic msg) => + Store.attach(getObjectBoxModel(), msg as String); - // Pass the store to the isolate - final env = TestEnv('isolates'); - expect(await call(env.store.reference), equals('store set')); +class IsolateInitMessage { + SendPort sendPort; + Store Function(dynamic) storeCreator; - { - // check simple box operations - expect(env.box.isEmpty(), isTrue); - expect(await call(['put', 'Foo']), equals(1)); // returns inserted id = 1 - expect(env.box.get(1)!.tString, equals('Foo')); - } + IsolateInitMessage(this.sendPort, this.storeCreator); +} - { - // verify that query streams (using observers) work fine across isolates - final queryStream = env.box.query().watch(); - // starts a subscription - final futureFirst = queryStream.map((q) => q.find()).first; - expect(await call(['put', 'Bar']), equals(2)); - List found = await futureFirst.timeout(defaultTimeout); - expect(found.length, equals(2)); - expect(found.last.tString, equals('Bar')); +Future testUsingStoreFromIsolate(Store Function(dynamic) storeCreator, + dynamic Function(TestEnv) storeRefGetter) async { + final receivePort = ReceivePort(); + final initMessage = IsolateInitMessage(receivePort.sendPort, storeCreator); + final isolate = await Isolate.spawn(createDataIsolate, initMessage); + + final sendPortCompleter = Completer(); + late Completer responseCompleter; + receivePort.listen((dynamic data) { + if (data is SendPort) { + sendPortCompleter.complete(data); + } else { + print('Main received: $data'); + responseCompleter.complete(data); } + }); - expect(await call(['close']), equals('done')); + // Receive the SendPort from the Isolate + SendPort sendPort = await sendPortCompleter.future; - isolate.kill(); - receivePort.close(); - env.closeAndDelete(); - }); + final call = (dynamic message) { + responseCompleter = Completer(); + sendPort.send(message); + return responseCompleter.future; + }; + + // Pass the store to the isolate + final env = TestEnv('isolates'); + expect(await call(storeRefGetter(env)), equals('store set')); + + { + // check simple box operations + expect(env.box.isEmpty(), isTrue); + expect(await call(['put', 'Foo']), equals(1)); // returns inserted id = 1 + expect(env.box.get(1)!.tString, equals('Foo')); + } + + { + // verify that query streams (using observers) work fine across isolates + final queryStream = env.box.query().watch(); + // starts a subscription + final futureFirst = queryStream.map((q) => q.find()).first; + expect(await call(['put', 'Bar']), equals(2)); + List found = await futureFirst.timeout(defaultTimeout); + expect(found.length, equals(2)); + expect(found.last.tString, equals('Bar')); + } + + expect(await call(['close']), equals('done')); + + isolate.kill(); + receivePort.close(); + env.closeAndDelete(); } // Echoes back any received message. @@ -118,11 +145,12 @@ void echoIsolate(SendPort sendPort) async { } // Creates data in the background, in the [Store] received as the first message. -void createDataIsolate(SendPort sendPort) async { +void createDataIsolate(IsolateInitMessage initMessage) async { // Open the ReceivePort to listen for incoming messages final port = ReceivePort(); // Send the port where the main isolate can contact us + final sendPort = initMessage.sendPort; sendPort.send(port.sendPort); Store? store; @@ -130,7 +158,7 @@ void createDataIsolate(SendPort sendPort) async { await for (final msg in port) { if (store == null) { // first message data is Store's C pointer address - store = Store.fromReference(getObjectBoxModel(), msg as ByteData); + store = initMessage.storeCreator(msg); sendPort.send('store set'); } else { print('Isolate received: $msg'); From 3d2f25e8f61a7bb847bd4f47a641619f729bd42f Mon Sep 17 00:00:00 2001 From: Markus Date: Mon, 14 Feb 2022 21:02:19 +0100 Subject: [PATCH 2/2] Store: add debugLogs, improved attach fail msg --- objectbox/lib/src/native/store.dart | 17 ++++++++++++++++- objectbox/test/isolates_test.dart | 9 +++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/objectbox/lib/src/native/store.dart b/objectbox/lib/src/native/store.dart index 1aa925048..3b310f71b 100644 --- a/objectbox/lib/src/native/store.dart +++ b/objectbox/lib/src/native/store.dart @@ -9,6 +9,7 @@ import 'dart:typed_data'; import 'package:ffi/ffi.dart'; import 'package:meta/meta.dart'; +import 'package:objectbox/src/native/version.dart'; import 'package:path/path.dart' as path; import '../common.dart'; @@ -30,6 +31,10 @@ class Store { /// Path of the default directory, currently 'objectbox'. static const String defaultDirectoryPath = 'objectbox'; + /// Enables a couple of debug logs. + /// This meant for tests only; do not enable for releases! + static bool debugLogs = false; + late final Pointer _cStore; HashMap? _entityTypeById; final _boxes = HashMap(); @@ -130,6 +135,11 @@ class Store { C.opt_free(opt); rethrow; } + if (debugLogs) { + print('Opening store (C lib V${libraryVersion()})... path=$directory' + ' isOpen=${isOpen(directory)}'); + } + _cStore = C.store_open(opt); _checkStorePointer(_cStore); @@ -233,12 +243,17 @@ class Store { final path = _safeDirectoryPath(directoryPath); final pathCStr = path.toNativeUtf8(); try { + if (debugLogs) { + final isOpen = C.store_is_open(pathCStr.cast()); + print('Attaching to store... path=$path isOpen=$isOpen'); + } _cStore = C.store_attach(pathCStr.cast()); } finally { malloc.free(pathCStr); } - _checkStorePointer(_cStore); + checkObxPtr(_cStore, + 'could not attach to the store at given path - please ensure it was opened before'); // Not setting _reference as this is a replacement for obtaining a store // via reference. diff --git a/objectbox/test/isolates_test.dart b/objectbox/test/isolates_test.dart index 0ee187f56..9c9b4be9f 100644 --- a/objectbox/test/isolates_test.dart +++ b/objectbox/test/isolates_test.dart @@ -55,6 +55,7 @@ void main() { /// Work with a single store across multiple isolates using /// the directory path to attach to an existing store. test('single store using attach', () async { + Store.debugLogs = true; await testUsingStoreFromIsolate(storeCreatorAttach, (env) => env.dir.path); }); } @@ -63,8 +64,10 @@ void main() { Store storeCreatorFromRef(dynamic msg) => Store.fromReference(getObjectBoxModel(), msg as ByteData); -Store storeCreatorAttach(dynamic msg) => - Store.attach(getObjectBoxModel(), msg as String); +Store storeCreatorAttach(dynamic msg) { + Store.debugLogs = true; + return Store.attach(getObjectBoxModel(), msg as String); +} class IsolateInitMessage { SendPort sendPort; @@ -101,6 +104,8 @@ Future testUsingStoreFromIsolate(Store Function(dynamic) storeCreator, // Pass the store to the isolate final env = TestEnv('isolates'); + expect(Store.isOpen('testdata-isolates'), true); + expect(await call(storeRefGetter(env)), equals('store set')); {