IntenseFinance/simplex-chat
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

android, desktop: possible fix of chat items race (#3520)

Browse Source 
* android, desktop: possible fix of chat items race

* fix compose threads desynchronization

* optimization and preventing race

* removed unused logs

---------

Co-authored-by: Evgeny Poberezkin <2769109+epoberezkin@users.noreply.github.com>
Co-authored-by: Evgeny Poberezkin <evgeny@poberezkin.com>
This commit is contained in:
Stanislav Dmitrenko 2024-01-28 01:46:27 +07:00 committed by GitHub
parent c4cbb49f57
commit 9b35ff6f11
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 86 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/platform/UI.android.kt
View File

@ -12,6 +12,8 @@ import androidx.activity.compose.setContent
import androidx.compose.runtime.*
import androidx.compose.ui.platform.LocalView
import chat.simplex.common.AppScreen
import chat.simplex.common.model.clear
import chat.simplex.common.ui.theme.SimpleXTheme
import chat.simplex.common.views.helpers.*
import androidx.compose.ui.platform.LocalContext as LocalContext1
import chat.simplex.res.MR

86
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt
View File

@ -2,6 +2,7 @@ package chat.simplex.common.model
import androidx.compose.material.*
import androidx.compose.runtime.*
import androidx.compose.runtime.snapshots.SnapshotStateList
import androidx.compose.runtime.snapshots.SnapshotStateMap
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.text.SpanStyle
@ -56,7 +57,7 @@ object ChatModel {
// current chat
val chatId = mutableStateOf<String?>(null)
val chatItems = mutableStateListOf<ChatItem>()
val chatItems = mutableStateOf(SnapshotStateList<ChatItem>())
// rhId, chatId
val deletedChats = mutableStateOf<List<Pair<Long?, String>>>(emptyList())
val chatItemStatuses = mutableMapOf<Long, CIStatus>()
@ -268,18 +269,15 @@ object ChatModel {
} else {
addChat(Chat(remoteHostId = rhId, chatInfo = cInfo, chatItems = arrayListOf(cItem)))
}
Log.d(TAG, "TODOCHAT: addChatItem: adding to chat ${chatId.value} from ${cInfo.id} ${cItem.id}, size ${chatItems.size}")
withContext(Dispatchers.Main) {
// add to current chat
if (chatId.value == cInfo.id) {
Log.d(TAG, "TODOCHAT: addChatItem: chatIds are equal, size ${chatItems.size}")
// Prevent situation when chat item already in the list received from backend
if (chatItems.none { it.id == cItem.id }) {
if (chatItems.lastOrNull()?.id == ChatItem.TEMP_LIVE_CHAT_ITEM_ID) {
chatItems.add(kotlin.math.max(0, chatItems.lastIndex), cItem)
if (chatItems.value.none { it.id == cItem.id }) {
if (chatItems.value.lastOrNull()?.id == ChatItem.TEMP_LIVE_CHAT_ITEM_ID) {
chatItems.add(kotlin.math.max(0, chatItems.value.lastIndex), cItem)
} else {
chatItems.add(cItem)
Log.d(TAG, "TODOCHAT: addChatItem: added to chat ${chatId.value} from ${cInfo.id} ${cItem.id}, size ${chatItems.size}")
}
}
}
@ -306,14 +304,13 @@ object ChatModel {
addChat(Chat(remoteHostId = rhId, chatInfo = cInfo, chatItems = arrayListOf(cItem)))
res = true
}
Log.d(TAG, "TODOCHAT: upsertChatItem: upserting to chat ${chatId.value} from ${cInfo.id} ${cItem.id}, size ${chatItems.size}")
return withContext(Dispatchers.Main) {
// update current chat
if (chatId.value == cInfo.id) {
val itemIndex = chatItems.indexOfFirst { it.id == cItem.id }
val items = chatItems.value
val itemIndex = items.indexOfFirst { it.id == cItem.id }
if (itemIndex >= 0) {
chatItems[itemIndex] = cItem
Log.d(TAG, "TODOCHAT: upsertChatItem: updated in chat $chatId from ${cInfo.id} ${cItem.id}, size ${chatItems.size}")
items[itemIndex] = cItem
false
} else {
val status = chatItemStatuses.remove(cItem.id)
@ -323,7 +320,6 @@ object ChatModel {
cItem
}
chatItems.add(ci)
Log.d(TAG, "TODOCHAT: upsertChatItem: added to chat $chatId from ${cInfo.id} ${cItem.id}, size ${chatItems.size}")
true
}
} else {
@ -335,9 +331,10 @@ object ChatModel {
suspend fun updateChatItem(cInfo: ChatInfo, cItem: ChatItem, status: CIStatus? = null) {
withContext(Dispatchers.Main) {
if (chatId.value == cInfo.id) {
val itemIndex = chatItems.indexOfFirst { it.id == cItem.id }
val items = chatItems.value
val itemIndex = items.indexOfFirst { it.id == cItem.id }
if (itemIndex >= 0) {
chatItems[itemIndex] = cItem
items[itemIndex] = cItem
}
} else if (status != null) {
chatItemStatuses[cItem.id] = status
@ -361,10 +358,10 @@ object ChatModel {
}
// remove from current chat
if (chatId.value == cInfo.id) {
val itemIndex = chatItems.indexOfFirst { it.id == cItem.id }
if (itemIndex >= 0) {
AudioPlayer.stop(chatItems[itemIndex])
chatItems.removeAt(itemIndex)
chatItems.removeAll {
val remove = it.id == cItem.id
if (remove) { AudioPlayer.stop(it) }
remove
}
}
}
@ -405,7 +402,7 @@ object ChatModel {
}
fun removeLiveDummy() {
if (chatItems.lastOrNull()?.id == ChatItem.TEMP_LIVE_CHAT_ITEM_ID) {
if (chatItems.value.lastOrNull()?.id == ChatItem.TEMP_LIVE_CHAT_ITEM_ID) {
chatItems.removeLast()
}
}
@ -437,14 +434,14 @@ object ChatModel {
var markedRead = 0
if (chatId.value == cInfo.id) {
var i = 0
Log.d(TAG, "TODOCHAT: markItemsReadInCurrentChat: marking read ${cInfo.id}, current chatId ${chatId.value}, size was ${chatItems.size}")
while (i < chatItems.count()) {
val item = chatItems[i]
val items = chatItems.value
while (i < items.size) {
val item = items[i]
if (item.meta.itemStatus is CIStatus.RcvNew && (range == null || (range.from <= item.id && item.id <= range.to))) {
val newItem = item.withStatus(CIStatus.RcvRead())
chatItems[i] = newItem
items[i] = newItem
if (newItem.meta.itemLive != true && newItem.meta.itemTimed?.ttl != null) {
chatItems[i] = newItem.copy(meta = newItem.meta.copy(itemTimed = newItem.meta.itemTimed.copy(
items[i] = newItem.copy(meta = newItem.meta.copy(itemTimed = newItem.meta.itemTimed.copy(
deleteAt = Clock.System.now() + newItem.meta.itemTimed.ttl.toDuration(DurationUnit.SECONDS)))
)
}
@ -452,7 +449,6 @@ object ChatModel {
}
i += 1
}
Log.d(TAG, "TODOCHAT: markItemsReadInCurrentChat: marked read ${cInfo.id}, current chatId ${chatId.value}, size now ${chatItems.size}")
}
return markedRead
}
@ -2006,6 +2002,46 @@ data class ChatItem (
}
}
fun MutableState<SnapshotStateList<ChatItem>>.add(index: Int, chatItem: ChatItem) {
value = SnapshotStateList<ChatItem>().apply { addAll(value); add(index, chatItem) }
}
fun MutableState<SnapshotStateList<ChatItem>>.add(chatItem: ChatItem) {
value = SnapshotStateList<ChatItem>().apply { addAll(value); add(chatItem) }
}
fun MutableState<SnapshotStateList<ChatItem>>.addAll(index: Int, chatItems: List<ChatItem>) {
value = SnapshotStateList<ChatItem>().apply { addAll(value); addAll(index, chatItems) }
}
fun MutableState<SnapshotStateList<ChatItem>>.addAll(chatItems: List<ChatItem>) {
value = SnapshotStateList<ChatItem>().apply { addAll(value); addAll(chatItems) }
}
fun MutableState<SnapshotStateList<ChatItem>>.removeAll(block: (ChatItem) -> Boolean) {
value = SnapshotStateList<ChatItem>().apply { addAll(value); removeAll(block) }
}
fun MutableState<SnapshotStateList<ChatItem>>.removeAt(index: Int) {
value = SnapshotStateList<ChatItem>().apply { addAll(value); removeAt(index) }
}
fun MutableState<SnapshotStateList<ChatItem>>.removeLast() {
value = SnapshotStateList<ChatItem>().apply { addAll(value); removeLast() }
}
fun MutableState<SnapshotStateList<ChatItem>>.replaceAll(chatItems: List<ChatItem>) {
value = SnapshotStateList<ChatItem>().apply { addAll(chatItems) }
}
fun MutableState<SnapshotStateList<ChatItem>>.clear() {
value = SnapshotStateList<ChatItem>()
}
fun State<SnapshotStateList<ChatItem>>.asReversed(): MutableList<ChatItem> = value.asReversed()
val State<List<ChatItem>>.size: Int get() = value.size
enum class CIMergeCategory {
MemberConnected,
RcvGroupEvent,

42
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatView.kt
View File

@ -67,14 +67,12 @@ fun ChatView(chatId: String, chatModel: ChatModel, onComposed: suspend (chatId:
launch {
snapshotFlow { chatModel.chatId.value }
.distinctUntilChanged()
.onEach { Log.d(TAG, "TODOCHAT: chatId: activeChatId ${activeChat.value?.id} == new chatId $it ${activeChat.value?.id == it} ") }
.filterNotNull()
.collect { chatId ->
if (activeChat.value?.id != chatId) {
// Redisplay the whole hierarchy if the chat is different to make going from groups to direct chat working correctly
// Also for situation when chatId changes after clicking in notification, etc
activeChat.value = chatModel.getChat(chatId)
Log.d(TAG, "TODOCHAT: chatId: activeChatId became ${activeChat.value?.id}")
}
markUnreadChatAsRead(activeChat, chatModel)
}
@ -94,12 +92,10 @@ fun ChatView(chatId: String, chatModel: ChatModel, onComposed: suspend (chatId:
}
}
.distinctUntilChanged()
.onEach { Log.d(TAG, "TODOCHAT: chats: activeChatId ${activeChat.value?.id} == new chatId ${it?.id} ${activeChat.value?.id == it?.id} ") }
// Only changed chatInfo is important thing. Other properties can be skipped for reducing recompositions
.filter { it != null && it?.chatInfo != activeChat.value?.chatInfo }
.filter { it != null && it.chatInfo != activeChat.value?.chatInfo }
.collect {
activeChat.value = it
Log.d(TAG, "TODOCHAT: chats: activeChatId became ${activeChat.value?.id}")
}
}
}
@ -150,7 +146,6 @@ fun ChatView(chatId: String, chatModel: ChatModel, onComposed: suspend (chatId:
},
attachmentOption,
attachmentBottomSheetState,
chatModel.chatItems,
searchText,
useLinkPreviews = useLinkPreviews,
linkMode = chatModel.simplexLinkMode.value,
@ -228,19 +223,17 @@ fun ChatView(chatId: String, chatModel: ChatModel, onComposed: suspend (chatId:
loadPrevMessages = {
if (chatModel.chatId.value != activeChat.value?.id) return@ChatLayout
val c = chatModel.getChat(chatModel.chatId.value ?: return@ChatLayout)
val firstId = chatModel.chatItems.firstOrNull()?.id
val firstId = chatModel.chatItems.value.firstOrNull()?.id
if (c != null && firstId != null) {
withBGApi {
Log.d(TAG, "TODOCHAT: loadPrevMessages: loading for ${c.id}, current chatId ${ChatModel.chatId.value}, size was ${ChatModel.chatItems.size}")
apiLoadPrevMessages(c, chatModel, firstId, searchText.value)
Log.d(TAG, "TODOCHAT: loadPrevMessages: loaded for ${c.id}, current chatId ${ChatModel.chatId.value}, size now ${ChatModel.chatItems.size}")
}
}
},
deleteMessage = { itemId, mode ->
withBGApi {
val cInfo = chat.chatInfo
val toDeleteItem = chatModel.chatItems.firstOrNull { it.id == itemId }
val toDeleteItem = chatModel.chatItems.value.firstOrNull { it.id == itemId }
val toModerate = toDeleteItem?.memberToModerate(chat.chatInfo)
val groupInfo = toModerate?.first
val groupMember = toModerate?.second
@ -500,7 +493,6 @@ fun ChatLayout(
composeView: (@Composable () -> Unit),
attachmentOption: MutableState<AttachmentOption?>,
attachmentBottomSheetState: ModalBottomSheetState,
chatItems: List<ChatItem>,
searchValue: State<String>,
useLinkPreviews: Boolean,
linkMode: SimplexLinkMode,
@ -587,7 +579,7 @@ fun ChatLayout(
.padding(contentPadding)
) {
ChatItemsList(
chat, unreadCount, composeState, chatItems, searchValue,
chat, unreadCount, composeState, searchValue,
useLinkPreviews, linkMode, showMemberInfo, loadPrevMessages, deleteMessage, deleteMessages,
receiveFile, cancelFile, joinGroup, acceptCall, acceptFeature, openDirectChat,
updateContactStats, updateMemberStats, syncContactConnection, syncMemberConnection, findModelChat, findModelMember,
@ -845,7 +837,6 @@ fun BoxWithConstraintsScope.ChatItemsList(
chat: Chat,
unreadCount: State<Int>,
composeState: MutableState<ComposeState>,
chatItems: List<ChatItem>,
searchValue: State<String>,
useLinkPreviews: Boolean,
linkMode: SimplexLinkMode,
@ -874,7 +865,7 @@ fun BoxWithConstraintsScope.ChatItemsList(
) {
val listState = rememberLazyListState()
val scope = rememberCoroutineScope()
ScrollToBottom(chat.id, listState, chatItems)
ScrollToBottom(chat.id, listState, chatModel.chatItems)
var prevSearchEmptiness by rememberSaveable { mutableStateOf(searchValue.value.isEmpty()) }
// Scroll to bottom when search value changes from something to nothing and back
LaunchedEffect(searchValue.value.isEmpty()) {
@ -891,7 +882,7 @@ fun BoxWithConstraintsScope.ChatItemsList(
PreloadItems(listState, ChatPagination.UNTIL_PRELOAD_COUNT, loadPrevMessages)
Spacer(Modifier.size(8.dp))
val reversedChatItems by remember { derivedStateOf { chatItems.reversed().toList() } }
val reversedChatItems by remember { derivedStateOf { chatModel.chatItems.asReversed() } }
val maxHeightRounded = with(LocalDensity.current) { maxHeight.roundToPx() }
val scrollToItem: (Long) -> Unit = { itemId: Long ->
val index = reversedChatItems.indexOfFirst { it.id == itemId }
@ -944,7 +935,7 @@ fun BoxWithConstraintsScope.ChatItemsList(
}
}
val provider = {
providerForGallery(i, chatItems, cItem.id) { indexInReversed ->
providerForGallery(i, chatModel.chatItems.value, cItem.id) { indexInReversed ->
scope.launch {
listState.scrollToItem(
kotlin.math.min(reversedChatItems.lastIndex, indexInReversed + 1),
@ -1067,11 +1058,11 @@ fun BoxWithConstraintsScope.ChatItemsList(
}
}
}
FloatingButtons(chatItems, unreadCount, chat.chatStats.minUnreadItemId, searchValue, markRead, setFloatingButton, listState)
FloatingButtons(chatModel.chatItems, unreadCount, chat.chatStats.minUnreadItemId, searchValue, markRead, setFloatingButton, listState)
}
@Composable
private fun ScrollToBottom(chatId: ChatId, listState: LazyListState, chatItems: List<ChatItem>) {
private fun ScrollToBottom(chatId: ChatId, listState: LazyListState, chatItems: State<List<ChatItem>>) {
val scope = rememberCoroutineScope()
// Helps to scroll to bottom after moving from Group to Direct chat
// and prevents scrolling to bottom on orientation change
@ -1089,7 +1080,7 @@ private fun ScrollToBottom(chatId: ChatId, listState: LazyListState, chatItems:
* When the first visible item (from bottom) is visible (even partially) we can autoscroll to 0 item. Or just scrollBy small distance otherwise
* */
LaunchedEffect(Unit) {
snapshotFlow { chatItems.lastOrNull()?.id }
snapshotFlow { chatItems.value.lastOrNull()?.id }
.distinctUntilChanged()
.filter { listState.layoutInfo.visibleItemsInfo.firstOrNull()?.key != it }
.collect {
@ -1112,7 +1103,7 @@ private fun ScrollToBottom(chatId: ChatId, listState: LazyListState, chatItems:
@Composable
fun BoxWithConstraintsScope.FloatingButtons(
chatItems: List<ChatItem>,
chatItems: State<List<ChatItem>>,
unreadCount: State<Int>,
minUnreadItemId: Long,
searchValue: State<String>,
@ -1146,10 +1137,11 @@ fun BoxWithConstraintsScope.FloatingButtons(
val bottomUnreadCount by remember {
derivedStateOf {
if (unreadCount.value == 0) return@derivedStateOf 0
val from = chatItems.lastIndex - firstVisibleIndex - lastIndexOfVisibleItems
if (chatItems.size <= from || from < 0) return@derivedStateOf 0
val items = chatItems.value
val from = items.lastIndex - firstVisibleIndex - lastIndexOfVisibleItems
if (items.size <= from || from < 0) return@derivedStateOf 0
chatItems.subList(from, chatItems.size).count { it.isRcvNew }
items.subList(from, items.size).count { it.isRcvNew }
}
}
val firstVisibleOffset = (-with(LocalDensity.current) { maxHeight.roundToPx() } * 0.8).toInt()
@ -1195,7 +1187,7 @@ fun BoxWithConstraintsScope.FloatingButtons(
painterResource(MR.images.ic_check),
onClick = {
markRead(
CC.ItemRange(minUnreadItemId, chatItems[chatItems.size - listState.layoutInfo.visibleItemsInfo.lastIndex - 1].id - 1),
CC.ItemRange(minUnreadItemId, chatItems.value[chatItems.size - listState.layoutInfo.visibleItemsInfo.lastIndex - 1].id - 1),
bottomUnreadCount
)
showDropDown.value = false
@ -1500,7 +1492,6 @@ fun PreviewChatLayout() {
composeView = {},
attachmentOption = remember { mutableStateOf<AttachmentOption?>(null) },
attachmentBottomSheetState = rememberModalBottomSheetState(initialValue = ModalBottomSheetValue.Hidden),
chatItems = chatItems,
searchValue,
useLinkPreviews = true,
linkMode = SimplexLinkMode.DESCRIPTION,
@ -1573,7 +1564,6 @@ fun PreviewGroupChatLayout() {
composeView = {},
attachmentOption = remember { mutableStateOf<AttachmentOption?>(null) },
attachmentBottomSheetState = rememberModalBottomSheetState(initialValue = ModalBottomSheetValue.Hidden),
chatItems = chatItems,
searchValue,
useLinkPreviews = true,
linkMode = SimplexLinkMode.DESCRIPTION,

2
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt
View File

@ -665,7 +665,7 @@ fun ComposeView(
fun editPrevMessage() {
if (composeState.value.contextItem != ComposeContextItem.NoContextItem || composeState.value.preview != ComposePreview.NoPreview) return
val lastEditable = chatModel.chatItems.findLast { it.meta.editable }
val lastEditable = chatModel.chatItems.value.findLast { it.meta.editable }
if (lastEditable != null) {
composeState.value = ComposeState(editingItem = lastEditable, useLinkPreviews = useLinkPreviews)
}

5
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupMemberInfoView.kt
View File

@ -3,11 +3,9 @@ package chat.simplex.common.views.chat.group
import InfoRow
import SectionBottomSpacer
import SectionDividerSpaced
import SectionItemView
import SectionSpacer
import SectionTextFooter
import SectionView
import TextIconSpaced
import androidx.compose.desktop.ui.tooling.preview.Preview
import java.net.URI
import androidx.compose.foundation.*
@ -74,9 +72,8 @@ fun GroupMemberInfoView(
if (chatModel.getContactChat(it) == null) {
chatModel.addChat(c)
}
chatModel.chatItems.clear()
chatModel.chatItemStatuses.clear()
chatModel.chatItems.addAll(c.chatItems)
chatModel.chatItems.replaceAll(c.chatItems)
chatModel.chatId.value = c.id
closeAll()
}

3
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/item/ChatItemView.kt
View File

@ -527,8 +527,9 @@ fun DeleteItemAction(
val range = chatViewItemsRange(currIndex, prevHidden)
if (range != null) {
val itemIds: ArrayList<Long> = arrayListOf()
val reversedChatItems = chatModel.chatItems.asReversed()
for (i in range) {
itemIds.add(chatModel.chatItems.asReversed()[i].id)
itemIds.add(reversedChatItems[i].id)
}
deleteMessagesAlertDialog(itemIds, generalGetString(MR.strings.delete_message_mark_deleted_warning), deleteMessages = deleteMessages)
} else {

8
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt
View File

@ -212,18 +212,15 @@ suspend fun openGroupChat(rhId: Long?, groupId: Long, chatModel: ChatModel) {
}
suspend fun openChat(rhId: Long?, chatInfo: ChatInfo, chatModel: ChatModel) {
Log.d(TAG, "TODOCHAT: openChat: opening ${chatInfo.id}, current chatId ${ChatModel.chatId.value}, size ${ChatModel.chatItems.size}")
val chat = chatModel.controller.apiGetChat(rhId, chatInfo.chatType, chatInfo.apiId)
if (chat != null) {
openLoadedChat(chat, chatModel)
Log.d(TAG, "TODOCHAT: openChat: opened ${chatInfo.id}, current chatId ${ChatModel.chatId.value}, size ${ChatModel.chatItems.size}")
}
}
fun openLoadedChat(chat: Chat, chatModel: ChatModel) {
chatModel.chatItems.clear()
chatModel.chatItemStatuses.clear()
chatModel.chatItems.addAll(chat.chatItems)
chatModel.chatItems.replaceAll(chat.chatItems)
chatModel.chatId.value = chat.chatInfo.id
}
@ -239,8 +236,7 @@ suspend fun apiFindMessages(ch: Chat, chatModel: ChatModel, search: String) {
val chatInfo = ch.chatInfo
val chat = chatModel.controller.apiGetChat(ch.remoteHostId, chatInfo.chatType, chatInfo.apiId, search = search) ?: return
if (chatModel.chatId.value != chat.id) return
chatModel.chatItems.clear()
chatModel.chatItems.addAll(0, chat.chatItems)
chatModel.chatItems.replaceAll(chat.chatItems)
}
suspend fun setGroupMembers(rhId: Long?, groupInfo: GroupInfo, chatModel: ChatModel) {

3
apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/DesktopApp.kt
View File

@ -14,8 +14,7 @@ import androidx.compose.ui.input.key.*
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.*
import chat.simplex.common.model.ChatController
import chat.simplex.common.model.ChatModel
import chat.simplex.common.model.*
import chat.simplex.common.platform.*
import chat.simplex.common.ui.theme.DEFAULT_START_MODAL_WIDTH
import chat.simplex.common.ui.theme.SimpleXTheme