From 5fe33a815247c1a1e5edc041759a2e5d95f6dd3b Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Fri, 31 Jul 2026 19:36:28 -0400 Subject: [PATCH] fix(relays): serialize writes to the single-sub EOSE cursor map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EOSERelayList backs SingleSubEoseManager with a plain mutableMapOf, and addOrUpdate is called from SubscriptionListener callbacks — i.e. from each relay's own socket-reader thread. A client holding a few hundred relays therefore had that many concurrent writers to one unsynchronized map. EOSEAccountFast wraps its lists in a lock for exactly this reason; a bare list handed to SingleSubEoseManager had nothing. The race predates this branch but the branch made it load-bearing: both merged managers (notifications and account metadata, across every logged-in account and every relay they read) now run through SingleSubEoseManager, and the EOSE refetch fix added a second writer in remove(). Writes are serialized with KmpLock, the same primitive ComposeSubscriptionManager uses. Reads still go through the live map from since(), deliberately — but the two merged managers no longer depend on that. They were reading `since` immediately after clearing a relay and relying on the mutation being visible through it, so hardening since() into a snapshot later would have silently disabled the refetch with no test to notice. Growth now zeroes the cursor for that pass explicitly, in addition to clearing it. Verified: both iOS targets, JVM, Android, desktop and the full suite build and pass; a cold start on emulator-5554 shows no fatal exceptions, no ConcurrentModificationException, and 0 nos.lol refusals. Co-Authored-By: Claude Opus 5 (1M context) --- .../metadata/AccountMetadataEoseManager.kt | 7 +++-- ...NotificationsEoseFromInboxRelaysManager.kt | 10 ++++--- .../amethyst/commons/relays/EOSERelayList.kt | 29 ++++++++++++++----- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/metadata/AccountMetadataEoseManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/metadata/AccountMetadataEoseManager.kt index 9abf466e80..283ee421e1 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/metadata/AccountMetadataEoseManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/metadata/AccountMetadataEoseManager.kt @@ -73,9 +73,12 @@ class AccountMetadataEoseManager( // the earlier accounts earned — it would never ask for its own profile, follows or lists. // This matters more here than for notifications: there is no backward pager to rescue it, // so the account would go without until the next launch cleared the in-memory cursor. - if (authorsPerRelay.gainedAuthors(relay, pubkeys)) clearEoseFor(relay) + // Drop the stored cursor AND ignore it for this pass, so the refetch does not depend on + // `since` being a live view of the map we just mutated. + val gained = authorsPerRelay.gainedAuthors(relay, pubkeys) + if (gained) clearEoseFor(relay) - val relaySince = since?.get(relay)?.time + val relaySince = if (gained) null else since?.get(relay)?.time // The account-switcher avatars: other logged-in accounts this screen wants to name. // Screens supply them; the background registry does not, so this is usually empty. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/nip01Notifications/AccountNotificationsEoseFromInboxRelaysManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/nip01Notifications/AccountNotificationsEoseFromInboxRelaysManager.kt index 9b8342508e..e01043fe6b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/nip01Notifications/AccountNotificationsEoseFromInboxRelaysManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/reqCommand/account/nip01Notifications/AccountNotificationsEoseFromInboxRelaysManager.kt @@ -92,9 +92,11 @@ class AccountNotificationsEoseFromInboxRelaysManager( val pubkeys = accounts.map { it.account.userProfile().pubkeyHex } // An account that joins a relay this subscription already covers would otherwise inherit - // the cursor the earlier accounts earned, and never ask for anything older than it. - // Dropping the cursor first means `since` below reads null and the merged filter refetches. - if (authorsPerRelay.gainedAuthors(relay, pubkeys)) clearEoseFor(relay) + // the cursor the earlier accounts earned, and never ask for anything older than it. Drop + // the stored cursor AND ignore it for this pass — not just the former, which would leave + // the refetch depending on `since` being a live view of the map we just mutated. + val gained = authorsPerRelay.gainedAuthors(relay, pubkeys) + if (gained) clearEoseFor(relay) // A cold-start floor, NOT paging — backward paging lives in // [AccountNotificationsHistoryEoseManager]. Read only when a relay has no EOSE yet; once it @@ -119,7 +121,7 @@ class AccountNotificationsEoseFromInboxRelaysManager( // could never rescue it — it only arms once the feed holds a full page, and the feed // could not fill because the query only ever asked for a week. A fresh install of an // established account hit the same deadlock. - val notificationSince = since?.get(relay)?.time ?: pagingBoundary + val notificationSince = (if (gained) null else since?.get(relay)?.time) ?: pagingBoundary // NIP-29 group activity (reactions/replies to my messages) is deliberately NOT requested // here. It lives on the group's host relay and used to be one `#h` filter per relay carrying diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relays/EOSERelayList.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relays/EOSERelayList.kt index 53c7aadc3c..dc831f3db6 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relays/EOSERelayList.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relays/EOSERelayList.kt @@ -20,6 +20,8 @@ */ package com.vitorpamplona.amethyst.commons.relays +import com.vitorpamplona.amethyst.commons.util.KmpLock +import com.vitorpamplona.amethyst.commons.util.withLock import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl typealias SincePerRelayMap = MutableMap @@ -31,10 +33,21 @@ typealias SincePerRelayMap = MutableMap class EOSERelayList { var relayList: SincePerRelayMap = mutableMapOf() + /** + * Writers are serialized because they are not all on one thread: [addOrUpdate] runs on each + * relay's own socket-reader thread as EOSE frames land, so a client holding a few hundred relays + * has that many potential writers to one plain map. [EOSEAccountFast] wraps its lists in a lock + * for the same reason; a bare list handed to [SingleSubEoseManager] had none. + * + * Reads still go through the map returned by [since] — deliberately live rather than a snapshot, + * since callers clear a relay and then re-read it inside one assembly pass. + */ + private val lock = KmpLock() + fun addOrUpdate( relayUrl: NormalizedRelayUrl, time: Long, - ) { + ) = lock.withLock { val eose = relayList[relayUrl] if (eose == null) { relayList[relayUrl] = MutableTime(time) @@ -43,9 +56,10 @@ class EOSERelayList { } } - fun clear() { - relayList = mutableMapOf() - } + fun clear() = + lock.withLock { + relayList = mutableMapOf() + } /** * Forgets one relay's cursor, so the next filter built for it asks from scratch. @@ -54,9 +68,10 @@ class EOSERelayList { * that starts covering another account has already-EOSE'd relays whose `since` would silence * exactly the history the new account still needs. */ - fun remove(relayUrl: NormalizedRelayUrl) { - relayList.remove(relayUrl) - } + fun remove(relayUrl: NormalizedRelayUrl) = + lock.withLock { + relayList.remove(relayUrl) + } fun since() = relayList