From 0dfcd604901a88484cdd260a905ec77969153cd6 Mon Sep 17 00:00:00 2001 From: Stanislav Dmitrenko <7953703+avently@users.noreply.github.com> Date: Thu, 2 Jan 2025 04:31:06 +0700 Subject: [PATCH] android, desktop: moving chats changing in main thread (#5461) * android, desktop: moving chats changing in main thread * modifying chat items in main thread only * comment --- .../simplex/common/platform/UI.android.kt | 11 ++++-- .../chat/simplex/common/model/ChatModel.kt | 39 ++++++++++++------- .../chat/simplex/common/model/SimpleXAPI.kt | 2 +- .../common/views/chat/ChatItemsLoader.kt | 20 +++++----- .../simplex/common/views/chat/ChatView.kt | 8 +++- .../views/chat/group/GroupMemberInfoView.kt | 2 +- .../views/chatlist/ChatListNavLinkView.kt | 16 +++++--- .../views/contacts/ContactListNavView.kt | 4 +- .../common/views/database/DatabaseView.kt | 2 +- .../common/views/newchat/AddGroupView.kt | 2 +- .../kotlin/chat/simplex/common/DesktopApp.kt | 9 ++++- 11 files changed, 74 insertions(+), 41 deletions(-) diff --git a/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/platform/UI.android.kt b/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/platform/UI.android.kt index ae5966b20f..1a4d0b72e9 100644 --- a/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/platform/UI.android.kt +++ b/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/platform/UI.android.kt @@ -14,6 +14,7 @@ import androidx.compose.runtime.* import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.LocalView import chat.simplex.common.AppScreen +import chat.simplex.common.model.ChatModel.withChats import chat.simplex.common.model.clear import chat.simplex.common.model.clearAndNotify import chat.simplex.common.views.helpers.* @@ -74,9 +75,13 @@ actual class GlobalExceptionsHandler: Thread.UncaughtExceptionHandler { if (ModalManager.start.hasModalsOpen()) { ModalManager.start.closeModal() } else if (chatModel.chatId.value != null) { - // Since no modals are open, the problem is probably in ChatView - chatModel.chatId.value = null - chatModel.chatItems.clearAndNotify() + withApi { + withChats { + // Since no modals are open, the problem is probably in ChatView + chatModel.chatId.value = null + chatItems.clearAndNotify() + } + } } else { // ChatList, nothing to do. Maybe to show other view except ChatList } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt index e2fe96e178..f4ffa5e175 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt @@ -61,7 +61,6 @@ object ChatModel { val incompleteInitializedDbRemoved = mutableStateOf(false) private val _chats = mutableStateOf(SnapshotStateList()) val chats: State> = _chats - private val chatsContext = ChatsContext() // map of connections network statuses, key is agent connection id val networkStatuses = mutableStateMapOf() val switchingUsersAndHosts = mutableStateOf(false) @@ -72,7 +71,10 @@ object ChatModel { * If some helper is missing, create it. Notify is needed to track state of items that we added manually (not via api call). See [apiLoadMessages]. * If you use api call to get the items, use just [add] instead of [addAndNotify]. * Never modify underlying list directly because it produces unexpected results in ChatView's LazyColumn (setting by index is ok) */ - val chatItems = mutableStateOf(SnapshotStateList()) + private val _chatItems = mutableStateOf(SnapshotStateList()) + val chatItems: State> = _chatItems + // declaration of chatsContext should be after any other variable that is directly attached to ChatsContext class, otherwise, strange crash with NullPointerException for "this" parameter in random functions + private val chatsContext = ChatsContext() // set listener here that will be notified on every add/delete of a chat item var chatItemsChangesListener: ChatItemsChangesListener? = null val chatState = ActiveChatState() @@ -157,7 +159,6 @@ object ChatModel { val updatingProgress = mutableStateOf(null as Float?) var updatingRequest: Closeable? = null - private val updatingChatsMutex: Mutex = Mutex() val changingActiveUserMutex: Mutex = Mutex() val desktopNoUserNoRemote: Boolean @Composable get() = appPlatform.isDesktop && currentUser.value == null && currentRemoteHost.value == null @@ -336,12 +337,14 @@ object ChatModel { } } - suspend fun withChats(action: suspend ChatsContext.() -> T): T = updatingChatsMutex.withLock { + // running everything inside the block on main thread. Make sure any heavy computation is moved to a background thread + suspend fun withChats(action: suspend ChatsContext.() -> T): T = withContext(Dispatchers.Main) { chatsContext.action() } class ChatsContext { val chats = _chats + val chatItems = _chatItems suspend fun addChat(chat: Chat) { chats.add(index = 0, chat) @@ -762,7 +765,7 @@ object ChatModel { suspend fun addLiveDummy(chatInfo: ChatInfo): ChatItem { val cItem = ChatItem.liveDummy(chatInfo is ChatInfo.Direct) - withContext(Dispatchers.Main) { + withChats { chatItems.addAndNotify(cItem) } return cItem @@ -770,7 +773,11 @@ object ChatModel { fun removeLiveDummy() { if (chatItems.value.lastOrNull()?.id == ChatItem.TEMP_LIVE_CHAT_ITEM_ID) { - chatItems.removeLastAndNotify() + withApi { + withChats { + chatItems.removeLastAndNotify() + } + } } } @@ -891,19 +898,25 @@ object ChatModel { fun replaceConnReqView(id: String, withId: String) { if (id == showingInvitation.value?.connId) { - showingInvitation.value = null - chatModel.chatItems.clearAndNotify() - chatModel.chatId.value = withId + withApi { + withChats { + showingInvitation.value = null + chatItems.clearAndNotify() + chatModel.chatId.value = withId + } + } ModalManager.start.closeModals() ModalManager.end.closeModals() } } - fun dismissConnReqView(id: String) { + fun dismissConnReqView(id: String) = withApi { if (id == showingInvitation.value?.connId) { - showingInvitation.value = null - chatModel.chatItems.clearAndNotify() - chatModel.chatId.value = null + withChats { + showingInvitation.value = null + chatItems.clearAndNotify() + chatModel.chatId.value = null + } // Close NewChatView ModalManager.start.closeModals() ModalManager.center.closeModals() diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt index a86be622b9..7ed138d7fa 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt @@ -3044,8 +3044,8 @@ object ChatController { chatModel.users.addAll(users) chatModel.currentUser.value = user if (user == null) { - chatModel.chatItems.clearAndNotify() withChats { + chatItems.clearAndNotify() chats.clear() popChatCollector.clear() } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatItemsLoader.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatItemsLoader.kt index be09c04ec1..5cbc01271a 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatItemsLoader.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatItemsLoader.kt @@ -45,9 +45,9 @@ suspend fun apiLoadMessages( addChat(chat) } } - withContext(Dispatchers.Main) { + withChats { chatModel.chatItemStatuses.clear() - chatModel.chatItems.replaceAll(chat.chatItems) + chatItems.replaceAll(chat.chatItems) chatModel.chatId.value = chat.chatInfo.id splits.value = newSplits if (chat.chatItems.isNotEmpty()) { @@ -70,8 +70,8 @@ suspend fun apiLoadMessages( ) val insertAt = (indexInCurrentItems - (wasSize - newItems.size) + trimmedIds.size).coerceAtLeast(0) newItems.addAll(insertAt, chat.chatItems) - withContext(Dispatchers.Main) { - chatModel.chatItems.replaceAll(newItems) + withChats { + chatItems.replaceAll(newItems) splits.value = newSplits chatState.moveUnreadAfterItem(oldUnreadSplitIndex, newUnreadSplitIndex, oldItems) } @@ -89,8 +89,8 @@ suspend fun apiLoadMessages( val indexToAdd = min(indexInCurrentItems + 1, newItems.size) val indexToAddIsLast = indexToAdd == newItems.size newItems.addAll(indexToAdd, chat.chatItems) - withContext(Dispatchers.Main) { - chatModel.chatItems.replaceAll(newItems) + withChats { + chatItems.replaceAll(newItems) splits.value = newSplits chatState.moveUnreadAfterItem(splits.value.firstOrNull() ?: newItems.last().id, newItems) // loading clear bottom area, updating number of unread items after the newest loaded item @@ -104,8 +104,8 @@ suspend fun apiLoadMessages( val newSplits = removeDuplicatesAndUpperSplits(newItems, chat, splits, visibleItemIndexesNonReversed) // currently, items will always be added on top, which is index 0 newItems.addAll(0, chat.chatItems) - withContext(Dispatchers.Main) { - chatModel.chatItems.replaceAll(newItems) + withChats { + chatItems.replaceAll(newItems) splits.value = listOf(chat.chatItems.last().id) + newSplits unreadAfterItemId.value = chat.chatItems.last().id totalAfter.value = navInfo.afterTotal @@ -119,8 +119,8 @@ suspend fun apiLoadMessages( newItems.addAll(oldItems) removeDuplicates(newItems, chat) newItems.addAll(chat.chatItems) - withContext(Dispatchers.Main) { - chatModel.chatItems.replaceAll(newItems) + withChats { + chatItems.replaceAll(newItems) unreadAfterNewestLoaded.value = 0 } } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatView.kt index c58561718e..a4c1d40602 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatView.kt @@ -550,7 +550,9 @@ fun ChatView(staleChatId: State, onComposed: suspend (chatId: String) - LaunchedEffect(chatInfo.id) { onComposed(chatInfo.id) ModalManager.end.closeModals() - chatModel.chatItems.clearAndNotify() + withChats { + chatItems.clearAndNotify() + } } } is ChatInfo.InvalidJSON -> { @@ -561,7 +563,9 @@ fun ChatView(staleChatId: State, onComposed: suspend (chatId: String) - LaunchedEffect(chatInfo.id) { onComposed(chatInfo.id) ModalManager.end.closeModals() - chatModel.chatItems.clearAndNotify() + withChats { + chatItems.clearAndNotify() + } } } else -> {} diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupMemberInfoView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupMemberInfoView.kt index e607efeddc..657c0923a5 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupMemberInfoView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupMemberInfoView.kt @@ -98,8 +98,8 @@ fun GroupMemberInfoView( val memberChat = Chat(remoteHostId = rhId, ChatInfo.Direct(memberContact), chatItems = arrayListOf()) withChats { addChat(memberChat) - openLoadedChat(memberChat) } + openLoadedChat(memberChat) closeAll() chatModel.setContactNetworkStatus(memberContact, NetworkStatus.Connected()) } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt index 2f0311b087..4c4c52e58d 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt @@ -216,15 +216,19 @@ suspend fun openChat(rhId: Long?, chatInfo: ChatInfo) = openChat(rhId, chatInfo. private suspend fun openChat(rhId: Long?, chatType: ChatType, apiId: Long) = apiLoadMessages(rhId, chatType, apiId, ChatPagination.Initial(ChatPagination.INITIAL_COUNT), chatModel.chatState) -fun openLoadedChat(chat: Chat) { - chatModel.chatItemStatuses.clear() - chatModel.chatItems.replaceAll(chat.chatItems) - chatModel.chatId.value = chat.chatInfo.id - chatModel.chatState.clear() +suspend fun openLoadedChat(chat: Chat) { + withChats { + chatModel.chatItemStatuses.clear() + chatItems.replaceAll(chat.chatItems) + chatModel.chatId.value = chat.chatInfo.id + chatModel.chatState.clear() + } } suspend fun apiFindMessages(ch: Chat, search: String) { - chatModel.chatItems.clearAndNotify() + withChats { + chatItems.clearAndNotify() + } apiLoadMessages(ch.remoteHostId, ch.chatInfo.chatType, ch.chatInfo.apiId, pagination = ChatPagination.Last(ChatPagination.INITIAL_COUNT), chatModel.chatState, search = search) } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/contacts/ContactListNavView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/contacts/ContactListNavView.kt index 0af8e7ca38..da70aef621 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/contacts/ContactListNavView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/contacts/ContactListNavView.kt @@ -21,7 +21,9 @@ fun onRequestAccepted(chat: Chat) { if (chatInfo is ChatInfo.Direct) { ModalManager.start.closeModals() if (chatInfo.contact.sndReady) { - openLoadedChat(chat) + withApi { + openLoadedChat(chat) + } } } } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/database/DatabaseView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/database/DatabaseView.kt index 28772f01d3..933bc0c93a 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/database/DatabaseView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/database/DatabaseView.kt @@ -528,9 +528,9 @@ fun deleteChatDatabaseFilesAndState() { // Clear sensitive data on screen just in case ModalManager will fail to prevent hiding its modals while database encrypts itself chatModel.chatId.value = null - chatModel.chatItems.clearAndNotify() withLongRunningApi { withChats { + chatItems.clearAndNotify() chats.clear() popChatCollector.clear() } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/AddGroupView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/AddGroupView.kt index 6cecbe4979..aa23eb355f 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/AddGroupView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/AddGroupView.kt @@ -44,7 +44,7 @@ fun AddGroupView(chatModel: ChatModel, rh: RemoteHostInfo?, close: () -> Unit, c if (groupInfo != null) { withChats { updateGroup(rhId = rhId, groupInfo) - chatModel.chatItems.clearAndNotify() + chatItems.clearAndNotify() chatModel.chatItemStatuses.clear() chatModel.chatId.value = groupInfo.id } diff --git a/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/DesktopApp.kt b/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/DesktopApp.kt index 2702862e47..221f1a1291 100644 --- a/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/DesktopApp.kt +++ b/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/DesktopApp.kt @@ -14,6 +14,7 @@ import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.unit.dp import androidx.compose.ui.window.* import chat.simplex.common.model.* +import chat.simplex.common.model.ChatModel.withChats import chat.simplex.common.platform.* import chat.simplex.common.ui.theme.DEFAULT_START_MODAL_WIDTH import chat.simplex.common.ui.theme.SimpleXTheme @@ -55,8 +56,12 @@ fun showApp() { // Better to not close fullscreen since it can contain passcode } else { // The last possible cause that can be closed - chatModel.chatId.value = null - chatModel.chatItems.clearAndNotify() + withApi { + withChats { + chatModel.chatId.value = null + chatItems.clearAndNotify() + } + } } chatModel.activeCall.value?.let { withBGApi {