From 8edccb8a0f3b47999533048b1cf5149d32840e16 Mon Sep 17 00:00:00 2001 From: Christien Rioux Date: Sun, 4 Aug 2024 18:49:49 -0500 Subject: [PATCH] fix deadlock clean up async handling improve styled alerts --- assets/i18n/en.json | 1 + .../cubits/account_record_cubit.dart | 4 +- ...per_account_collection_bloc_map_cubit.dart | 6 +- .../cubits/per_account_collection_cubit.dart | 8 +-- .../views/edit_account_page.dart | 55 +++++++++---------- .../cubits/contact_invitation_list_cubit.dart | 2 +- .../waiting_invitations_bloc_map_cubit.dart | 6 +- .../models/valid_contact_invitation.dart | 2 +- .../active_conversations_bloc_map_cubit.dart | 4 +- ...ve_single_contact_chat_bloc_map_cubit.dart | 29 ++++------ .../cubits/conversation_cubit.dart | 5 +- lib/theme/views/styled_alert.dart | 16 +++--- lib/tick.dart | 6 +- packages/veilid_support/example/pubspec.lock | 10 ++-- packages/veilid_support/example/pubspec.yaml | 2 +- .../src/dht_record/dht_record.dart | 2 +- .../src/dht_record/dht_record_pool.dart | 41 +++++++++++--- packages/veilid_support/pubspec.lock | 8 +-- packages/veilid_support/pubspec.yaml | 10 ++-- pubspec.lock | 8 +-- pubspec.yaml | 8 +-- 21 files changed, 125 insertions(+), 108 deletions(-) diff --git a/assets/i18n/en.json b/assets/i18n/en.json index 552da6f..7e968da 100644 --- a/assets/i18n/en.json +++ b/assets/i18n/en.json @@ -4,6 +4,7 @@ }, "menu": { "accounts_menu_tooltip": "Accounts Menu", + "settings_tooltip": "Settings", "contacts_tooltip": "Contacts List", "new_chat_tooltip": "Start New Chat", "add_account_tooltip": "Add Account", diff --git a/lib/account_manager/cubits/account_record_cubit.dart b/lib/account_manager/cubits/account_record_cubit.dart index ee8ec89..a0a24a7 100644 --- a/lib/account_manager/cubits/account_record_cubit.dart +++ b/lib/account_manager/cubits/account_record_cubit.dart @@ -8,7 +8,7 @@ import '../../proto/proto.dart' as proto; import '../account_manager.dart'; typedef AccountRecordState = proto.Account; -typedef _sspUpdateState = ( +typedef _SspUpdateState = ( AccountSpec accountSpec, Future Function() onSuccess ); @@ -96,5 +96,5 @@ class AccountRecordCubit extends DefaultDHTRecordCubit { } } - final _sspUpdate = SingleStateProcessor<_sspUpdateState>(); + final _sspUpdate = SingleStateProcessor<_SspUpdateState>(); } diff --git a/lib/account_manager/cubits/per_account_collection_bloc_map_cubit.dart b/lib/account_manager/cubits/per_account_collection_bloc_map_cubit.dart index f5334c1..ded50e4 100644 --- a/lib/account_manager/cubits/per_account_collection_bloc_map_cubit.dart +++ b/lib/account_manager/cubits/per_account_collection_bloc_map_cubit.dart @@ -24,13 +24,13 @@ class PerAccountCollectionBlocMapCubit extends BlocMapCubit _addPerAccountCollectionCubit( {required TypedKey superIdentityRecordKey}) async => - add(() => MapEntry( + add( superIdentityRecordKey, - PerAccountCollectionCubit( + () async => PerAccountCollectionCubit( locator: _locator, accountInfoCubit: AccountInfoCubit( accountRepository: _accountRepository, - superIdentityRecordKey: superIdentityRecordKey)))); + superIdentityRecordKey: superIdentityRecordKey))); /// StateFollower ///////////////////////// diff --git a/lib/account_manager/cubits/per_account_collection_cubit.dart b/lib/account_manager/cubits/per_account_collection_cubit.dart index 226d213..089443a 100644 --- a/lib/account_manager/cubits/per_account_collection_cubit.dart +++ b/lib/account_manager/cubits/per_account_collection_cubit.dart @@ -78,13 +78,13 @@ class PerAccountCollectionCubit extends Cubit { await _accountRecordSubscription?.cancel(); _accountRecordSubscription = null; - // Update state to 'loading' - nextState = _updateAccountRecordState(nextState, null); - emit(nextState); - // Close AccountRecordCubit await accountRecordCubit?.close(); accountRecordCubit = null; + + // Update state to 'loading' + nextState = _updateAccountRecordState(nextState, null); + emit(nextState); } else { ///////////////// Logged in /////////////////// diff --git a/lib/account_manager/views/edit_account_page.dart b/lib/account_manager/views/edit_account_page.dart index eaa0e37..5674449 100644 --- a/lib/account_manager/views/edit_account_page.dart +++ b/lib/account_manager/views/edit_account_page.dart @@ -120,22 +120,22 @@ class _EditAccountPageState extends WindowSetupState { try { final success = await AccountRepository.instance.deleteLocalAccount( widget.superIdentityRecordKey, widget.accountRecord); - if (success && mounted) { - context - .read() - .info(text: translate('edit_account_page.account_removed')); - GoRouterHelper(context).pop(); - } else if (mounted) { - context - .read() - .error(text: translate('edit_account_page.failed_to_remove')); + if (mounted) { + if (success) { + context + .read() + .info(text: translate('edit_account_page.account_removed')); + GoRouterHelper(context).pop(); + } else { + context + .read() + .error(text: translate('edit_account_page.failed_to_remove')); + } } } finally { - if (mounted) { - setState(() { - _isInAsyncCall = false; - }); - } + setState(() { + _isInAsyncCall = false; + }); } } on Exception catch (e, st) { if (mounted) { @@ -188,22 +188,21 @@ class _EditAccountPageState extends WindowSetupState { try { final success = await AccountRepository.instance.destroyAccount( widget.superIdentityRecordKey, widget.accountRecord); - if (success && mounted) { - context - .read() - .info(text: translate('edit_account_page.account_destroyed')); - GoRouterHelper(context).pop(); - } else if (mounted) { - context - .read() - .error(text: translate('edit_account_page.failed_to_destroy')); + if (mounted) { + if (success) { + context + .read() + .info(text: translate('edit_account_page.account_destroyed')); + GoRouterHelper(context).pop(); + } else { + context.read().error( + text: translate('edit_account_page.failed_to_destroy')); + } } } finally { - if (mounted) { - setState(() { - _isInAsyncCall = false; - }); - } + setState(() { + _isInAsyncCall = false; + }); } } on Exception catch (e, st) { if (mounted) { diff --git a/lib/contact_invitation/cubits/contact_invitation_list_cubit.dart b/lib/contact_invitation/cubits/contact_invitation_list_cubit.dart index f5663af..959445c 100644 --- a/lib/contact_invitation/cubits/contact_invitation_list_cubit.dart +++ b/lib/contact_invitation/cubits/contact_invitation_list_cubit.dart @@ -252,7 +252,7 @@ class ContactInvitationListCubit .openRecordRead(contactRequestInboxKey, debugName: 'ContactInvitationListCubit::validateInvitation::' 'ContactRequestInbox', - parent: pool.getParentRecordKey(contactRequestInboxKey) ?? + parent: await pool.getParentRecordKey(contactRequestInboxKey) ?? _accountInfo.accountRecordKey) .withCancel(cancelRequest)) .maybeDeleteScope(!isSelf, (contactRequestInbox) async { diff --git a/lib/contact_invitation/cubits/waiting_invitations_bloc_map_cubit.dart b/lib/contact_invitation/cubits/waiting_invitations_bloc_map_cubit.dart index 3787205..953575d 100644 --- a/lib/contact_invitation/cubits/waiting_invitations_bloc_map_cubit.dart +++ b/lib/contact_invitation/cubits/waiting_invitations_bloc_map_cubit.dart @@ -48,15 +48,15 @@ class WaitingInvitationsBlocMapCubit extends BlocMapCubit _addWaitingInvitation( {required proto.ContactInvitationRecord contactInvitationRecord}) async => - add(() => MapEntry( + add( contactInvitationRecord.contactRequestInbox.recordKey.toVeilid(), - WaitingInvitationCubit( + () async => WaitingInvitationCubit( ContactRequestInboxCubit( accountInfo: _accountInfo, contactInvitationRecord: contactInvitationRecord), accountInfo: _accountInfo, accountRecordCubit: _accountRecordCubit, - contactInvitationRecord: contactInvitationRecord))); + contactInvitationRecord: contactInvitationRecord)); // Process all accepted or rejected invitations Future _invitationStatusListener( diff --git a/lib/contact_invitation/models/valid_contact_invitation.dart b/lib/contact_invitation/models/valid_contact_invitation.dart index fb8b8de..f19e951 100644 --- a/lib/contact_invitation/models/valid_contact_invitation.dart +++ b/lib/contact_invitation/models/valid_contact_invitation.dart @@ -37,7 +37,7 @@ class ValidContactInvitation { return (await pool.openRecordWrite(_contactRequestInboxKey, _writer, debugName: 'ValidContactInvitation::accept::' 'ContactRequestInbox', - parent: pool.getParentRecordKey(_contactRequestInboxKey) ?? + parent: await pool.getParentRecordKey(_contactRequestInboxKey) ?? _accountInfo.accountRecordKey)) // ignore: prefer_expression_function_bodies .maybeDeleteScope(!isSelf, (contactRequestInbox) async { diff --git a/lib/conversation/cubits/active_conversations_bloc_map_cubit.dart b/lib/conversation/cubits/active_conversations_bloc_map_cubit.dart index 4330db6..fbf0e80 100644 --- a/lib/conversation/cubits/active_conversations_bloc_map_cubit.dart +++ b/lib/conversation/cubits/active_conversations_bloc_map_cubit.dart @@ -78,7 +78,7 @@ class ActiveConversationsBlocMapCubit extends BlocMapCubit - add(() { + add(localConversationRecordKey, () async { // Conversation cubit the tracks the state between the local // and remote halves of a contact's relationship with this account final conversationCubit = ConversationCubit( @@ -123,7 +123,7 @@ class ActiveConversationsBlocMapCubit extends BlocMapCubit _addConversationMessages(_SingleContactChatState state) async { // xxx could use atomic update() function - - final cubit = await tryOperateAsync( - state.localConversationRecordKey, closure: (cubit) async { - await cubit.updateRemoteMessagesRecordKey(state.remoteMessagesRecordKey); - return cubit; - }); - if (cubit == null) { - await add(() => MapEntry( - state.localConversationRecordKey, - SingleContactMessagesCubit( - accountInfo: _accountInfo, - remoteIdentityPublicKey: state.remoteIdentityPublicKey, - localConversationRecordKey: state.localConversationRecordKey, - remoteConversationRecordKey: state.remoteConversationRecordKey, - localMessagesRecordKey: state.localMessagesRecordKey, - remoteMessagesRecordKey: state.remoteMessagesRecordKey, - ))); - } + await update(state.localConversationRecordKey, + onUpdate: (cubit) async => + cubit.updateRemoteMessagesRecordKey(state.remoteMessagesRecordKey), + onCreate: () async => SingleContactMessagesCubit( + accountInfo: _accountInfo, + remoteIdentityPublicKey: state.remoteIdentityPublicKey, + localConversationRecordKey: state.localConversationRecordKey, + remoteConversationRecordKey: state.remoteConversationRecordKey, + localMessagesRecordKey: state.localMessagesRecordKey, + remoteMessagesRecordKey: state.remoteMessagesRecordKey, + )); } _SingleContactChatState? _mapStateValue( diff --git a/lib/conversation/cubits/conversation_cubit.dart b/lib/conversation/cubits/conversation_cubit.dart index b5895d9..e2a1802 100644 --- a/lib/conversation/cubits/conversation_cubit.dart +++ b/lib/conversation/cubits/conversation_cubit.dart @@ -73,8 +73,9 @@ class ConversationCubit extends Cubit> { final record = await pool.openRecordRead(_remoteConversationRecordKey, debugName: 'ConversationCubit::RemoteConversation', - parent: pool.getParentRecordKey(_remoteConversationRecordKey) ?? - accountInfo.accountRecordKey, + parent: + await pool.getParentRecordKey(_remoteConversationRecordKey) ?? + accountInfo.accountRecordKey, crypto: crypto); return record; diff --git a/lib/theme/views/styled_alert.dart b/lib/theme/views/styled_alert.dart index 2104fa1..eb5b492 100644 --- a/lib/theme/views/styled_alert.dart +++ b/lib/theme/views/styled_alert.dart @@ -95,8 +95,8 @@ Future showErrorModal( required String title, required String text}) async { final theme = Theme.of(context); - final scale = theme.extension()!; - final scaleConfig = theme.extension()!; + // final scale = theme.extension()!; + // final scaleConfig = theme.extension()!; await Alert( context: context, @@ -145,8 +145,8 @@ Future showWarningModal( required String title, required String text}) async { final theme = Theme.of(context); - final scale = theme.extension()!; - final scaleConfig = theme.extension()!; + // final scale = theme.extension()!; + // final scaleConfig = theme.extension()!; await Alert( context: context, @@ -184,8 +184,8 @@ Future showWarningWidgetModal( required String title, required Widget child}) async { final theme = Theme.of(context); - final scale = theme.extension()!; - final scaleConfig = theme.extension()!; + // final scale = theme.extension()!; + // final scaleConfig = theme.extension()!; await Alert( context: context, @@ -223,8 +223,8 @@ Future showConfirmModal( required String title, required String text}) async { final theme = Theme.of(context); - final scale = theme.extension()!; - final scaleConfig = theme.extension()!; + // final scale = theme.extension()!; + // final scaleConfig = theme.extension()!; var confirm = false; diff --git a/lib/tick.dart b/lib/tick.dart index 8ec3db7..21c18a3 100644 --- a/lib/tick.dart +++ b/lib/tick.dart @@ -28,11 +28,7 @@ class BackgroundTickerState extends State { @override void dispose() { - final tickTimer = _tickTimer; - if (tickTimer != null) { - tickTimer.cancel(); - } - + _tickTimer?.cancel(); super.dispose(); } diff --git a/packages/veilid_support/example/pubspec.lock b/packages/veilid_support/example/pubspec.lock index aa857d4..236861d 100644 --- a/packages/veilid_support/example/pubspec.lock +++ b/packages/veilid_support/example/pubspec.lock @@ -37,10 +37,10 @@ packages: dependency: "direct dev" description: name: async_tools - sha256: "9166e8fe65fc65eb79202a6d540f4de768553d78141b885f5bd3f8d7d30eef5e" + sha256: "93df8b92d54d92e3323c630277e902b4ad4f05f798b55cfbc451e98c3e2fb7ba" url: "https://pub.dev" source: hosted - version: "0.1.5" + version: "0.1.6" bloc: dependency: transitive description: @@ -53,10 +53,10 @@ packages: dependency: transitive description: name: bloc_advanced_tools - sha256: "2b2dd492a350e7192a933d09f15ea04d5d00e7bd3fe2a906fe629cd461ddbf94" + sha256: f0b2dbe028792c97d1eb30480ed4e8035b5c70ea3bcc95a9c5255142592857f7 url: "https://pub.dev" source: hosted - version: "0.1.5" + version: "0.1.7" boolean_selector: dependency: transitive description: @@ -650,7 +650,7 @@ packages: path: "../../../../veilid/veilid-flutter" relative: true source: path - version: "0.3.3" + version: "0.3.4" veilid_support: dependency: "direct main" description: diff --git a/packages/veilid_support/example/pubspec.yaml b/packages/veilid_support/example/pubspec.yaml index eb1c45a..17d17f4 100644 --- a/packages/veilid_support/example/pubspec.yaml +++ b/packages/veilid_support/example/pubspec.yaml @@ -14,7 +14,7 @@ dependencies: path: ../ dev_dependencies: - async_tools: ^0.1.5 + async_tools: ^0.1.6 integration_test: sdk: flutter lint_hard: ^4.0.0 diff --git a/packages/veilid_support/lib/dht_support/src/dht_record/dht_record.dart b/packages/veilid_support/lib/dht_support/src/dht_record/dht_record.dart index a0994bf..9aa380c 100644 --- a/packages/veilid_support/lib/dht_support/src/dht_record/dht_record.dart +++ b/packages/veilid_support/lib/dht_support/src/dht_record/dht_record.dart @@ -79,7 +79,7 @@ class DHTRecord implements DHTDeleteable { return false; } - await serialFuturePause((this, _sfListen)); + await serialFutureClose((this, _sfListen)); await _watchController?.close(); _watchController = null; await DHTRecordPool.instance._recordClosed(this); diff --git a/packages/veilid_support/lib/dht_support/src/dht_record/dht_record_pool.dart b/packages/veilid_support/lib/dht_support/src/dht_record/dht_record_pool.dart index f1ce324..cddea38 100644 --- a/packages/veilid_support/lib/dht_support/src/dht_record/dht_record_pool.dart +++ b/packages/veilid_support/lib/dht_support/src/dht_record/dht_record_pool.dart @@ -65,7 +65,7 @@ class OwnedDHTRecordPointer with _$OwnedDHTRecordPointer { class DHTRecordPool with TableDBBackedJson { DHTRecordPool._(Veilid veilid, VeilidRoutingContext routingContext) : _state = const DHTRecordPoolAllocations(), - _mutex = Mutex(), + _mutex = Mutex(debugLockTimeout: 30), _recordTagLock = AsyncTagLock(), _opened = {}, _markedForDelete = {}, @@ -207,10 +207,8 @@ class DHTRecordPool with TableDBBackedJson { ); /// Get the parent of a DHTRecord key if it exists - TypedKey? getParentRecordKey(TypedKey child) { - final childJson = child.toJson(); - return _state.parentByChild[childJson]; - } + Future getParentRecordKey(TypedKey child) => + _mutex.protect(() async => _getParentRecordKeyInner(child)); /// Check if record is allocated Future isValidRecordKey(TypedKey key) => @@ -505,12 +503,16 @@ class DHTRecordPool with TableDBBackedJson { // Check to see if this key can finally be deleted // If any parents are marked for deletion, try them first Future _checkForLateDeletesInner(TypedKey key) async { + if (!_mutex.isLocked) { + throw StateError('should be locked here'); + } + // Get parent list in bottom up order including our own key final parents = []; TypedKey? nextParent = key; while (nextParent != null) { parents.add(nextParent); - nextParent = getParentRecordKey(nextParent); + nextParent = _getParentRecordKeyInner(nextParent); } // If any parent is ready to delete all its children do it @@ -547,6 +549,10 @@ class DHTRecordPool with TableDBBackedJson { // Actual delete function Future _finalizeDeleteRecordInner(TypedKey recordKey) async { + if (!_mutex.isLocked) { + throw StateError('should be locked here'); + } + log('_finalizeDeleteRecordInner: key=$recordKey'); // Remove this child from parents @@ -557,6 +563,10 @@ class DHTRecordPool with TableDBBackedJson { // Deep delete mechanism inside mutex Future _deleteRecordInner(TypedKey recordKey) async { + if (!_mutex.isLocked) { + throw StateError('should be locked here'); + } + final toDelete = _readyForDeleteInner(recordKey); if (toDelete.isNotEmpty) { // delete now @@ -656,7 +666,20 @@ class DHTRecordPool with TableDBBackedJson { } } + TypedKey? _getParentRecordKeyInner(TypedKey child) { + if (!_mutex.isLocked) { + throw StateError('should be locked here'); + } + + final childJson = child.toJson(); + return _state.parentByChild[childJson]; + } + bool _isValidRecordKeyInner(TypedKey key) { + if (!_mutex.isLocked) { + throw StateError('should be locked here'); + } + if (_state.rootRecords.contains(key)) { return true; } @@ -667,6 +690,10 @@ class DHTRecordPool with TableDBBackedJson { } bool _isDeletedRecordKeyInner(TypedKey key) { + if (!_mutex.isLocked) { + throw StateError('should be locked here'); + } + // Is this key gone? if (!_isValidRecordKeyInner(key)) { return true; @@ -679,7 +706,7 @@ class DHTRecordPool with TableDBBackedJson { if (_markedForDelete.contains(nextParent)) { return true; } - nextParent = getParentRecordKey(nextParent); + nextParent = _getParentRecordKeyInner(nextParent); } return false; diff --git a/packages/veilid_support/pubspec.lock b/packages/veilid_support/pubspec.lock index c3162cf..1fdb0d3 100644 --- a/packages/veilid_support/pubspec.lock +++ b/packages/veilid_support/pubspec.lock @@ -37,10 +37,10 @@ packages: dependency: "direct main" description: name: async_tools - sha256: "9166e8fe65fc65eb79202a6d540f4de768553d78141b885f5bd3f8d7d30eef5e" + sha256: "93df8b92d54d92e3323c630277e902b4ad4f05f798b55cfbc451e98c3e2fb7ba" url: "https://pub.dev" source: hosted - version: "0.1.5" + version: "0.1.6" bloc: dependency: "direct main" description: @@ -53,10 +53,10 @@ packages: dependency: "direct main" description: name: bloc_advanced_tools - sha256: "2ad82be752ab5e983ad9097ed9f334e47a4472c04d5c6b61c99a1bb14a039053" + sha256: f0b2dbe028792c97d1eb30480ed4e8035b5c70ea3bcc95a9c5255142592857f7 url: "https://pub.dev" source: hosted - version: "0.1.6" + version: "0.1.7" boolean_selector: dependency: transitive description: diff --git a/packages/veilid_support/pubspec.yaml b/packages/veilid_support/pubspec.yaml index 280ef3b..59b821d 100644 --- a/packages/veilid_support/pubspec.yaml +++ b/packages/veilid_support/pubspec.yaml @@ -7,9 +7,9 @@ environment: sdk: '>=3.2.0 <4.0.0' dependencies: - async_tools: ^0.1.5 + async_tools: ^0.1.6 bloc: ^8.1.4 - bloc_advanced_tools: ^0.1.6 + bloc_advanced_tools: ^0.1.7 charcode: ^1.3.1 collection: ^1.18.0 equatable: ^2.0.5 @@ -26,11 +26,11 @@ dependencies: # veilid: ^0.0.1 path: ../../../veilid/veilid-flutter -#dependency_overrides: +# dependency_overrides: # async_tools: # path: ../../../dart_async_tools -# bloc_advanced_tools: -# path: ../../../bloc_advanced_tools +# bloc_advanced_tools: +# path: ../../../bloc_advanced_tools dev_dependencies: build_runner: ^2.4.10 diff --git a/pubspec.lock b/pubspec.lock index 33a7671..bc10095 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -85,10 +85,10 @@ packages: dependency: "direct main" description: name: async_tools - sha256: "9166e8fe65fc65eb79202a6d540f4de768553d78141b885f5bd3f8d7d30eef5e" + sha256: "93df8b92d54d92e3323c630277e902b4ad4f05f798b55cfbc451e98c3e2fb7ba" url: "https://pub.dev" source: hosted - version: "0.1.5" + version: "0.1.6" awesome_extensions: dependency: "direct main" description: @@ -141,10 +141,10 @@ packages: dependency: "direct main" description: name: bloc_advanced_tools - sha256: "2ad82be752ab5e983ad9097ed9f334e47a4472c04d5c6b61c99a1bb14a039053" + sha256: f0b2dbe028792c97d1eb30480ed4e8035b5c70ea3bcc95a9c5255142592857f7 url: "https://pub.dev" source: hosted - version: "0.1.6" + version: "0.1.7" blurry_modal_progress_hud: dependency: "direct main" description: diff --git a/pubspec.yaml b/pubspec.yaml index 871ba4d..e9efd02 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -14,12 +14,12 @@ dependencies: animated_theme_switcher: ^2.0.10 ansicolor: ^2.0.2 archive: ^3.6.1 - async_tools: ^0.1.5 + async_tools: ^0.1.6 awesome_extensions: ^2.0.16 badges: ^3.1.2 basic_utils: ^5.7.0 bloc: ^8.1.4 - bloc_advanced_tools: ^0.1.6 + bloc_advanced_tools: ^0.1.7 blurry_modal_progress_hud: ^1.1.1 change_case: ^2.1.0 charcode: ^1.3.1 @@ -114,8 +114,8 @@ dependencies: # dependency_overrides: # async_tools: # path: ../dart_async_tools -# bloc_advanced_tools: -# path: ../bloc_advanced_tools +# bloc_advanced_tools: +# path: ../bloc_advanced_tools # searchable_listview: # path: ../Searchable-Listview # flutter_chat_ui: