refactor: replace @Synchronized / @Volatile with KMP primitives

Clears the last of the JVM-only synchronization annotations from commons/commonMain so the model layer can compile on iOS. 15 methods across 4 files migrated. - @Synchronized -> KmpLock.withLock { } with one per-instance syncLock field per class. Original semantics preserved: @Synchronized on methods of the same class synchronized on `this`, and a single per-instance KmpLock gives the same exclusion. * Channel.kt: addRelaySync, createOrDestroyFlowSync * Chatroom.kt: addMessageSync, removeMessageSync * MarmotGroupChatroom.kt: placeholderNote, addMessageSync, restoreMessageSync, removeMessageSync, clearAllMessagesSync * Note.kt: innerAddZap, innerAddOnchainZap, innerRemoveOnchainZapForSource, innerAddZapPayment, addRelaySync, createOrDestroyFlowSync - Note.kt's @Volatile fields: now use kotlin.concurrent.Volatile (KMP) instead of kotlin.jvm.Volatile (JVM-only) via explicit import. Volatile semantics preserved on every target. NestViewModel.kt also uses @Volatile (and the nestsClient project dep); that file moves to jvmAndroid in a separate PR as planned (audio rooms is Phase 5).
2026-08-10 16:33:27 +00:00 · 2026-05-24 23:06:30 +00:00
parent 5c2f93f82f
commit 6f1292bfcf
4 changed files with 221 additions and 199 deletions
@@ -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<NormalizedRelayUrl, Counter>()

+    // 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<MutableSharedFlow<ListChange<Note>>>? = null

    fun changesFlow(): MutableSharedFlow<ListChange<Note>> {
@@ -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) {
@@ -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) {
@@ -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<Note> {
-        val toRemove = messages
-        if (toRemove.isEmpty()) return toRemove
-        messages = emptySet()
-        newestMessage = null
-        unreadCount.value = 0
-        changesFlow?.get()?.tryEmit(ListChange.SetDeletion<Note>(toRemove))
-        return toRemove
-    }
+    fun clearAllMessagesSync(): Set<Note> =
+        syncLock.withLock {
+            val toRemove = messages
+            if (toRemove.isEmpty()) return@withLock toRemove
+            messages = emptySet()
+            newestMessage = null
+            unreadCount.value = 0
+            changesFlow?.get()?.tryEmit(ListChange.SetDeletion<Note>(toRemove))
+            toRemove
+        }
 }
@@ -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<MutableSharedFlow<ListChange<Note>>>? = null

    fun changesFlow(): MutableSharedFlow<ListChange<Note>> {
@@ -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<HexKey>): Boolean = activeSenders.any { it.pubkeyHex in keySet }