mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-09 08:04:45 +00:00
fix(chats): stop crashing when a relay's group directory is sorted
Browsing a relay's groups (Relay Groups → Find groups → pick a relay)
crashed the app to the launcher:
java.lang.IllegalArgumentException: Comparison method violates its
general contract!
at java.util.TimSort.mergeLo
...
at RelayGroupChannelListScreen$allChannels$3$1$1.emit
`sortedBy { it.toBestDisplayName().lowercase() }` re-evaluates its key on
every comparison, and the key comes from mutable shared state — the
channel's display name, which a kind-39000 directory event can change
while the sort is running. TimSort detects the inconsistency and throws.
The failure scales with directory size and needs no user action: a relay
hosting 1237 groups tripped it on the first browse, while a relay with 18
never did — which is why an earlier sweep of this same screen missed it.
v1.12.6 already fixed this class elsewhere ("snapshot live-stream status
order before sorting"); these sites were not covered.
Adds `sortedBySnapshot`, which computes each key ONCE before any
comparison runs, and applies it to all four sites that sorted live
objects by their display name: the relay group directory (both the
initial value and the observer), the parent-group picker, and the
name-ordered search results. The picker's site also had to materialise
its Sequence first, since sorting lazily would have re-introduced the
same window.
Verified on device: the exact tap that crashed now loads the screen with
zero FATAL EXCEPTIONs and the app stays in the foreground.
Found while setting up a NIP-29 group to test the admin surface, which is
also how the directory got large enough to expose it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
894dc7d99b
commit
a87049e451
+3
-2
@@ -53,6 +53,7 @@ import com.vitorpamplona.amethyst.R
|
||||
import com.vitorpamplona.amethyst.commons.icons.symbols.Icon
|
||||
import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols
|
||||
import com.vitorpamplona.amethyst.commons.model.nip29RelayGroups.RelayGroupChannel
|
||||
import com.vitorpamplona.amethyst.commons.util.sortedBySnapshot
|
||||
import com.vitorpamplona.amethyst.model.LocalCache
|
||||
import com.vitorpamplona.amethyst.model.nip11RelayInfo.isRelaySignedRelayGroup
|
||||
import com.vitorpamplona.amethyst.model.nip11RelayInfo.loadRelayInfo
|
||||
@@ -101,13 +102,13 @@ fun RelayGroupChannelListScreen(
|
||||
// updates as directory events arrive with no polling. The initial value is sorted too
|
||||
// so the first frame doesn't reshuffle when the first emission arrives.
|
||||
val allChannels by produceState(
|
||||
initialValue = accountViewModel.getRelayGroupChannelsOnRelay(relay).sortedBy { it.toBestDisplayName().lowercase() },
|
||||
initialValue = accountViewModel.getRelayGroupChannelsOnRelay(relay).sortedBySnapshot { it.toBestDisplayName().lowercase() },
|
||||
relay,
|
||||
) {
|
||||
LocalCache
|
||||
.observeEvents<GroupMetadataEvent>(Filter(kinds = listOf(GroupMetadataEvent.KIND)))
|
||||
.collect {
|
||||
value = accountViewModel.getRelayGroupChannelsOnRelay(relay).sortedBy { it.toBestDisplayName().lowercase() }
|
||||
value = accountViewModel.getRelayGroupChannelsOnRelay(relay).sortedBySnapshot { it.toBestDisplayName().lowercase() }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+2
-1
@@ -65,6 +65,7 @@ import com.vitorpamplona.amethyst.R
|
||||
import com.vitorpamplona.amethyst.commons.icons.symbols.Icon
|
||||
import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols
|
||||
import com.vitorpamplona.amethyst.commons.model.nip29RelayGroups.RelayGroupChannel
|
||||
import com.vitorpamplona.amethyst.commons.util.sortedBySnapshot
|
||||
import com.vitorpamplona.amethyst.model.LocalCache
|
||||
import com.vitorpamplona.amethyst.model.nip11RelayInfo.isRelaySignedRelayGroup
|
||||
import com.vitorpamplona.amethyst.model.nip11RelayInfo.loadRelayInfo
|
||||
@@ -467,8 +468,8 @@ private fun pickCandidates(
|
||||
.asSequence()
|
||||
.filter { it.groupId.id !in forbidden }
|
||||
.filter { it.event != null && isRelaySignedRelayGroup(it, relayInfo) }
|
||||
.sortedBy { it.toBestDisplayName().lowercase() }
|
||||
.toList()
|
||||
.sortedBySnapshot { it.toBestDisplayName().lowercase() }
|
||||
|
||||
/**
|
||||
* The set of group ids reachable as descendants of [rootId] on [relay], following each group's
|
||||
|
||||
+2
-1
@@ -21,6 +21,7 @@
|
||||
package com.vitorpamplona.amethyst.commons.search
|
||||
|
||||
import com.vitorpamplona.amethyst.commons.model.User
|
||||
import com.vitorpamplona.amethyst.commons.util.sortedBySnapshot
|
||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip23LongContent.LongTextNoteEvent
|
||||
import com.vitorpamplona.quartz.utils.currentTimeSeconds
|
||||
@@ -64,7 +65,7 @@ object SearchResultSorter {
|
||||
order: SearchSortOrder,
|
||||
): List<User> =
|
||||
when (order) {
|
||||
SearchSortOrder.NAME_AZ -> people.sortedBy { it.toBestDisplayName().lowercase() }
|
||||
SearchSortOrder.NAME_AZ -> people.sortedBySnapshot { it.toBestDisplayName().lowercase() }
|
||||
SearchSortOrder.NAME_ZA -> people.sortedByDescending { it.toBestDisplayName().lowercase() }
|
||||
else -> people
|
||||
}
|
||||
|
||||
+39
@@ -0,0 +1,39 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.commons.util
|
||||
|
||||
/**
|
||||
* Sorts by a key computed ONCE per element, before any comparison runs.
|
||||
*
|
||||
* `sortedBy { it.liveProperty }` re-evaluates the key on every comparison, which is a crash — not a
|
||||
* cosmetic issue — when the key comes from mutable shared state such as a User's or channel's display
|
||||
* name. A relay event landing mid-sort changes the key underneath TimSort, it detects the
|
||||
* inconsistency, and throws `IllegalArgumentException: Comparison method violates its general
|
||||
* contract!`. It needs no user action and scales with list size: a directory of a thousand groups
|
||||
* trips it readily while a handful never does.
|
||||
*
|
||||
* Snapshotting first makes the comparator total and stable for the duration of the sort, whatever the
|
||||
* network does. The extra list is the price of not crashing.
|
||||
*/
|
||||
inline fun <T, R : Comparable<R>> Iterable<T>.sortedBySnapshot(key: (T) -> R): List<T> =
|
||||
map { it to key(it) }
|
||||
.sortedBy { it.second }
|
||||
.map { it.first }
|
||||
Reference in New Issue
Block a user