fix: replace JVM-only synchronized with Mutex in quartz commonMain
MarmotInboundProcessor and CommitOrdering.EpochCommitTracker used
`synchronized(lock) { ... }`, which is a JVM-only intrinsic. Compiling
the quartz KMP module for iosSimulatorArm64 (and other non-JVM targets)
failed with "Unresolved reference 'synchronized'".
Switch to kotlinx.coroutines.sync.Mutex + withLock, matching the pattern
already used in MlsGroupManager. EpochCommitTracker's public API becomes
suspend — update the MarmotInboundProcessor delegates (pendingCommitGroupEpochs,
clearPendingCommits) and wrap the commonTest cases in runTest. The
MarmotPipelineTest jvmAndroid tests already run inside runBlocking, so
no changes needed there.
Also reshape the processGroupEvent dedup check to hoist the "already
processed?" read out of the lock block so the early return isn't a
non-local return from the withLock lambda.
This commit is contained in:
+11
-8
@@ -35,6 +35,8 @@ import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupManager
|
|||||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||||
import com.vitorpamplona.quartz.nip01Core.core.HexKey
|
import com.vitorpamplona.quartz.nip01Core.core.HexKey
|
||||||
import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray
|
import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray
|
||||||
|
import kotlinx.coroutines.sync.Mutex
|
||||||
|
import kotlinx.coroutines.sync.withLock
|
||||||
import kotlin.io.encoding.Base64
|
import kotlin.io.encoding.Base64
|
||||||
import kotlin.io.encoding.ExperimentalEncodingApi
|
import kotlin.io.encoding.ExperimentalEncodingApi
|
||||||
|
|
||||||
@@ -126,7 +128,7 @@ class MarmotInboundProcessor(
|
|||||||
private val keyPackageRotationManager: KeyPackageRotationManager,
|
private val keyPackageRotationManager: KeyPackageRotationManager,
|
||||||
) {
|
) {
|
||||||
private val commitTracker = CommitOrdering.EpochCommitTracker()
|
private val commitTracker = CommitOrdering.EpochCommitTracker()
|
||||||
private val processedIdsLock = Any()
|
private val processedIdsMutex = Mutex()
|
||||||
private val processedEventIds = LinkedHashSet<String>()
|
private val processedEventIds = LinkedHashSet<String>()
|
||||||
|
|
||||||
companion object {
|
companion object {
|
||||||
@@ -156,11 +158,12 @@ class MarmotInboundProcessor(
|
|||||||
suspend fun processGroupEvent(groupEvent: GroupEvent): GroupEventResult {
|
suspend fun processGroupEvent(groupEvent: GroupEvent): GroupEventResult {
|
||||||
// Deduplicate already-processed events (thread-safe)
|
// Deduplicate already-processed events (thread-safe)
|
||||||
val eventId = groupEvent.id
|
val eventId = groupEvent.id
|
||||||
synchronized(processedIdsLock) {
|
val alreadyProcessed =
|
||||||
if (eventId in processedEventIds) {
|
processedIdsMutex.withLock {
|
||||||
val gId = groupEvent.groupId()
|
eventId in processedEventIds
|
||||||
return GroupEventResult.Duplicate(gId ?: "")
|
|
||||||
}
|
}
|
||||||
|
if (alreadyProcessed) {
|
||||||
|
return GroupEventResult.Duplicate(groupEvent.groupId() ?: "")
|
||||||
}
|
}
|
||||||
|
|
||||||
val groupId =
|
val groupId =
|
||||||
@@ -189,7 +192,7 @@ class MarmotInboundProcessor(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Track ALL processed events for deduplication (including errors to prevent replay DoS)
|
// Track ALL processed events for deduplication (including errors to prevent replay DoS)
|
||||||
synchronized(processedIdsLock) {
|
processedIdsMutex.withLock {
|
||||||
processedEventIds.add(eventId)
|
processedEventIds.add(eventId)
|
||||||
// Trim the set if it exceeds the max size
|
// Trim the set if it exceeds the max size
|
||||||
if (processedEventIds.size > MAX_PROCESSED_IDS) {
|
if (processedEventIds.size > MAX_PROCESSED_IDS) {
|
||||||
@@ -288,12 +291,12 @@ class MarmotInboundProcessor(
|
|||||||
/**
|
/**
|
||||||
* Get all (group, epoch) keys that have pending unresolved commits.
|
* Get all (group, epoch) keys that have pending unresolved commits.
|
||||||
*/
|
*/
|
||||||
fun pendingCommitGroupEpochs(): Set<CommitOrdering.GroupEpochKey> = commitTracker.pendingGroupEpochs()
|
suspend fun pendingCommitGroupEpochs(): Set<CommitOrdering.GroupEpochKey> = commitTracker.pendingGroupEpochs()
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Clear all pending commit state.
|
* Clear all pending commit state.
|
||||||
*/
|
*/
|
||||||
fun clearPendingCommits() {
|
suspend fun clearPendingCommits() {
|
||||||
commitTracker.clear()
|
commitTracker.clear()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+17
-13
@@ -20,6 +20,9 @@
|
|||||||
*/
|
*/
|
||||||
package com.vitorpamplona.quartz.marmot.mip03GroupMessages
|
package com.vitorpamplona.quartz.marmot.mip03GroupMessages
|
||||||
|
|
||||||
|
import kotlinx.coroutines.sync.Mutex
|
||||||
|
import kotlinx.coroutines.sync.withLock
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Deterministic commit conflict resolution for MLS over Nostr (MIP-03).
|
* Deterministic commit conflict resolution for MLS over Nostr (MIP-03).
|
||||||
*
|
*
|
||||||
@@ -82,7 +85,7 @@ object CommitOrdering {
|
|||||||
* to determine which commit wins for each (group, epoch).
|
* to determine which commit wins for each (group, epoch).
|
||||||
*/
|
*/
|
||||||
class EpochCommitTracker {
|
class EpochCommitTracker {
|
||||||
private val lock = Any()
|
private val mutex = Mutex()
|
||||||
private val pendingByGroupEpoch = mutableMapOf<GroupEpochKey, MutableList<GroupEvent>>()
|
private val pendingByGroupEpoch = mutableMapOf<GroupEpochKey, MutableList<GroupEvent>>()
|
||||||
|
|
||||||
companion object {
|
companion object {
|
||||||
@@ -97,11 +100,11 @@ object CommitOrdering {
|
|||||||
* @param epoch the MLS epoch number this commit targets
|
* @param epoch the MLS epoch number this commit targets
|
||||||
* @param commit the GroupEvent containing the commit
|
* @param commit the GroupEvent containing the commit
|
||||||
*/
|
*/
|
||||||
fun addCommit(
|
suspend fun addCommit(
|
||||||
groupId: String,
|
groupId: String,
|
||||||
epoch: Long,
|
epoch: Long,
|
||||||
commit: GroupEvent,
|
commit: GroupEvent,
|
||||||
) = synchronized(lock) {
|
) = mutex.withLock {
|
||||||
val key = GroupEpochKey(groupId, epoch)
|
val key = GroupEpochKey(groupId, epoch)
|
||||||
pendingByGroupEpoch.getOrPut(key) { mutableListOf() }.add(commit)
|
pendingByGroupEpoch.getOrPut(key) { mutableListOf() }.add(commit)
|
||||||
|
|
||||||
@@ -120,11 +123,11 @@ object CommitOrdering {
|
|||||||
/**
|
/**
|
||||||
* Returns pending commits for a specific group and epoch.
|
* Returns pending commits for a specific group and epoch.
|
||||||
*/
|
*/
|
||||||
fun pendingForEpoch(
|
suspend fun pendingForEpoch(
|
||||||
groupId: String,
|
groupId: String,
|
||||||
epoch: Long,
|
epoch: Long,
|
||||||
): List<GroupEvent> =
|
): List<GroupEvent> =
|
||||||
synchronized(lock) {
|
mutex.withLock {
|
||||||
pendingByGroupEpoch[GroupEpochKey(groupId, epoch)]?.toList() ?: emptyList()
|
pendingByGroupEpoch[GroupEpochKey(groupId, epoch)]?.toList() ?: emptyList()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -135,37 +138,38 @@ object CommitOrdering {
|
|||||||
* @param epoch the MLS epoch to resolve
|
* @param epoch the MLS epoch to resolve
|
||||||
* @return the winning commit, or null if no commits exist for this (group, epoch)
|
* @return the winning commit, or null if no commits exist for this (group, epoch)
|
||||||
*/
|
*/
|
||||||
fun resolve(
|
suspend fun resolve(
|
||||||
groupId: String,
|
groupId: String,
|
||||||
epoch: Long,
|
epoch: Long,
|
||||||
): GroupEvent? =
|
): GroupEvent? =
|
||||||
synchronized(lock) {
|
mutex.withLock {
|
||||||
selectWinner(pendingByGroupEpoch[GroupEpochKey(groupId, epoch)] ?: emptyList())
|
selectWinner(pendingByGroupEpoch[GroupEpochKey(groupId, epoch)] ?: emptyList())
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Clears pending commits for a (group, epoch) after it has been resolved.
|
* Clears pending commits for a (group, epoch) after it has been resolved.
|
||||||
*/
|
*/
|
||||||
fun clearEpoch(
|
suspend fun clearEpoch(
|
||||||
groupId: String,
|
groupId: String,
|
||||||
epoch: Long,
|
epoch: Long,
|
||||||
) = synchronized(lock) {
|
) = mutex.withLock {
|
||||||
pendingByGroupEpoch.remove(GroupEpochKey(groupId, epoch))
|
pendingByGroupEpoch.remove(GroupEpochKey(groupId, epoch))
|
||||||
|
Unit
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns all (group, epoch) keys that have pending commits.
|
* Returns all (group, epoch) keys that have pending commits.
|
||||||
*/
|
*/
|
||||||
fun pendingGroupEpochs(): Set<GroupEpochKey> =
|
suspend fun pendingGroupEpochs(): Set<GroupEpochKey> =
|
||||||
synchronized(lock) {
|
mutex.withLock {
|
||||||
pendingByGroupEpoch.keys.toSet()
|
pendingByGroupEpoch.keys.toSet()
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Clears all pending state.
|
* Clears all pending state.
|
||||||
*/
|
*/
|
||||||
fun clear() =
|
suspend fun clear() =
|
||||||
synchronized(lock) {
|
mutex.withLock {
|
||||||
pendingByGroupEpoch.clear()
|
pendingByGroupEpoch.clear()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+44
-39
@@ -22,6 +22,7 @@ package com.vitorpamplona.quartz.marmot
|
|||||||
|
|
||||||
import com.vitorpamplona.quartz.marmot.mip03GroupMessages.CommitOrdering
|
import com.vitorpamplona.quartz.marmot.mip03GroupMessages.CommitOrdering
|
||||||
import com.vitorpamplona.quartz.marmot.mip03GroupMessages.GroupEvent
|
import com.vitorpamplona.quartz.marmot.mip03GroupMessages.GroupEvent
|
||||||
|
import kotlinx.coroutines.test.runTest
|
||||||
import kotlin.test.Test
|
import kotlin.test.Test
|
||||||
import kotlin.test.assertEquals
|
import kotlin.test.assertEquals
|
||||||
import kotlin.test.assertFalse
|
import kotlin.test.assertFalse
|
||||||
@@ -134,57 +135,61 @@ class CommitOrderingTest {
|
|||||||
// ===== EpochCommitTracker =====
|
// ===== EpochCommitTracker =====
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testEpochCommitTracker_Basic() {
|
fun testEpochCommitTracker_Basic() =
|
||||||
val tracker = CommitOrdering.EpochCommitTracker()
|
runTest {
|
||||||
val epoch1Commit1 = makeGroupEvent("bbb", 1000)
|
val tracker = CommitOrdering.EpochCommitTracker()
|
||||||
val epoch1Commit2 = makeGroupEvent("aaa", 1001)
|
val epoch1Commit1 = makeGroupEvent("bbb", 1000)
|
||||||
|
val epoch1Commit2 = makeGroupEvent("aaa", 1001)
|
||||||
|
|
||||||
tracker.addCommit(groupId, 1L, epoch1Commit1)
|
tracker.addCommit(groupId, 1L, epoch1Commit1)
|
||||||
tracker.addCommit(groupId, 1L, epoch1Commit2)
|
tracker.addCommit(groupId, 1L, epoch1Commit2)
|
||||||
|
|
||||||
assertEquals(2, tracker.pendingForEpoch(groupId, 1L).size)
|
assertEquals(2, tracker.pendingForEpoch(groupId, 1L).size)
|
||||||
assertEquals(0, tracker.pendingForEpoch(groupId, 2L).size)
|
assertEquals(0, tracker.pendingForEpoch(groupId, 2L).size)
|
||||||
|
|
||||||
// Resolve: epoch1Commit1 wins (earlier timestamp)
|
// Resolve: epoch1Commit1 wins (earlier timestamp)
|
||||||
val winner = tracker.resolve(groupId, 1L)
|
val winner = tracker.resolve(groupId, 1L)
|
||||||
assertEquals(epoch1Commit1, winner)
|
assertEquals(epoch1Commit1, winner)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testEpochCommitTracker_MultipleEpochs() {
|
fun testEpochCommitTracker_MultipleEpochs() =
|
||||||
val tracker = CommitOrdering.EpochCommitTracker()
|
runTest {
|
||||||
val e1 = makeGroupEvent("aaa", 1000)
|
val tracker = CommitOrdering.EpochCommitTracker()
|
||||||
val e2 = makeGroupEvent("bbb", 2000)
|
val e1 = makeGroupEvent("aaa", 1000)
|
||||||
|
val e2 = makeGroupEvent("bbb", 2000)
|
||||||
|
|
||||||
tracker.addCommit(groupId, 1L, e1)
|
tracker.addCommit(groupId, 1L, e1)
|
||||||
tracker.addCommit(groupId, 2L, e2)
|
tracker.addCommit(groupId, 2L, e2)
|
||||||
|
|
||||||
val expectedKeys =
|
val expectedKeys =
|
||||||
setOf(
|
setOf(
|
||||||
CommitOrdering.GroupEpochKey(groupId, 1L),
|
CommitOrdering.GroupEpochKey(groupId, 1L),
|
||||||
CommitOrdering.GroupEpochKey(groupId, 2L),
|
CommitOrdering.GroupEpochKey(groupId, 2L),
|
||||||
)
|
)
|
||||||
assertEquals(expectedKeys, tracker.pendingGroupEpochs())
|
assertEquals(expectedKeys, tracker.pendingGroupEpochs())
|
||||||
|
|
||||||
tracker.clearEpoch(groupId, 1L)
|
tracker.clearEpoch(groupId, 1L)
|
||||||
assertEquals(setOf(CommitOrdering.GroupEpochKey(groupId, 2L)), tracker.pendingGroupEpochs())
|
assertEquals(setOf(CommitOrdering.GroupEpochKey(groupId, 2L)), tracker.pendingGroupEpochs())
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testEpochCommitTracker_ClearAll() {
|
fun testEpochCommitTracker_ClearAll() =
|
||||||
val tracker = CommitOrdering.EpochCommitTracker()
|
runTest {
|
||||||
tracker.addCommit(groupId, 1L, makeGroupEvent("aaa", 1000))
|
val tracker = CommitOrdering.EpochCommitTracker()
|
||||||
tracker.addCommit(groupId, 2L, makeGroupEvent("bbb", 2000))
|
tracker.addCommit(groupId, 1L, makeGroupEvent("aaa", 1000))
|
||||||
|
tracker.addCommit(groupId, 2L, makeGroupEvent("bbb", 2000))
|
||||||
|
|
||||||
tracker.clear()
|
tracker.clear()
|
||||||
|
|
||||||
assertTrue(tracker.pendingGroupEpochs().isEmpty())
|
assertTrue(tracker.pendingGroupEpochs().isEmpty())
|
||||||
assertNull(tracker.resolve(groupId, 1L))
|
assertNull(tracker.resolve(groupId, 1L))
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testEpochCommitTracker_ResolveEmpty() {
|
fun testEpochCommitTracker_ResolveEmpty() =
|
||||||
val tracker = CommitOrdering.EpochCommitTracker()
|
runTest {
|
||||||
assertNull(tracker.resolve(groupId, 999L))
|
val tracker = CommitOrdering.EpochCommitTracker()
|
||||||
}
|
assertNull(tracker.resolve(groupId, 999L))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user