From 07b0c87c38e1a51d22ffcbcd6a4cb26ee52a7d48 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 3 Jun 2026 00:04:09 +0000 Subject: [PATCH] fix: stop giftwrap/rooms loads completing before their REQs are sent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The silence + connect-grace backstops are only meaningful when the owner feeds onReqSent, which only the convo NIP-04 path does. But connectStalled keyed off the ABSENCE of a recorded REQ, so for giftwrap/rooms (which never call onReqSent) every relay looked connect-stalled after connectGrace — the window completed at ~15s before its REQs had even gone out during a slow connect storm ('giftwrap.live load done: settled/silent' 45s before the REQ), prematurely declaring an empty round done and tripping the no-progress guard, so giftwraps stopped loading. Gate both REQ-aware backstops behind a tracksReqSends flag that only the convo manager sets; everyone else keeps the plain settle / idle / cap behavior. Adds a regression test that a non-tracking tracker keeps blocking a never-heard-from relay until it actually settles. https://claude.ai/code/session_01B1fmmmX8JjQWH3amMLdvcW --- .../eoseManagers/WindowLoadTracker.kt | 13 +++++-- .../ChatroomNip04HistorySubAssembler.kt | 2 +- .../WindowLoadTrackerSilenceTest.kt | 34 ++++++++++++++++++- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/eoseManagers/WindowLoadTracker.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/eoseManagers/WindowLoadTracker.kt index 0f5e3dff10..3387ce2c49 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/eoseManagers/WindowLoadTracker.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/eoseManagers/WindowLoadTracker.kt @@ -70,10 +70,19 @@ import kotlin.time.Duration.Companion.seconds * blocking the round after [connectGrace] from the load start — but it is NOT given up (it may be a * genuinely slow connect), so the owner keeps it and retries it next round. And an [absoluteCap] is * the final ceiling on a window that somehow defeats all of the above. + * + * The two REQ-aware backstops (silence + connect-grace) only make sense when the owner actually feeds + * [onReqSent], so they are gated behind [tracksReqSends]. A tracker that does NOT track REQ sends keeps + * the plain settle / idle / cap behavior — otherwise, with an always-empty [reqSentAt], EVERY relay + * would look "connect-stalled" after [connectGrace] and the window would complete before its REQs even + * went out (e.g. during a slow connect storm), prematurely declaring an empty round done. */ class WindowLoadTracker( // Short label for the DMPagination logs (e.g. "giftwrap", "rooms.nip04", "convo.nip04"). private val name: String = "dm", + // Whether the owner feeds [onReqSent]; enables the silence + connect-grace backstops. Off by + // default so trackers that don't track REQ sends are unaffected by them. + private val tracksReqSends: Boolean = false, private val idleTimeout: Duration = 3.seconds, private val silenceTimeout: Duration = 10.seconds, private val connectGrace: Duration = 15.seconds, @@ -187,7 +196,7 @@ class WindowLoadTracker( private fun silencedOut( relay: NormalizedRelayUrl, now: Long, - ): Boolean = relay !in heardFrom && (reqSentAt[relay]?.let { now - it >= silenceTimeout.inWholeMilliseconds } ?: false) + ): Boolean = tracksReqSends && relay !in heardFrom && (reqSentAt[relay]?.let { now - it >= silenceTimeout.inWholeMilliseconds } ?: false) // A relay that is still expected but has neither been heard from nor even received its REQ within // [connectGrace] of the load start — i.e. stuck connecting / reconnecting. It stops blocking the @@ -195,7 +204,7 @@ class WindowLoadTracker( private fun connectStalled( relay: NormalizedRelayUrl, now: Long, - ): Boolean = relay !in heardFrom && !reqSentAt.containsKey(relay) && now - loadStartMs >= connectGrace.inWholeMilliseconds + ): Boolean = tracksReqSends && relay !in heardFrom && !reqSentAt.containsKey(relay) && now - loadStartMs >= connectGrace.inWholeMilliseconds /** Records which relays the current REQ was sent to. Completes immediately if there are none. */ @Synchronized diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/datasource/ChatroomNip04HistorySubAssembler.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/datasource/ChatroomNip04HistorySubAssembler.kt index 1bb665f986..1b8a456eac 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/datasource/ChatroomNip04HistorySubAssembler.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/datasource/ChatroomNip04HistorySubAssembler.kt @@ -69,7 +69,7 @@ class ChatroomNip04HistorySubAssembler( private val started = ConcurrentHashMap.newKeySet() private val askedRelays = ConcurrentHashMap>() - private val windowLoad = WindowLoadTracker("convo.nip04.history", onAbandoned = ::onRelaysAbandoned) + private val windowLoad = WindowLoadTracker("convo.nip04.history", tracksReqSends = true, onAbandoned = ::onRelaysAbandoned) val loadingMore: StateFlow = windowLoad.loading private val _exhausted = MutableStateFlow(false) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/eoseManagers/WindowLoadTrackerSilenceTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/eoseManagers/WindowLoadTrackerSilenceTest.kt index a4f1d42ac1..c7a31f1e8d 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/eoseManagers/WindowLoadTrackerSilenceTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/eoseManagers/WindowLoadTrackerSilenceTest.kt @@ -50,6 +50,7 @@ class WindowLoadTrackerSilenceTest { val tracker = WindowLoadTracker( name = "test", + tracksReqSends = true, silenceTimeout = 50.milliseconds, onAbandoned = { abandoned.set(it) }, ) @@ -77,6 +78,7 @@ class WindowLoadTrackerSilenceTest { val tracker = WindowLoadTracker( name = "test", + tracksReqSends = true, connectGrace = 50.milliseconds, onAbandoned = { abandoned.set(it) }, ) @@ -94,11 +96,41 @@ class WindowLoadTrackerSilenceTest { scope.cancel() } + @Test + fun withoutReqTrackingAStalledRelayKeepsBlockingUntilItSettles() = + runBlocking { + val scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) + // A tracker that does NOT feed onReqSent (giftwrap / rooms): the REQ-aware backstops must + // stay off, or every never-heard-from relay would look stalled and the window would finish + // before its REQs even went out (the connect-storm regression). + val tracker = + WindowLoadTracker( + name = "test", + tracksReqSends = false, + silenceTimeout = 50.milliseconds, + connectGrace = 50.milliseconds, + ) + + tracker.startLoading(scope) + tracker.setExpectedRelays(setOf(good, silent)) + tracker.onRelaySettled(good) + + // `silent` was never heard from and never got a REQ; well past both short backstops it must + // STILL block, because this tracker doesn't track REQ sends. + Thread.sleep(400) + assertTrue("non-req-tracking tracker must not abandon a stalled relay", tracker.loading.value) + + // Only an actual terminal signal completes it. + tracker.onRelaySettled(silent) + withTimeout(3000) { tracker.loading.first { !it } } + scope.cancel() + } + @Test fun aSilentRelayThatNeverGotAReqStillBlocksUntilItSettles() = runBlocking { val scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) - val tracker = WindowLoadTracker(name = "test", silenceTimeout = 50.milliseconds) + val tracker = WindowLoadTracker(name = "test", tracksReqSends = true, silenceTimeout = 50.milliseconds) tracker.startLoading(scope) tracker.setExpectedRelays(setOf(good, silent))