mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
fix(relays): serialize writes to the single-sub EOSE cursor map
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b8d0caa8ff
commit
5fe33a8152
+5
-2
@@ -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.
|
||||
|
||||
+6
-4
@@ -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
|
||||
|
||||
+22
-7
@@ -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<NormalizedRelayUrl, MutableTime>
|
||||
@@ -31,10 +33,21 @@ typealias SincePerRelayMap = MutableMap<NormalizedRelayUrl, MutableTime>
|
||||
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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user