dandri/simplex-chat
1
0
Fork 0
mirror of https://github.com/simplex-chat/simplex-chat.git synced 2026-09-16 06:12:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

core, ui: fix races and alert-dismissal bugs in profile-for-invitation flow

Browse Source 
- widen changingActiveUserMutex to cover currentUser/listUsers/users list
  update atomically in changeActiveUser_
- avoid IndexOutOfBoundsException risk in the stale-core resync branch by
  replacing indexed users-list mutation with map/clear/addAll
- make the resync retry cancellation-transparent and retry once on failure
- add a 60s watchdog so creatingProfileForInvitation can't get stuck true
- guard chatModel.showingInvitation updates against stale/cross-invitation
  overwrites via a previousConnId check
- verify the switch actually landed (userId + activeUser flag) rather than
  trusting an unthrown exception
- iOS: guard the retry-alert watchdog's dismiss so it never cascades onto
  an unrelated alert stacked on top of it
- iOS: consolidate ContextProfilePickerView's two competing sheet booleans
  into one Identifiable enum driving a single .sheet(item:)
This commit is contained in:
Narasimha-sc
2026-08-29 12:08:48 +00:00
parent 6756bc79b4
commit 833e888b7f
10 changed files with 181 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/ios/Shared/Model/SimpleXAPI.swift
+31 -4
View File
@@ -129,16 +129,42 @@ func chatApiSendCmdWithRetry<R: ChatAPIResult>(_ cmd: ChatCommand, bgTask: Bool
if inProgress == nil || inProgress?.boxedValue == true,
case let .error(e) = r, let alert = retryableNetworkErrorAlert(e) {
return await withCheckedContinuation { cont in
let resumeLock = NSLock()
var resumed = false
var watchdog: DispatchWorkItem?
var presentedAlert: UIAlertController?
func resumeOnce(_ result: APIResult<R>?) {
resumeLock.lock()
let alreadyResumed = resumed
resumed = true
resumeLock.unlock()
if !alreadyResumed {
watchdog?.cancel()
cont.resume(returning: result)
}
}
showRetryAlert(
alert,
onCancel: { _ in
cont.resume(returning: nil)
resumeOnce(nil)
},
onRetry: {
watchdog?.cancel()
let r1: APIResult<R>? = await chatApiSendCmdWithRetry(cmd, bgTask: bgTask, bgDelay: bgDelay, inProgress: inProgress, retryNum: retryNum + 1)
cont.resume(returning: r1)
resumeOnce(r1)
},
onPresented: { alert in
presentedAlert = alert
}
)
let work = DispatchWorkItem {
resumeOnce(nil)
if let a = presentedAlert, a.presentedViewController == nil {
a.dismiss(animated: true)
}
}
watchdog = work
DispatchQueue.main.asyncAfter(deadline: .now() + 60, execute: work)
}
} else {
return r
@@ -146,9 +172,9 @@ func chatApiSendCmdWithRetry<R: ChatAPIResult>(_ cmd: ChatCommand, bgTask: Bool
}
@inline(__always)
func showRetryAlert(_ alert: (title: String, message: String), onCancel: @escaping (UIAlertAction) -> Void, onRetry: @escaping () async -> Void) {
func showRetryAlert(_ alert: (title: String, message: String), onCancel: @escaping (UIAlertAction) -> Void, onRetry: @escaping () async -> Void, onPresented: ((UIAlertController) -> Void)? = nil) {
DispatchQueue.main.async {
showAlert(
let presented = showAlertReturningController(
alert.title,
message: alert.message,
actions: {[
@@ -164,6 +190,7 @@ func showRetryAlert(_ alert: (title: String, message: String), onCancel: @escapi
)
]}
)
if let presented { onPresented?(presented) }
}
}
apps/ios/Shared/Views/Chat/ComposeMessage/ComposeView.swift
+1
View File
@@ -393,6 +393,7 @@ struct ComposeView: View {
chat: chat,
selectedUser: user
)
.id(chat.id)
Divider()
}
apps/ios/Shared/Views/Chat/ComposeMessage/ContextProfilePickerView.swift
+29 -19
View File
@@ -12,6 +12,12 @@ import SimpleXChat
let USER_ROW_SIZE: CGFloat = 60
let MAX_VISIBLE_USER_ROWS: CGFloat = 4.8
private enum ContextProfilePickerSheet: Identifiable, Hashable {
case incognitoHelp
case addProfile
var id: Self { self }
}
struct ContextProfilePickerView: View {
@ObservedObject var chat: Chat
@EnvironmentObject var chatModel: ChatModel
@@ -20,8 +26,7 @@ struct ContextProfilePickerView: View {
@State private var users: [User] = []
@State private var listExpanded = false
@State private var expandedListReady = false
@State private var showIncognitoSheet = false
@State private var showAddProfile = false
@State private var activeSheet: ContextProfilePickerSheet? = nil
@State private var creatingProfile = false
@State private var changingProfile = false
@@ -34,9 +39,6 @@ struct ContextProfilePickerView: View {
.map { $0.user }
.filter { u in u.activeUser || !u.hidden }
}
.sheet(isPresented: $showIncognitoSheet) {
IncognitoHelp()
}
}
private func viewBody() -> some View {
@@ -47,15 +49,18 @@ struct ContextProfilePickerView: View {
profilePicker()
}
}
// On the Group: the row and profilePicker() are both disposed while this is
// presented. Stacking sheets is supported from iOS 14.5; the target is 15.
.sheet(isPresented: $showAddProfile) {
NavigationView {
CreateProfile(onSubmit: { displayName, shortDescr, image in
try await createProfileForChat(displayName, shortDescr, image)
}, submitting: creatingProfile)
.sheet(item: $activeSheet) { sheet in
switch sheet {
case .incognitoHelp:
IncognitoHelp()
case .addProfile:
NavigationView {
CreateProfile(onSubmit: { displayName, shortDescr, image in
try await createProfileForChat(displayName, shortDescr, image)
}, submitting: creatingProfile)
}
.interactiveDismissDisabled(creatingProfile)
}
.interactiveDismissDisabled(creatingProfile)
}
}
@@ -224,7 +229,7 @@ struct ContextProfilePickerView: View {
if chat.chatInfo.profileChangeProhibited {
showCantChangeProfileAlert()
} else {
showAddProfile = true
activeSheet = .addProfile
}
} label: {
HStack {
@@ -274,7 +279,7 @@ struct ContextProfilePickerView: View {
}
await MainActor.run {
// Unconditional: the switch only removes this view when it succeeded
showAddProfile = false
activeSheet = nil
// Registered here rather than trusting the resync: changeActiveUserAsync_
// calls apiSetActiveUserAsync and listUsersAsync before the MainActor.run
// that writes m.users, so a throw leaves both lists without the profile and
@@ -312,8 +317,8 @@ struct ContextProfilePickerView: View {
// and a notification action can have switched it while we were creating. After the
// await above, not before it: checked first, that await reopens the very window
// this closes. Kotlin orders it the same way.
guard await MainActor.run({ chatModel.currentUser?.userId }) == ownerUserId else {
await MainActor.run { showAddProfile = false }
guard await MainActor.run(body: { chatModel.currentUser?.userId }) == ownerUserId else {
await MainActor.run { activeSheet = nil }
alertAfterDismissal(NSLocalizedString("Error changing chat profile", comment: "alert title"))
return
}
@@ -324,7 +329,7 @@ struct ContextProfilePickerView: View {
users = updatedUsers.map { $0.user }.filter { u in u.activeUser || !u.hidden }
}
// changingProfile here too: the defer clears creatingProfile as soon as this returns
showAddProfile = false
activeSheet = nil
changingProfile = true
}
changeProfile(newUser, dismissingSheet: true)
@@ -366,6 +371,11 @@ struct ContextProfilePickerView: View {
}
do {
try await changeActiveUserAsync_(newUser.userId, viewPwd: nil, keepingChatId: chat.id)
await MainActor.run {
if let chatId = chatModel.chatId, chatModel.getChat(chatId) == nil {
chatModel.chatId = nil
}
}
} catch {
report(
NSLocalizedString("Error switching profile", comment: "alert title"),
@@ -411,7 +421,7 @@ struct ContextProfilePickerView: View {
.font(.system(size: 16))
.foregroundColor(theme.colors.primary)
.onTapGesture {
showIncognitoSheet = true
activeSheet = .incognitoHelp
}
Spacer()
apps/ios/Shared/Views/Helpers/ShareSheet.swift
+14 -5
View File
@@ -63,11 +63,20 @@ func showAlert(
message: String? = nil,
actions: () -> [UIAlertAction] = { [okAlertAction] }
) {
if let topController = getTopViewController() {
let alert = UIAlertController(title: title, message: message, preferredStyle: .alert)
for action in actions() { alert.addAction(action) }
topController.present(alert, animated: true)
}
showAlertReturningController(title, message: message, actions: actions)
}
@discardableResult
func showAlertReturningController(
_ title: String,
message: String? = nil,
actions: () -> [UIAlertAction] = { [okAlertAction] }
) -> UIAlertController? {
guard let topController = getTopViewController() else { return nil }
let alert = UIAlertController(title: title, message: message, preferredStyle: .alert)
for action in actions() { alert.addAction(action) }
topController.present(alert, animated: true)
return alert
}
/// getTopViewController() keeps returning a sheet until its dismissal transition ends, so
apps/ios/Shared/Views/NewChat/NewChatView.swift
+3 -1
View File
@@ -474,11 +474,13 @@ private struct ActiveProfilePicker: View {
do {
try await changeActiveUserAsync_(profile.userId, viewPwd: profile.hidden ? trimmedSearchTextOrPassword : nil)
await MainActor.run {
chatModel.showingInvitation = ShowingInvitation(pcc: conn, connChatUsed: false)
profileSwitchStatus = .idle
dismiss()
}
} catch {
await MainActor.run {
chatModel.showingInvitation = ShowingInvitation(pcc: conn, connChatUsed: false)
profileSwitchStatus = .idle
alert = SomeAlert(
alert: Alert(
@@ -665,7 +667,7 @@ private struct ActiveProfilePicker: View {
// selectedProfile handler runs, and a notification action can have switched it.
// After the await above, not before it: checked first, that await reopens the very
// window this closes. Kotlin orders it the same way.
guard await MainActor.run({ chatModel.currentUser?.userId }) == ownerUserId else {
guard await MainActor.run(body: { chatModel.currentUser?.userId }) == ownerUserId else {
await MainActor.run { showAddProfile = false }
alertAfterDismissal(NSLocalizedString("Error changing chat profile", comment: "alert title"))
return
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt
+3 -3
View File
@@ -654,14 +654,14 @@ object ChatController {
val currentUser = changingActiveUserMutex.withLock {
(if (toUserId != null) apiSetActiveUser(rhId, toUserId, viewPwd) else apiGetActiveUser(rhId)).also {
chatModel.currentUser.value = it
val users = listUsers(rhId)
chatModel.users.clear()
chatModel.users.addAll(users)
}
}
if (prevActiveUser?.hidden == true) {
ntfManager.cancelNotificationsForUser(prevActiveUser.userId)
}
val users = listUsers(rhId)
chatModel.users.clear()
chatModel.users.addAll(users)
getUserChatData(rhId, keepingChatId = keepingChatId)
val invitation = chatModel.callInvitations.values.firstOrNull { inv -> inv.user.userId == toUserId }
if (invitation != null && currentUser != null) {
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/WelcomeView.kt
+39 -7
View File
@@ -47,9 +47,11 @@ import chat.simplex.common.views.usersettings.EditImageButton
import chat.simplex.common.views.usersettings.SettingsActionItem
import chat.simplex.res.MR
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.cancelAndJoin
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.launch
import kotlinx.coroutines.sync.withLock
import java.net.URI
const val MAX_BIO_LENGTH_BYTES = 160
@@ -386,7 +388,7 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) {
chatModel.creatingProfileForInvitation.value = true
// On Main, like the pickers' own handlers: every call here suspends into IO, and the
// chat model is updated on Main by the receiver loop.
withApi {
val job = withApi {
try {
val ownerUserId = chatModel.currentUser.value?.userId
val profile = Profile(displayName.trim(), "", shortDescr.trim().ifEmpty { null }, image = image)
@@ -398,13 +400,31 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) {
if (modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) close()
// changeActiveUser_, not changeActiveUser: the latter shows its own alert on
// failure, which would stack with the one below reporting the same thing.
runCatching { controller.changeActiveUser_(newUser.remoteHostId, newUser.userId, null) }
.onFailure { Log.e(TAG, "createProfileForInvitation: resync failed: ${it.stackTraceToString()}") }
suspend fun tryResync(): Boolean {
return try {
controller.changeActiveUser_(newUser.remoteHostId, newUser.userId, null)
true
} catch (e: CancellationException) {
throw e
} catch (e: Exception) {
Log.e(TAG, "createProfileForInvitation: resync failed: ${e.stackTraceToString()}")
false
}
}
if (!tryResync()) tryResync()
// Not relying on that resync to list it: changeActiveUser_ writes currentUser
// inside its mutex before it calls listUsers, so a host that drops between the
// two leaves the model pointing at a profile its own list does not contain.
if (chatModel.remoteHostId() == rhId && chatModel.users.none { it.user.userId == newUser.userId }) {
chatModel.users.add(UserInfo(newUser, 0))
chatModel.changingActiveUserMutex.withLock {
if (chatModel.remoteHostId() == rhId && chatModel.users.none { it.user.userId == newUser.userId }) {
val isActive = chatModel.currentUser.value?.userId == newUser.userId
if (isActive) {
val updated = chatModel.users.map { if (it.user.activeUser) it.copy(user = it.user.copy(activeUser = false)) else it }
chatModel.users.clear()
chatModel.users.addAll(updated)
}
chatModel.users.add(UserInfo(newUser.copy(activeUser = isActive), 0))
}
}
AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user))
return@withApi
@@ -417,8 +437,10 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) {
// from Main while changeActiveUser_ can be doing the same from withBGApi, and it
// has nothing to add - newUser is the API's own record and a profile created a
// moment ago has no unread messages.
if (chatModel.remoteHostId() == rhId && chatModel.users.none { it.user.userId == newUser.userId }) {
chatModel.users.add(UserInfo(newUser, 0))
chatModel.changingActiveUserMutex.withLock {
if (chatModel.remoteHostId() == rhId && chatModel.users.none { it.user.userId == newUser.userId }) {
chatModel.users.add(UserInfo(newUser, 0))
}
}
// onCreated resolves the invitation under whatever is active when it runs, and a
// notification tap or a host switch can have changed that while we were creating.
@@ -460,6 +482,16 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) {
chatModel.creatingProfileForInvitation.value = false
}
}
withApi {
delay(60_000L)
if (job.isActive) {
job.cancelAndJoin()
if (job.isCancelled) {
AlertManager.shared.hideAlert()
AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user))
}
}
}
}
}
}
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeContextProfilePickerView.kt
+24 -15
View File
@@ -23,6 +23,7 @@ import chat.simplex.common.views.helpers.*
import chat.simplex.common.views.newchat.IncognitoOptionImage
import chat.simplex.common.views.usersettings.IncognitoView
import chat.simplex.res.MR
import kotlinx.coroutines.CancellationException
import dev.icerock.moko.resources.compose.painterResource
import dev.icerock.moko.resources.compose.stringResource
@@ -106,13 +107,22 @@ fun ComposeContextProfilePickerView(
// Only switch if the chat moved, or the user ends up in another profile with the
// invitation left behind. apiChangePrepared*User reports the failure itself.
if (chatMoved) {
chatModel.controller.changeActiveUser_(
rhId = newUser.remoteHostId,
toUserId = newUser.userId,
viewPwd = null,
keepingChatId = chat.id
)
if (chatModel.currentUser.value?.userId != newUser.userId) {
val switched = try {
chatModel.controller.changeActiveUser_(
rhId = newUser.remoteHostId,
toUserId = newUser.userId,
viewPwd = null,
keepingChatId = chat.id
)
true
} catch (e: CancellationException) {
throw e
} catch (e: Exception) {
Log.e(TAG, "changeProfileTo: changeActiveUser_ failed: ${e.stackTraceToString()}")
false
}
if (!switched || chatModel.currentUser.value?.userId != newUser.userId ||
chatModel.users.none { it.user.userId == newUser.userId && it.user.activeUser }) {
AlertManager.shared.showAlertMsg(
generalGetString(MR.strings.switching_profile_error_title),
String.format(generalGetString(MR.strings.switching_profile_error_message), newUser.chatViewName)
@@ -249,10 +259,11 @@ fun ComposeContextProfilePickerView(
Modifier
.fillMaxWidth()
.sizeIn(minHeight = DEFAULT_MIN_SECTION_ITEM_HEIGHT + 8.dp)
.clickable(enabled = !busy, onClick = {
// Live state the receiver loop flips, so it can turn true after the row was laid out
.clickable(onClick = {
if (!chat.chatInfo.profileChangeProhibited) {
createProfileForInvitation(rhId) { changeProfileTo(it) }
if (!busy) {
createProfileForInvitation(rhId) { changeProfileTo(it) }
}
} else {
showCantChangeProfileAlert()
}
@@ -326,13 +337,11 @@ fun ComposeContextProfilePickerView(
ProfilePickerUserOption(user)
}
// Emitted last, so with reverseLayout it renders at the top of the expanded
// list - furthest from the compose box, with the current selection nearest.
// The divider goes under the row, not over it: reverseLayout flips the items, not the
// content of one, so emitting it first would draw a line along the list's top edge.
item {
NewProfileOption()
Divider(Modifier.padding(horizontal = DEFAULT_PADDING_HALF))
if (otherUsers.isEmpty()) {
Divider(Modifier.padding(horizontal = DEFAULT_PADDING_HALF))
}
}
}
}
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt
+7 -5
View File
@@ -1666,11 +1666,13 @@ fun ComposeView(
Column {
val currentUser = chatModel.currentUser.value
if (chat.chatInfo.nextConnectPrepared && !composeState.value.inProgress && currentUser != null) {
ComposeContextProfilePickerView(
rhId = rhId,
chat = chat,
currentUser = currentUser
)
key(chat.id) {
ComposeContextProfilePickerView(
rhId = rhId,
chat = chat,
currentUser = currentUser
)
}
}
val gInfo = (chat.chatInfo as? ChatInfo.Group)?.groupInfo
apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/NewChatView.kt
+30 -15
View File
@@ -200,7 +200,8 @@ private fun CreatingLinkProgressView() {
DefaultProgressView(stringResource(MR.strings.creating_link))
}
private fun updateShownConnection(conn: PendingContactConnection) {
private fun updateShownConnection(previousConnId: String, conn: PendingContactConnection) {
if (chatModel.showingInvitation.value?.connId != previousConnId) return
chatModel.showingInvitation.value = chatModel.showingInvitation.value?.copy(
conn = conn,
connId = conn.id,
@@ -294,6 +295,7 @@ fun ActiveProfilePicker(
showIncognito: Boolean = true
) {
val switchingProfile = remember { mutableStateOf(false) }
val currentContactConnection = remember { mutableStateOf(contactConnection) }
val incognito = remember {
chatModel.showingInvitation.value?.conn?.incognito ?: controller.appPrefs.incognito.get()
}
@@ -323,27 +325,38 @@ fun ActiveProfilePicker(
try {
var updatedConn: PendingContactConnection? = null
if (contactConnection != null) {
updatedConn = controller.apiChangeConnectionUser(rhId, contactConnection.pccConnId, user.userId)
val conn = currentContactConnection.value
if (conn != null) {
updatedConn = controller.apiChangeConnectionUser(rhId, conn.pccConnId, user.userId)
// Not moved - leave the picker open rather than stranding a profile just created
// for this invitation. This call provisions a new queue, so it is what fails offline.
if (updatedConn == null) return
currentContactConnection.value = updatedConn
withContext(Dispatchers.Main) {
chatModel.chatsContext.updateContactConnection(rhId, updatedConn)
updateShownConnection(updatedConn)
updateShownConnection(conn.id, updatedConn)
}
}
// After the move, not before: the picker now stays open on failure, and clearing the
// app-wide default there would leave it off with Incognito still ticked in the picker
appPreferences.incognito.set(false)
controller.changeActiveUser_(
rhId = user.remoteHostId,
toUserId = user.userId,
viewPwd = if (user.hidden) searchTextOrPassword.value else null
)
val switched = try {
controller.changeActiveUser_(
rhId = user.remoteHostId,
toUserId = user.userId,
viewPwd = if (user.hidden) searchTextOrPassword.value else null
)
true
} catch (e: CancellationException) {
throw e
} catch (e: Exception) {
Log.e(TAG, "selectProfileAsync: changeActiveUser_ failed: ${e.stackTraceToString()}")
false
}
if (chatModel.currentUser.value?.userId != user.userId) {
if (!switched || chatModel.currentUser.value?.userId != user.userId ||
chatModel.users.none { it.user.userId == user.userId && it.user.activeUser }) {
AlertManager.shared.showAlertMsg(generalGetString(
MR.strings.switching_profile_error_title),
String.format(generalGetString(MR.strings.switching_profile_error_message), user.chatViewName)
@@ -408,20 +421,22 @@ fun ActiveProfilePicker(
title = stringResource(MR.strings.incognito),
selected = incognito,
onSelected = {
if (incognito || busy || contactConnection == null) return@ProfilePickerOption
val conn = currentContactConnection.value
if (incognito || busy || conn == null) return@ProfilePickerOption
switchingProfile.value = true
withApi {
try {
val conn = controller.apiSetConnectionIncognito(rhId, contactConnection.pccConnId, true)
if (conn != null) {
val updatedConn = controller.apiSetConnectionIncognito(rhId, conn.pccConnId, true)
if (updatedConn != null) {
currentContactConnection.value = updatedConn
// Only once the connection is actually incognito, as on the profile path
// above: set before the call, a failure leaves the app-wide default on and
// the next connection silently uses a random profile.
appPreferences.incognito.set(true)
withContext(Dispatchers.Main) {
chatModel.chatsContext.updateContactConnection(rhId, conn)
updateShownConnection(conn)
chatModel.chatsContext.updateContactConnection(rhId, updatedConn)
updateShownConnection(conn.id, updatedConn)
}
close()
}