diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/Amber.kt b/app/src/main/java/com/greenart7c3/nostrsigner/Amber.kt index fd892913..65643912 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/Amber.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/Amber.kt @@ -43,6 +43,7 @@ import com.greenart7c3.nostrsigner.okhttp.HttpClientManager import com.greenart7c3.nostrsigner.okhttp.OkHttpWebSocket import com.greenart7c3.nostrsigner.relays.AmberRelayStats import com.greenart7c3.nostrsigner.relays.NostrClientLoggerListener +import com.greenart7c3.nostrsigner.service.ApplicationNameCache import com.greenart7c3.nostrsigner.service.ClearLogsWorker import com.greenart7c3.nostrsigner.service.ConnectivityService import com.greenart7c3.nostrsigner.service.NotificationSubscription @@ -147,6 +148,36 @@ class Amber : private var logDatabases = ConcurrentHashMap() private var historyDatabases = ConcurrentHashMap() + // Hoisted out of runMigrations() so re-entry (e.g. test re-init) can't + // double-register and double-fire foreground/background callbacks. + private var processLifecycleObserverRegistered = false + private val processLifecycleObserver = object : DefaultLifecycleObserver { + override fun onStart(owner: LifecycleOwner) { + Log.d("ProcessLifecycleOwner", "App in foreground") + isAppInForeground = true + + // activates the profile filter only when the app is in the foreground + if (!settings.killSwitch.value) { + applicationIOScope.launch { + profileSubscription.updateFilter() + if (settings.autoCheckUpdates) { + maybeCheckForUpdates() + } + } + } + } + + override fun onStop(owner: LifecycleOwner) { + Log.d("ProcessLifecycleOwner", "App in background") + isAppInForeground = false + + // closes the filter when in the background + applicationIOScope.launch { + profileSubscription.closeSub() + } + } + } + val isOnMobileDataState = mutableStateOf(false) val isOnWifiDataState = mutableStateOf(false) val isOnOfflineState = mutableStateOf(false) @@ -155,7 +186,11 @@ class Amber : val keystoreFailedAccounts = MutableStateFlow>(emptyList()) @Volatile var intentionalDisconnectTime = 0L - val notificationCache = LruCache(10) + + // Capacity 10 was too small under bursty NIP-46 traffic — duplicate-detection + // started missing recent events. 512 covers high-volume relays without being + // a meaningful memory cost (event-id hex strings + Long). + val notificationCache = LruCache(512) fun isSocksProxyAlive(proxyHost: String, proxyPort: Int): Boolean { if (settings.torMode == TorMode.BUILTIN) { @@ -294,32 +329,10 @@ class Amber : } launch(Dispatchers.Main) { - ProcessLifecycleOwner.get().lifecycle.addObserver(object : DefaultLifecycleObserver { - override fun onStart(owner: LifecycleOwner) { - Log.d("ProcessLifecycleOwner", "App in foreground") - isAppInForeground = true - - // activates the profile filter only when the app is in the foreground - if (!settings.killSwitch.value) { - applicationIOScope.launch { - profileSubscription.updateFilter() - if (settings.autoCheckUpdates) { - maybeCheckForUpdates() - } - } - } - } - - override fun onStop(owner: LifecycleOwner) { - Log.d("ProcessLifecycleOwner", "App in background") - isAppInForeground = false - - // closes the filter when in the background - applicationIOScope.launch { - profileSubscription.closeSub() - } - } - }) + if (!processLifecycleObserverRegistered) { + ProcessLifecycleOwner.get().lifecycle.addObserver(processLifecycleObserver) + processLifecycleObserverRegistered = true + } } // Wait for Tor to be ready before establishing relay connections @@ -422,6 +435,18 @@ class Amber : return historyDatabases[npub]!! } + /** + * Closes and evicts every cached Room handle for [npub] so file locks and + * worker threads are released. Called from the logout path; without it each + * removed account leaks 3 open Room connections forever. + */ + fun closeDatabasesFor(npub: String) { + databases.remove(npub)?.runCatching { close() } + logDatabases.remove(npub)?.runCatching { close() } + historyDatabases.remove(npub)?.runCatching { close() } + ApplicationNameCache.clearForAccount(npub) + } + fun getSavedRelays(account: Account): Set { val database = getDatabase(account.npub) val savedRelays = buildSet { diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/LocalPreferences.kt b/app/src/main/java/com/greenart7c3/nostrsigner/LocalPreferences.kt index 3e1590b1..1b23cf33 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/LocalPreferences.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/LocalPreferences.kt @@ -374,6 +374,10 @@ object LocalPreferences { @SuppressLint("ApplySharedPref") fun updatePrefsForLogout(npub: String, context: Context): Boolean { accountCache.remove(npub) + // Close Room handles BEFORE deleting on-disk preference/data files so + // we release file locks and worker-pool threads instead of leaking + // them for the lifetime of the process. + Amber.instance.closeDatabasesFor(npub) val userPrefs = sharedPrefs(context, npub) userPrefs.edit(commit = true) { clear() } removeAccount(context, npub) diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/ApplicationNameCache.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/ApplicationNameCache.kt index c79afb9f..8ca9fe14 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/ApplicationNameCache.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/ApplicationNameCache.kt @@ -1,7 +1,28 @@ package com.greenart7c3.nostrsigner.service -import java.util.concurrent.ConcurrentHashMap +import androidx.collection.LruCache +/** + * Bounded cache of "$npub-$key" -> display name. Previously an unbounded + * ConcurrentHashMap; long-running sessions with many connecting apps were + * leaking memory here. The size cap is conservative — names are short, but + * the cache grows with every external app the user has ever interacted with. + */ object ApplicationNameCache { - val names = ConcurrentHashMap() + private const val MAX_ENTRIES = 256 + + private val cache = LruCache(MAX_ENTRIES) + + operator fun get(key: String): String? = synchronized(cache) { cache.get(key) } + + operator fun set(key: String, value: String) { + synchronized(cache) { cache.put(key, value) } + } + + fun clearForAccount(npub: String) { + synchronized(cache) { + val toRemove = cache.snapshot().keys.filter { it.startsWith("$npub-") } + toRemove.forEach { cache.remove(it) } + } + } } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt index 825b6101..5dff653d 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/EventNotificationConsumer.kt @@ -54,7 +54,6 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal import com.vitorpamplona.quartz.nip01Core.tags.people.taggedUsers import com.vitorpamplona.quartz.nip04Dm.crypto.EncryptedInfo -import com.vitorpamplona.quartz.nip19Bech32.toNpub import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestConnect import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestNip04Decrypt @@ -108,7 +107,7 @@ class EventNotificationConsumer(private val applicationContext: Context) { } } - fun consume( + suspend fun consume( event: Event, relay: NormalizedRelayUrl, ) { @@ -144,24 +143,18 @@ class EventNotificationConsumer(private val applicationContext: Context) { saveLog("Direct account match for ${acc.npub}", relay.url) } - // If not a direct account match, search for a connection with this local pubkey + // Connection-based match via the in-memory index populated by + // NotificationSubscription.updateFilter. This replaces an O(accounts × connections) + // synchronous scan that previously ran inside runBlocking on the relay event thread. if (acc == null) { - val accounts = LocalPreferences.allSavedAccounts(applicationContext) - outer@ for (accountInfo in accounts) { - val account = LocalPreferences.loadFromEncryptedStorageSync(applicationContext, accountInfo.npub) - ?: continue - val connections = - kotlinx.coroutines.runBlocking { - Amber.instance.getDatabase(account.npub).dao().getAllWithLocalKey(account.hexKey) - } - val taggedNpub = taggedKey.toNPub() - for (conn in connections) { - if (conn.localKey.isNotEmpty() && conn.localPubKey.hexToByteArray().toNpub() == taggedNpub) { - acc = account - connectionPrivKey = conn.localKey - saveLog("Found connection for ${acc.npub} with local pubkey ${conn.localPubKey}", relay.url) - break@outer - } + val taggedHex = taggedKey.pubKey + val match = LocalKeyAccountIndex.lookup(taggedHex) + if (match != null) { + val resolved = LocalPreferences.loadFromEncryptedStorageSync(applicationContext, match.npub) + if (resolved != null) { + acc = resolved + connectionPrivKey = match.localPrivKey + saveLog("Found connection for ${acc.npub} with local pubkey $taggedHex", relay.url) } } } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/IntentRateLimiter.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/IntentRateLimiter.kt index c90f7dba..cddf832a 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/IntentRateLimiter.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/IntentRateLimiter.kt @@ -17,8 +17,11 @@ import java.util.concurrent.ConcurrentHashMap object IntentRateLimiter { const val UNKNOWN_PACKAGE = "" private const val UNKNOWN_PACKAGE_LIMIT = 3 - private const val CLEANUP_WHEN_SIZE_EXCEEDS = 256 - private const val CLEANUP_STALE_MS = 60L * 60L * 1000L + + // Cleanup runs at every checkAndRecord; tighter bounds make it harder for a + // misbehaving caller to balloon the bucket map before reclamation kicks in. + private const val CLEANUP_WHEN_SIZE_EXCEEDS = 64 + private const val CLEANUP_STALE_MS = 5L * 60L * 1000L data class BucketKey( val pkg: String, diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/LocalKeyAccountIndex.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/LocalKeyAccountIndex.kt new file mode 100644 index 00000000..72065c4b --- /dev/null +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/LocalKeyAccountIndex.kt @@ -0,0 +1,26 @@ +package com.greenart7c3.nostrsigner.service + +import java.util.concurrent.ConcurrentHashMap + +/** + * Maps a connection's local public key (hex) to the account npub it belongs to + * and the connection's local private key. Maintained by [NotificationSubscription.updateFilter]; + * read by [EventNotificationConsumer.consume] to avoid an O(accounts × connections) + * scan with synchronous DB calls and key encoding on every incoming bunker event. + */ +object LocalKeyAccountIndex { + data class Match(val npub: String, val localPrivKey: String) + + private val byLocalPubKey = ConcurrentHashMap() + + fun lookup(localPubKey: String): Match? = byLocalPubKey[localPubKey] + + fun replaceAll(entries: Map) { + byLocalPubKey.keys.retainAll(entries.keys) + byLocalPubKey.putAll(entries) + } + + fun clearForAccount(npub: String) { + byLocalPubKey.entries.removeAll { it.value.npub == npub } + } +} diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationSubscription.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationSubscription.kt index c9756279..a64d4125 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationSubscription.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationSubscription.kt @@ -35,6 +35,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter import com.vitorpamplona.quartz.nip46RemoteSigner.NostrConnectEvent import com.vitorpamplona.quartz.utils.TimeUtils import java.util.UUID +import kotlinx.coroutines.launch class NotificationSubscription( val client: NostrClient, @@ -43,15 +44,31 @@ class NotificationSubscription( private val eventNotificationConsumer = EventNotificationConsumer(appContext) private val subIds = mutableMapOf() + @Volatile + private var registered = false + init { - // listens until the app crashes. - client.addConnectionListener(this) + // listens until the app crashes — guard so re-init in tests + // doesn't double-register the listener. + if (!registered) { + client.addConnectionListener(this) + registered = true + } + } + + fun dispose() { + if (registered) { + client.removeConnectionListener(this) + registered = false + } } override fun onIncomingMessage(relay: IRelayClient, msgStr: String, msg: Message) { if (msg is EventMessage) { if (subIds.containsValue(msg.subId)) { - eventNotificationConsumer.consume(msg.event, relay.url) + Amber.instance.applicationIOScope.launch { + eventNotificationConsumer.consume(msg.event, relay.url) + } } } super.onIncomingMessage(relay, msgStr, msg) @@ -73,6 +90,7 @@ class NotificationSubscription( suspend fun updateFilter() { if (BuildFlavorChecker.isOfflineFlavor()) return val activeSubKeys = mutableSetOf() + val indexEntries = mutableMapOf() LocalPreferences.allAccounts(appContext).forEach { account -> val since = computeSince() @@ -84,6 +102,7 @@ class NotificationSubscription( // Per-connection subscription on each connection's own relays for (conn in connectionsWithLocalKey) { val connPubKey = conn.localPubKey + indexEntries[connPubKey] = LocalKeyAccountIndex.Match(account.npub, conn.localKey) val subKey = "${account.hexKey}_$connPubKey" activeSubKeys.add(subKey) if (!subIds.containsKey(subKey)) { @@ -135,6 +154,9 @@ class NotificationSubscription( client.unsubscribe(subId) } } + + // Refresh the localPubKey -> account index used by EventNotificationConsumer. + LocalKeyAccountIndex.replaceAll(indexEntries) } private fun computeSince(): Long { diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/ProfileSubscription.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/ProfileSubscription.kt index 1e891bf6..133898de 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/ProfileSubscription.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/ProfileSubscription.kt @@ -54,9 +54,21 @@ class ProfileSubscription( private val relaysPerSubId = mutableMapOf>() private val timeoutJobs = mutableMapOf() + @Volatile + private var registered = false + init { - // listens until the app crashes. - client.addConnectionListener(this) + if (!registered) { + client.addConnectionListener(this) + registered = true + } + } + + fun dispose() { + if (registered) { + client.removeConnectionListener(this) + registered = false + } } override fun onIncomingMessage(relay: IRelayClient, msgStr: String, msg: Message) { diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/ZapstoreUpdater.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/ZapstoreUpdater.kt index 979fb01f..12b079c7 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/ZapstoreUpdater.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/ZapstoreUpdater.kt @@ -66,8 +66,21 @@ class ZapstoreUpdater( UPDATE_RELAY_URLS.mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) } private var timeoutJob: Job? = null + @Volatile + private var registered = false + init { - client.addConnectionListener(this) + if (!registered) { + client.addConnectionListener(this) + registered = true + } + } + + fun dispose() { + if (registered) { + client.removeConnectionListener(this) + registered = false + } } override fun onIncomingMessage(relay: IRelayClient, msgStr: String, msg: Message) { diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/ActivitiesScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/ActivitiesScreen.kt index 0fef1df1..d2910e29 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/ActivitiesScreen.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/ActivitiesScreen.kt @@ -327,7 +327,7 @@ fun ApplicationName( LaunchedEffect(key) { val cacheKey = "${account.npub.toShortenHex()}-$key" - val cached = ApplicationNameCache.names[cacheKey] + val cached = ApplicationNameCache[cacheKey] if (cached != null) { name = cached } else { @@ -336,7 +336,7 @@ fun ApplicationName( } if (appName != null) { name = appName - ApplicationNameCache.names[cacheKey] = appName + ApplicationNameCache[cacheKey] = appName } } } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/ToastManager.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/ToastManager.kt index 4adb376e..7c283926 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/ToastManager.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/ToastManager.kt @@ -1,10 +1,8 @@ package com.greenart7c3.nostrsigner.ui import androidx.compose.runtime.Immutable -import com.greenart7c3.nostrsigner.Amber import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.launch @Immutable open class ToastMsg @@ -31,17 +29,19 @@ class ResourceToastMsg( ) : ToastMsg() object ToastManager { + // DROP_OLDEST means tryEmit always succeeds, so callers no longer need to + // spawn a coroutine on the application scope just to push a message. val toasts = MutableSharedFlow(0, 3, onBufferOverflow = BufferOverflow.DROP_OLDEST) fun clearToasts() { - Amber.instance.applicationIOScope.launch { toasts.emit(null) } + toasts.tryEmit(null) } fun toast( title: String, message: String, ) { - Amber.instance.applicationIOScope.launch { toasts.emit(StringToastMsg(title, message)) } + toasts.tryEmit(StringToastMsg(title, message)) } fun toast( @@ -49,7 +49,7 @@ object ToastManager { message: String, onOk: () -> Unit, ) { - Amber.instance.applicationIOScope.launch { toasts.emit(ConfirmationToastMsg(title, message, onOk)) } + toasts.tryEmit(ConfirmationToastMsg(title, message, onOk)) } fun toast( @@ -58,6 +58,6 @@ object ToastManager { onAccept: () -> Unit, onReject: () -> Unit, ) { - Amber.instance.applicationIOScope.launch { toasts.emit(AcceptRejectToastMsg(title, message, onAccept, onReject)) } + toasts.tryEmit(AcceptRejectToastMsg(title, message, onAccept, onReject)) } } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/BunkerMultiEventHomeScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/BunkerMultiEventHomeScreen.kt index 2ee4b85b..4f17a91c 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/BunkerMultiEventHomeScreen.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/components/BunkerMultiEventHomeScreen.kt @@ -82,7 +82,7 @@ fun BunkerMultiEventHomeScreen( val key = bunkerRequests.first().localKey var rememberType by remember { mutableStateOf(RememberType.NEVER) } var relayAuthScope by remember { mutableStateOf(RelayAuthScope.SPECIFIC) } - var appName by remember { mutableStateOf(ApplicationNameCache.names["$localAccount-$key"] ?: key.toShortenHex()) } + var appName by remember { mutableStateOf(ApplicationNameCache["$localAccount-$key"] ?: key.toShortenHex()) } LaunchedEffect(Unit) { MultiEventScreenIntents.checkedStates.clear() @@ -97,14 +97,14 @@ fun BunkerMultiEventHomeScreen( bunkerRequests.first().currentAccount, )?.npub?.toShortenHex() ?: "" - if (ApplicationNameCache.names["$localAccount-$key"] == null) { + if (ApplicationNameCache["$localAccount-$key"] == null) { val app = Amber.instance.getDatabase(accountParam.npub).dao().getByKey(key) app?.let { appName = it.application.name - ApplicationNameCache.names["$localAccount-$key"] = it.application.name + ApplicationNameCache["$localAccount-$key"] = it.application.name } } else { - ApplicationNameCache.names["$localAccount-$key"]?.let { + ApplicationNameCache["$localAccount-$key"]?.let { appName = it } }