From 7a5b013c921a97755c9fa0d3ee733b544f3655ba Mon Sep 17 00:00:00 2001 From: Christian Pauly Date: Tue, 13 Dec 2022 10:12:42 +0100 Subject: [PATCH 1/2] refactor: Remove database transaction workaround The workaround was from the time when we have used sqflite or when Hive had bugs. But now HiveCollections already supports transactions in Dart zones and concurrent write operations shouldn't be a problem anymore. --- lib/src/database/database_api.dart | 2 +- .../database/hive_collections_database.dart | 56 +------------------ lib/src/database/hive_database.dart | 2 +- test/database_api_test.dart | 12 ---- 4 files changed, 4 insertions(+), 68 deletions(-) diff --git a/lib/src/database/database_api.dart b/lib/src/database/database_api.dart index 618b25da..fc519a2d 100644 --- a/lib/src/database/database_api.dart +++ b/lib/src/database/database_api.dart @@ -308,7 +308,7 @@ abstract class DatabaseApi { Future close(); - Future transaction(Future Function() action); + Future transaction(Future Function() action); Future exportDump(); diff --git a/lib/src/database/hive_collections_database.dart b/lib/src/database/hive_collections_database.dart index a096268d..67ed8e69 100644 --- a/lib/src/database/hive_collections_database.dart +++ b/lib/src/database/hive_collections_database.dart @@ -1310,61 +1310,9 @@ class HiveCollectionsDatabase extends DatabaseApi { return; } - Completer? _transactionLock; - final _transactionZones = {}; - @override - Future transaction(Future Function() action) async { - // we want transactions to lock, however NOT if transactoins are run inside of each other. - // to be able to do this, we use dart zones (https://dart.dev/articles/archive/zones). - // _transactionZones holds a set of all zones which are currently running a transaction. - // _transactionLock holds the lock. - - // first we try to determine if we are inside of a transaction currently - var isInTransaction = false; - Zone? zone = Zone.current; - // for that we keep on iterating to the parent zone until there is either no zone anymore - // or we have found a zone inside of _transactionZones. - while (zone != null) { - if (_transactionZones.contains(zone)) { - isInTransaction = true; - break; - } - zone = zone.parent; - } - // if we are inside a transaction....just run the action - if (isInTransaction) { - return await action(); - } - // if we are *not* in a transaction, time to wait for the lock! - while (_transactionLock != null) { - await _transactionLock!.future; - } - // claim the lock - final lock = Completer(); - _transactionLock = lock; - try { - // run the action inside of a new zone - return await runZoned(() async { - try { - // don't forget to add the new zone to _transactionZones! - _transactionZones.add(Zone.current); - late final T result; - await _collection.transaction(() async { - result = await action(); - }); - return result; - } finally { - // aaaand remove the zone from _transactionZones again - _transactionZones.remove(Zone.current); - } - }); - } finally { - // aaaand finally release the lock - _transactionLock = null; - lock.complete(); - } - } + Future transaction(Future Function() action) => + _collection.transaction(action); @override Future updateClient( diff --git a/lib/src/database/hive_database.dart b/lib/src/database/hive_database.dart index 8cda667f..ae744210 100644 --- a/lib/src/database/hive_database.dart +++ b/lib/src/database/hive_database.dart @@ -1255,7 +1255,7 @@ class FamedlySdkHiveDatabase extends DatabaseApi { final _transactionZones = {}; @override - Future transaction(Future Function() action) async { + Future transaction(Future Function() action) async { // we want transactions to lock, however NOT if transactoins are run inside of each other. // to be able to do this, we use dart zones (https://dart.dev/articles/archive/zones). // _transactionZones holds a set of all zones which are currently running a transaction. diff --git a/test/database_api_test.dart b/test/database_api_test.dart index 476d566f..ecdec026 100644 --- a/test/database_api_test.dart +++ b/test/database_api_test.dart @@ -64,18 +64,6 @@ void testDatabase( }); expect(counter++, 3); }); - - // we can't use Zone.root.run inside of tests so we abuse timers instead - Timer(Duration(milliseconds: 50), () async { - await database.transaction(() async { - expect(counter++, 6); - }); - }); - await database.transaction(() async { - expect(counter++, 4); - await Future.delayed(Duration(milliseconds: 100)); - expect(counter++, 5); - }); }); test('insertIntoToDeviceQueue', () async { toDeviceQueueIndex = await database.insertIntoToDeviceQueue( From 956a2f793fafc168f74deb69bb7728d2263dc766 Mon Sep 17 00:00:00 2001 From: Krille Fear Date: Sun, 25 Dec 2022 13:20:09 +0100 Subject: [PATCH 2/2] fix: Add timeout to sync http call --- lib/src/client.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/src/client.dart b/lib/src/client.dart index db9a8467..71e60283 100644 --- a/lib/src/client.dart +++ b/lib/src/client.dart @@ -1569,10 +1569,11 @@ class Client extends MatrixApi { } dynamic syncError; await _checkSyncFilter(); + timeout ??= const Duration(seconds: 30); final syncRequest = sync( filter: syncFilterId, since: prevBatch, - timeout: timeout?.inMilliseconds ?? 30000, + timeout: timeout.inMilliseconds, setPresence: syncPresence, ).then((v) => Future.value(v)).catchError((e) { syncError = e; @@ -1580,7 +1581,8 @@ class Client extends MatrixApi { }); _currentSyncId = syncRequest.hashCode; onSyncStatus.add(SyncStatusUpdate(SyncStatus.waitingForResponse)); - final syncResp = await syncRequest; + final syncResp = + await syncRequest.timeout(timeout + const Duration(seconds: 10)); onSyncStatus.add(SyncStatusUpdate(SyncStatus.processing)); if (syncResp == null) throw syncError ?? 'Unknown sync error'; if (_currentSyncId != syncRequest.hashCode) {