diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Channel.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Channel.kt index af328640c4..0023d5ad5c 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Channel.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Channel.kt @@ -21,7 +21,9 @@ package com.vitorpamplona.amethyst.commons.model import androidx.compose.runtime.Stable +import com.vitorpamplona.amethyst.commons.util.KmpLock import com.vitorpamplona.amethyst.commons.util.WeakReference +import com.vitorpamplona.amethyst.commons.util.withLock import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.utils.cache.LargeCache @@ -41,6 +43,10 @@ abstract class Channel : NotesGatherer { private var relays = mapOf() + // Single per-instance lock that previously @Synchronized methods share. + // Replaces JVM-only @Synchronized so this class compiles on iOS. + private val syncLock = KmpLock() + private var changesFlow: WeakReference>>? = null fun changesFlow(): MutableSharedFlow> { @@ -75,12 +81,12 @@ abstract class Channel : NotesGatherer { flowSet?.metadata?.invalidateData() } - @Synchronized - fun addRelaySync(briefInfo: NormalizedRelayUrl) { - if (briefInfo !in relays) { - relays = relays + Pair(briefInfo, Counter(1)) + fun addRelaySync(briefInfo: NormalizedRelayUrl) = + syncLock.withLock { + if (briefInfo !in relays) { + relays = relays + Pair(briefInfo, Counter(1)) + } } - } fun addRelay(relay: NormalizedRelayUrl) { val counter = relays[relay] @@ -165,18 +171,18 @@ abstract class Channel : NotesGatherer { var flowSet: ChannelFlowSet? = null - @Synchronized - fun createOrDestroyFlowSync(create: Boolean) { - if (create) { - if (flowSet == null) { - flowSet = ChannelFlowSet(this) - } - } else { - if (flowSet != null && flowSet?.isInUse() == false) { - flowSet = null + fun createOrDestroyFlowSync(create: Boolean) = + syncLock.withLock { + if (create) { + if (flowSet == null) { + flowSet = ChannelFlowSet(this) + } + } else { + if (flowSet != null && flowSet?.isInUse() == false) { + flowSet = null + } } } - } fun flow(): ChannelFlowSet { if (flowSet == null) { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index d250ae7d2a..5e604f97a7 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -24,9 +24,11 @@ import androidx.compose.runtime.Immutable import androidx.compose.runtime.Stable import com.vitorpamplona.amethyst.commons.model.nip88Polls.PollResponsesCache import com.vitorpamplona.amethyst.commons.threading.checkNotInMainThread +import com.vitorpamplona.amethyst.commons.util.KmpLock import com.vitorpamplona.amethyst.commons.util.firstFullCharOrEmoji import com.vitorpamplona.amethyst.commons.util.replace import com.vitorpamplona.amethyst.commons.util.toShortDisplay +import com.vitorpamplona.amethyst.commons.util.withLock import com.vitorpamplona.quartz.experimental.bounties.addedRewardValue import com.vitorpamplona.quartz.experimental.bounties.hasAdditionalReward import com.vitorpamplona.quartz.experimental.ephemChat.chat.EphemeralChatEvent @@ -73,6 +75,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.flatMapLatest import java.math.BigDecimal +import kotlin.concurrent.Volatile interface NotesGatherer { fun removeNote(note: Note) @@ -105,6 +108,11 @@ class AddressableNote( open class Note( val idHex: HexKey, ) : NotesGatherer { + // Per-instance lock shared by the previously @Synchronized methods (zap / + // onchain-zap / zap-payment / relay-add / flowSet lifecycle). Replaces the + // JVM-only @Synchronized annotation so this class compiles on iOS. + private val syncLock = KmpLock() + // These fields are only available after the Text Note event is received. // They are immutable after that. var event: Event? = null @@ -165,7 +173,7 @@ open class Note( * only counts CONFIRMED amounts. * * `@Volatile` ensures cross-thread visibility: writes happen on `applicationIOScope` - * (inside the @Synchronized inner methods) and reads happen on the Compose Main + * (inside the syncLock-guarded inner methods) and reads happen on the Compose Main * thread (gallery recomposition + the reverification driver's `any { … }` check). */ @Volatile @@ -421,19 +429,18 @@ open class Note( } } - @Synchronized private fun innerAddZap( zapRequest: Note, zap: Note?, - ): Boolean { - if (zaps[zapRequest] == null) { - zaps = zaps + Pair(zapRequest, zap) - return true + ): Boolean = + syncLock.withLock { + if (zaps[zapRequest] == null) { + zaps = zaps + Pair(zapRequest, zap) + return@withLock true + } + return@withLock false } - return false - } - fun addZap( zapRequest: Note, zap: Note?, @@ -447,48 +454,48 @@ open class Note( } } - @Synchronized private fun innerAddOnchainZap( txid: String, entry: OnchainZapEntry, - ): Boolean { - val existing = onchainZaps[txid] - if (existing != null) { - // Exact structural duplicate (same source Note + same fields) — typical - // relay echo of the same event. Skip the rewrite to avoid spurious - // flowSet invalidation. - if (entry == existing) return false - // Reject downgrades using the explicit OnchainZapStatus.level (not ordinal) - // so the upgrade contract survives future enum reordering or insertions. - if (entry.status.level < existing.status.level) return false - // Same level: accept only when verifiedSats grows OR the source differs - // (legitimate alternate signer republishing a split-zap receipt). A strictly - // smaller verifiedSats is a backend downgrade and we ignore it. - if (entry.status.level == existing.status.level && entry.verifiedSats < existing.verifiedSats) return false + ): Boolean = + syncLock.withLock { + val existing = onchainZaps[txid] + if (existing != null) { + // Exact structural duplicate (same source Note + same fields) — typical + // relay echo of the same event. Skip the rewrite to avoid spurious + // flowSet invalidation. + if (entry == existing) return@withLock false + // Reject downgrades using the explicit OnchainZapStatus.level (not ordinal) + // so the upgrade contract survives future enum reordering or insertions. + if (entry.status.level < existing.status.level) return@withLock false + // Same level: accept only when verifiedSats grows OR the source differs + // (legitimate alternate signer republishing a split-zap receipt). A strictly + // smaller verifiedSats is a backend downgrade and we ignore it. + if (entry.status.level == existing.status.level && entry.verifiedSats < existing.verifiedSats) return@withLock false + } + onchainZaps = onchainZaps + Pair(txid, entry) + return@withLock true } - onchainZaps = onchainZaps + Pair(txid, entry) - return true - } - @Synchronized private fun innerRemoveOnchainZapForSource( txid: String, sourceAuthorPubKey: HexKey, - ): Boolean { - val existing = onchainZaps[txid] ?: return false - // Anti-spoof: only remove the entry if its source matches the rejecting event. - // Otherwise a malicious third party could erase a legitimate CONFIRMED entry - // by publishing a spoofed kind:8333 with the same txid but a bystander recipient. - // Also refuse to remove a CONFIRMED entry — once chain-verified, only a fresh - // CONFIRMED replacement should change it; a transient backend hiccup must not - // wipe a previously-CONFIRMED entry just because some other target on the same - // event is still UNVERIFIED. - if (existing.status == OnchainZapStatus.CONFIRMED) return false - if (existing.source.author?.pubkeyHex == null) return false - if (existing.source.author?.pubkeyHex != sourceAuthorPubKey) return false - onchainZaps = onchainZaps - txid - return true - } + ): Boolean = + syncLock.withLock { + val existing = onchainZaps[txid] ?: return@withLock false + // Anti-spoof: only remove the entry if its source matches the rejecting event. + // Otherwise a malicious third party could erase a legitimate CONFIRMED entry + // by publishing a spoofed kind:8333 with the same txid but a bystander recipient. + // Also refuse to remove a CONFIRMED entry — once chain-verified, only a fresh + // CONFIRMED replacement should change it; a transient backend hiccup must not + // wipe a previously-CONFIRMED entry just because some other target on the same + // event is still UNVERIFIED. + if (existing.status == OnchainZapStatus.CONFIRMED) return@withLock false + if (existing.source.author?.pubkeyHex == null) return@withLock false + if (existing.source.author?.pubkeyHex != sourceAuthorPubKey) return@withLock false + onchainZaps = onchainZaps - txid + return@withLock true + } /** * Register a NIP-BC onchain zap targeting this note. `source` is the OnchainZapEvent's own @@ -535,19 +542,18 @@ open class Note( } } - @Synchronized private fun innerAddZapPayment( zapPaymentRequest: Note, zapPayment: Note?, - ): Boolean { - if (zapPayments[zapPaymentRequest] == null) { - zapPayments = zapPayments + Pair(zapPaymentRequest, zapPayment) - return true + ): Boolean = + syncLock.withLock { + if (zapPayments[zapPaymentRequest] == null) { + zapPayments = zapPayments + Pair(zapPaymentRequest, zapPayment) + return@withLock true + } + return@withLock false } - return false - } - fun addZapPayment( zapPaymentRequest: Note, zapPayment: Note?, @@ -589,12 +595,12 @@ open class Note( } } - @Synchronized - fun addRelaySync(relay: NormalizedRelayUrl) { - if (relay !in relays) { - relays = relays + relay + fun addRelaySync(relay: NormalizedRelayUrl) = + syncLock.withLock { + if (relay !in relays) { + relays = relays + relay + } } - } fun hasRelay(relay: NormalizedRelayUrl) = relay in relays @@ -1035,18 +1041,18 @@ open class Note( var flowSet: NoteFlowSet? = null - @Synchronized - fun createOrDestroyFlowSync(create: Boolean) { - if (create) { - if (flowSet == null) { - flowSet = NoteFlowSet(this) - } - } else { - if (flowSet != null && flowSet?.isInUse() == false) { - flowSet = null + fun createOrDestroyFlowSync(create: Boolean) = + syncLock.withLock { + if (create) { + if (flowSet == null) { + flowSet = NoteFlowSet(this) + } + } else { + if (flowSet != null && flowSet?.isInUse() == false) { + flowSet = null + } } } - } fun flow(): NoteFlowSet { if (flowSet == null) { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupChatroom.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupChatroom.kt index c5befd527b..92b63c5b89 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupChatroom.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupChatroom.kt @@ -26,7 +26,9 @@ import com.vitorpamplona.amethyst.commons.model.Channel.Companion.DefaultFeedOrd import com.vitorpamplona.amethyst.commons.model.ListChange import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.model.NotesGatherer +import com.vitorpamplona.amethyst.commons.util.KmpLock import com.vitorpamplona.amethyst.commons.util.WeakReference +import com.vitorpamplona.amethyst.commons.util.withLock import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import kotlinx.coroutines.channels.BufferOverflow @@ -84,22 +86,25 @@ class MarmotGroupChatroom( return adminPubkeys.value.any { it in followingKeySet } } + // Per-instance lock shared by previously @Synchronized methods. + private val syncLock = KmpLock() + // Synthetic note used by list views to represent the group when no // messages have been received yet. Lazily created and kept stable so // equality-based feed diffing treats it as the same row across refreshes. private var cachedPlaceholder: Note? = null - @Synchronized - fun placeholderNote(): Note { - val existing = cachedPlaceholder - if (existing != null) return existing - val created = - Note(placeholderIdHex(nostrGroupId)).apply { - addGatherer(this@MarmotGroupChatroom) - } - cachedPlaceholder = created - return created - } + fun placeholderNote(): Note = + syncLock.withLock { + val existing = cachedPlaceholder + if (existing != null) return@withLock existing + val created = + Note(placeholderIdHex(nostrGroupId)).apply { + addGatherer(this@MarmotGroupChatroom) + } + cachedPlaceholder = created + created + } companion object { fun placeholderIdHex(nostrGroupId: HexKey): HexKey = "marmot-empty-$nostrGroupId" @@ -119,23 +124,23 @@ class MarmotGroupChatroom( removeMessageSync(note) } - @Synchronized - fun addMessageSync(msg: Note): Boolean { - if (msg !in messages) { - messages = messages + msg - msg.addGatherer(this) + fun addMessageSync(msg: Note): Boolean = + syncLock.withLock { + if (msg !in messages) { + messages = messages + msg + msg.addGatherer(this) - val createdAt = msg.createdAt() ?: 0L - if (createdAt > (newestMessage?.createdAt() ?: 0L)) { - newestMessage = msg + val createdAt = msg.createdAt() ?: 0L + if (createdAt > (newestMessage?.createdAt() ?: 0L)) { + newestMessage = msg + } + + unreadCount.value += 1 + changesFlow?.get()?.tryEmit(ListChange.Addition(msg)) + return@withLock true } - - unreadCount.value += 1 - changesFlow?.get()?.tryEmit(ListChange.Addition(msg)) - return true + return@withLock false } - return false - } /** * Add a message that is being restored from persistent storage on app @@ -143,38 +148,38 @@ class MarmotGroupChatroom( * count — restored messages were already seen by the user in a previous * session. */ - @Synchronized - fun restoreMessageSync(msg: Note): Boolean { - if (msg !in messages) { - messages = messages + msg - msg.addGatherer(this) + fun restoreMessageSync(msg: Note): Boolean = + syncLock.withLock { + if (msg !in messages) { + messages = messages + msg + msg.addGatherer(this) - val createdAt = msg.createdAt() ?: 0L - if (createdAt > (newestMessage?.createdAt() ?: 0L)) { - newestMessage = msg + val createdAt = msg.createdAt() ?: 0L + if (createdAt > (newestMessage?.createdAt() ?: 0L)) { + newestMessage = msg + } + + changesFlow?.get()?.tryEmit(ListChange.Addition(msg)) + return@withLock true } - - changesFlow?.get()?.tryEmit(ListChange.Addition(msg)) - return true + return@withLock false } - return false - } - @Synchronized - fun removeMessageSync(msg: Note): Boolean { - if (msg in messages) { - messages = messages - msg - msg.removeGatherer(this) + fun removeMessageSync(msg: Note): Boolean = + syncLock.withLock { + if (msg in messages) { + messages = messages - msg + msg.removeGatherer(this) - if (msg == newestMessage) { - newestMessage = messages.maxByOrNull { it.createdAt() ?: 0L } + if (msg == newestMessage) { + newestMessage = messages.maxByOrNull { it.createdAt() ?: 0L } + } + + changesFlow?.get()?.tryEmit(ListChange.Deletion(msg)) + return@withLock true } - - changesFlow?.get()?.tryEmit(ListChange.Deletion(msg)) - return true + return@withLock false } - return false - } fun markAsRead() { unreadCount.value = 0 @@ -209,14 +214,14 @@ class MarmotGroupChatroom( * decrypted inner notes become eligible for GC out of LocalCache (which * holds them weakly). */ - @Synchronized - fun clearAllMessagesSync(): Set { - val toRemove = messages - if (toRemove.isEmpty()) return toRemove - messages = emptySet() - newestMessage = null - unreadCount.value = 0 - changesFlow?.get()?.tryEmit(ListChange.SetDeletion(toRemove)) - return toRemove - } + fun clearAllMessagesSync(): Set = + syncLock.withLock { + val toRemove = messages + if (toRemove.isEmpty()) return@withLock toRemove + messages = emptySet() + newestMessage = null + unreadCount.value = 0 + changesFlow?.get()?.tryEmit(ListChange.SetDeletion(toRemove)) + toRemove + } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/privateChats/Chatroom.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/privateChats/Chatroom.kt index f4476777f2..43678ca38a 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/privateChats/Chatroom.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/privateChats/Chatroom.kt @@ -26,7 +26,9 @@ import com.vitorpamplona.amethyst.commons.model.ListChange import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.model.NotesGatherer import com.vitorpamplona.amethyst.commons.model.User +import com.vitorpamplona.amethyst.commons.util.KmpLock import com.vitorpamplona.amethyst.commons.util.WeakReference +import com.vitorpamplona.amethyst.commons.util.withLock import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip04Dm.messages.PrivateDmEvent import com.vitorpamplona.quartz.nip14Subject.subject @@ -44,6 +46,9 @@ class Chatroom : NotesGatherer { var ownerSentMessage: Boolean = false var newestMessage: Note? = null + // Per-instance lock shared by previously @Synchronized methods. + private val syncLock = KmpLock() + private var changesFlow: WeakReference>>? = null fun changesFlow(): MutableSharedFlow> { @@ -58,68 +63,68 @@ class Chatroom : NotesGatherer { removeMessageSync(note) } - @Synchronized - fun addMessageSync(msg: Note): Boolean { - if (msg !in messages) { - messages = messages + msg - msg.addGatherer(this) + fun addMessageSync(msg: Note): Boolean = + syncLock.withLock { + if (msg !in messages) { + messages = messages + msg + msg.addGatherer(this) - msg.author?.let { author -> - if (author !in activeSenders) { - activeSenders + author - } - } - - val createdAt = msg.createdAt() ?: 0L - if (createdAt > (newestMessage?.createdAt() ?: 0L)) { - newestMessage = msg - } - - val newSubject = msg.event?.subject() - - if (newSubject != null && (msg.createdAt() ?: 0L) > (subjectCreatedAt ?: 0)) { - subject.tryEmit(newSubject) - subjectCreatedAt = msg.createdAt() - } - - changesFlow?.get()?.tryEmit(ListChange.Addition(msg)) - - return true - } - return false - } - - @Synchronized - fun removeMessageSync(msg: Note): Boolean { - if (msg in messages) { - messages = messages - msg - msg.removeGatherer(this) - - if (msg == newestMessage) { - newestMessage = messages.maxByOrNull { it.createdAt() ?: 0L } - } - - if (msg.event?.subject() == subject.value) { - messages - .maxByOrNull { - val noteEvent = it.event - if (noteEvent?.subject() != null) { - noteEvent.createdAt - } else { - 0 - } - }?.let { - subject.tryEmit(it.event?.subject()) - subjectCreatedAt = it.createdAt() + msg.author?.let { author -> + if (author !in activeSenders) { + activeSenders + author } + } + + val createdAt = msg.createdAt() ?: 0L + if (createdAt > (newestMessage?.createdAt() ?: 0L)) { + newestMessage = msg + } + + val newSubject = msg.event?.subject() + + if (newSubject != null && (msg.createdAt() ?: 0L) > (subjectCreatedAt ?: 0)) { + subject.tryEmit(newSubject) + subjectCreatedAt = msg.createdAt() + } + + changesFlow?.get()?.tryEmit(ListChange.Addition(msg)) + + return@withLock true } - - changesFlow?.get()?.tryEmit(ListChange.Deletion(msg)) - - return true + return@withLock false + } + + fun removeMessageSync(msg: Note): Boolean = + syncLock.withLock { + if (msg in messages) { + messages = messages - msg + msg.removeGatherer(this) + + if (msg == newestMessage) { + newestMessage = messages.maxByOrNull { it.createdAt() ?: 0L } + } + + if (msg.event?.subject() == subject.value) { + messages + .maxByOrNull { + val noteEvent = it.event + if (noteEvent?.subject() != null) { + noteEvent.createdAt + } else { + 0 + } + }?.let { + subject.tryEmit(it.event?.subject()) + subjectCreatedAt = it.createdAt() + } + } + + changesFlow?.get()?.tryEmit(ListChange.Deletion(msg)) + + return@withLock true + } + return@withLock false } - return false - } fun senderIntersects(keySet: Set): Boolean = activeSenders.any { it.pubkeyHex in keySet }