Skip to content

Finalizers for query & property-query #240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions objectbox/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
This is a 1.0 release candidate - we encourage everyone to try it out and provide any last-minute feedback,
especially to new/changed APIs.

* Query now supports auto-closing. You can still call `close()` manually if you want to free native resources sooner
than they would be by Dart's garbage collector, but it's not mandatory anymore.
* Change the "meta-model" fields to provide completely type-safe query building.
Conditions you specify are now checked at compile time to match the queried entity.
* Make property queries fully typed, `PropertyQuery.find()` now returns the appropriate `List<...>` type without casts.
Expand Down
28 changes: 19 additions & 9 deletions objectbox/lib/src/native/bindings/bindings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'objectbox-c.dart';
export 'objectbox-c.dart';

// ignore_for_file: public_member_api_docs
// ignore_for_file: non_constant_identifier_names

/// Tries to use an already loaded objectbox dynamic library. This is the only
/// option for macOS and iOS and is ~5 times faster than loading from file so
Expand All @@ -18,7 +19,8 @@ ObjectBoxC? _tryObjectBoxLibProcess() {

ObjectBoxC? obxc;
try {
obxc = ObjectBoxC(DynamicLibrary.process());
_lib = DynamicLibrary.process();
obxc = ObjectBoxC(_lib!);
_isSupportedVersion(obxc); // may throw in case symbols are not found
} catch (_) {
// ignore errors (i.e. symbol not found)
Expand All @@ -27,19 +29,19 @@ ObjectBoxC? _tryObjectBoxLibProcess() {
}

ObjectBoxC? _tryObjectBoxLibFile() {
DynamicLibrary? lib;
_lib = null;
var libName = 'objectbox';
if (Platform.isWindows) {
libName += '.dll';
try {
lib = DynamicLibrary.open(libName);
_lib = DynamicLibrary.open(libName);
} on ArgumentError {
libName = 'lib/' + libName;
}
} else if (Platform.isMacOS) {
libName = 'lib' + libName + '.dylib';
try {
lib = DynamicLibrary.open(libName);
_lib = DynamicLibrary.open(libName);
} on ArgumentError {
libName = '/usr/local/lib/' + libName;
}
Expand All @@ -50,8 +52,8 @@ ObjectBoxC? _tryObjectBoxLibFile() {
} else {
return null;
}
lib ??= DynamicLibrary.open(libName);
return ObjectBoxC(lib);
_lib ??= DynamicLibrary.open(libName);
return ObjectBoxC(_lib!);
}

bool _isSupportedVersion(ObjectBoxC obxc) => obxc.version_is_at_least(
Expand All @@ -77,9 +79,8 @@ ObjectBoxC loadObjectBoxLib() {
return obxc;
}

ObjectBoxC? _cachedBindings;

ObjectBoxC get C => _cachedBindings ??= loadObjectBoxLib();
DynamicLibrary? _lib;
late final ObjectBoxC C = loadObjectBoxLib();

/// Init DartAPI in C for async callbacks.
///
Expand Down Expand Up @@ -111,3 +112,12 @@ void initializeDartAPI() {
// -1 => failed to initialize - incompatible Dart version
int _dartAPIInitialized = 0;
Object? _dartAPIInitException;

/// A couple of native functions we need as callbacks to pass back to native.
/// Unfortunately, ffigen keeps those private.
typedef _native_close = Int32 Function(Pointer<Void> ptr);

late final native_query_close =
_lib!.lookup<NativeFunction<_native_close>>('obx_query_close');
late final native_query_prop_close =
_lib!.lookup<NativeFunction<_native_close>>('obx_query_prop_close');
5 changes: 1 addition & 4 deletions objectbox/lib/src/native/query/builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,7 @@ class QueryBuilder<T> extends _QueryBuilder<T> {
onListen: subscribe,
onResume: subscribe,
onPause: () => subscription.pause(),
onCancel: () {
subscription.cancel();
query.close();
});
onCancel: () => subscription.cancel());
if (triggerImmediately) controller.add(query);
return controller.stream;
}
Expand Down
25 changes: 22 additions & 3 deletions objectbox/lib/src/native/query/property.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,36 @@ part of query;
/// Property query base.
class PropertyQuery<T> {
final Pointer<OBX_query_prop> _cProp;
late final Pointer<OBX_dart_finalizer> _cFinalizer;
bool _closed = false;
final int _type;
bool _distinct = false;
bool _caseSensitive = false;

PropertyQuery._(Pointer<OBX_query> cQuery, ModelProperty property)
: _type = property.type,
_cProp =
checkObxPtr(C.query_prop(cQuery, property.id.id), 'property query');
_cProp = checkObxPtr(
C.query_prop(cQuery, property.id.id), 'property query') {
_cFinalizer = C.dartc_attach_finalizer(
this, native_query_prop_close, _cProp.cast(), 64);
if (_cFinalizer == nullptr) {
close();
throwLatestNativeError();
}
}

/// Close the property query, freeing its resources
void close() => checkObx(C.query_prop_close(_cProp));
void close() {
if (!_closed) {
_closed = true;
var err = 0;
if (_cFinalizer != nullptr) {
err = C.dartc_detach_finalizer(_cFinalizer, this);
}
checkObx(C.query_prop_close(_cProp));
checkObx(err);
}
}

int _count() {
final ptr = malloc<Uint64>();
Expand Down
25 changes: 21 additions & 4 deletions objectbox/lib/src/native/query/query.dart
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@ class _ConditionGroupAll<EntityT> extends _ConditionGroup<EntityT> {
class Query<T> {
bool _closed = false;
final Pointer<OBX_query> _cQuery;
late final Pointer<OBX_dart_finalizer> _cFinalizer;
final Store _store;
final EntityDefinition<T> _entity;

Expand All @@ -631,7 +632,17 @@ class Query<T> {
}

Query._(this._store, Pointer<OBX_query_builder> cBuilder, this._entity)
: _cQuery = checkObxPtr(C.query(cBuilder), 'create query');
: _cQuery = checkObxPtr(C.query(cBuilder), 'create query') {
initializeDartAPI();

// Keep the finalizer so we can detach it when close() is called manually.
_cFinalizer =
C.dartc_attach_finalizer(this, native_query_close, _cQuery.cast(), 256);
if (_cFinalizer == nullptr) {
close();
throwLatestNativeError();
Comment on lines +642 to +643
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking: can C.query_close work if C.dartc_attach_finalizer failed? E.g. if not, would always replace the original error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, it should work as just fine.

}
}

/// Configure an [offset] for this query.
///
Expand Down Expand Up @@ -675,9 +686,15 @@ class Query<T> {

/// Close the query and free resources.
void close() {
if (_closed) return;
_closed = true;
checkObx(C.query_close(_cQuery));
if (!_closed) {
_closed = true;
var err = 0;
if (_cFinalizer != nullptr) {
err = C.dartc_detach_finalizer(_cFinalizer, this);
}
checkObx(C.query_close(_cQuery));
checkObx(err);
}
}

/// Finds Objects matching the query and returns the first result or null
Expand Down
2 changes: 1 addition & 1 deletion objectbox/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dev_dependencies:
objectbox_generator: any
pedantic: ^1.11.0
test: ^1.16.5
ffigen: ^2.2.2
ffigen: ^2.4.2
# No null-safety compatible version yet and we only need it in tests.
# flat_buffers: 1.12.0

Expand Down
8 changes: 8 additions & 0 deletions objectbox/test/query_property_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ void main() {

final tString = TestEntity_.tString;

test('property query auto-close', () {
// Finalizer is executed after the query object goes out of scope.
// Note: only caught by valgrind - I've tested that it actually catches
// when the finalizer assignment was disabled. Now, this will only fail in
// CI when running valgrind.sh - if finalizer won't work properly.
box.query().build().property(TestEntity_.tString).find();
});

test('.count (basic query)', () {
box.putMany(integerList());
box.putMany(stringList());
Expand Down
8 changes: 8 additions & 0 deletions objectbox/test/query_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ void main() {
});
tearDown(() => env.close());

test('Query auto-close', () {
// Finalizer is executed after the query object goes out of scope.
// Note: only caught by valgrind - I've tested that it actually catches
// when the finalizer assignment was disabled. Now, this will only fail in
// CI when running valgrind.sh - if finalizer won't work properly.
box.query().build().find();
});

test('Query with no conditions, and order as desc ints', () {
box.putMany(<TestEntity>[
TestEntity(tInt: 0),
Expand Down
10 changes: 5 additions & 5 deletions objectbox/test/stream_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,17 @@ void main() {
await sub2.cancel();
});

test('can only use query during listen()', () async {
test('can use query after subscription is canceled', () async {
// This subscribes, gets the first element and cancels immediately.
// We're testing that if user keeps the query instance, they can use it
// later. This is only possible because of query auto-close with finalizers.
final query = await box
.query()
.watch(triggerImmediately: true)
.first
.timeout(defaultTimeout);

expect(
query.count,
throwsA(predicate(
(StateError e) => e.toString().contains('Query already closed'))));
expect(query.count(), 0);
});

test(
Expand Down