From 6da5c11731aa352eb6cf181acf900706f2d1d83a Mon Sep 17 00:00:00 2001 From: Koen J Date: Thu, 12 Dec 2024 14:13:24 +0100 Subject: [PATCH] Fixed concurrent modification crash in ServiceRecordAggregator and link clicking in scroll. --- .../mdns/ServiceRecordAggregator.kt | 85 ++++++++++--------- .../others/PlatformLinkMovementMethod.kt | 8 +- .../views/behavior/NonScrollingTextView.kt | 8 +- 3 files changed, 56 insertions(+), 45 deletions(-) diff --git a/app/src/main/java/com/futo/platformplayer/mdns/ServiceRecordAggregator.kt b/app/src/main/java/com/futo/platformplayer/mdns/ServiceRecordAggregator.kt index bb4d1007..5292d375 100644 --- a/app/src/main/java/com/futo/platformplayer/mdns/ServiceRecordAggregator.kt +++ b/app/src/main/java/com/futo/platformplayer/mdns/ServiceRecordAggregator.kt @@ -55,21 +55,25 @@ class ServiceRecordAggregator { if (_cts != null) throw Exception("Already started.") _cts = CoroutineScope(Dispatchers.Default).launch { - while (isActive) { - val now = Date() - synchronized(_currentServices) { - _cachedAddressRecords.forEach { it.value.removeAll { record -> now.after(record.expirationTime) } } - _cachedTxtRecords.entries.removeIf { now.after(it.value.expirationTime) } - _cachedSrvRecords.entries.removeIf { now.after(it.value.expirationTime) } - _cachedPtrRecords.forEach { it.value.removeAll { record -> now.after(record.expirationTime) } } + try { + while (isActive) { + val now = Date() + synchronized(_currentServices) { + _cachedAddressRecords.forEach { it.value.removeAll { record -> now.after(record.expirationTime) } } + _cachedTxtRecords.entries.removeIf { now.after(it.value.expirationTime) } + _cachedSrvRecords.entries.removeIf { now.after(it.value.expirationTime) } + _cachedPtrRecords.forEach { it.value.removeAll { record -> now.after(record.expirationTime) } } - val newServices = getCurrentServices() - _currentServices.clear() - _currentServices.addAll(newServices) + val newServices = getCurrentServices() + _currentServices.clear() + _currentServices.addAll(newServices) + } + + onServicesUpdated?.invoke(_currentServices.toList()) + delay(5000) } - - onServicesUpdated?.invoke(_currentServices.toList()) - delay(5000) + } catch (e: Throwable) { + Logger.e(TAG, "Unexpected failure in MDNS loop", e) } } } @@ -83,6 +87,7 @@ class ServiceRecordAggregator { } fun add(packet: DnsPacket) { + val currentServices: List val dnsResourceRecords = packet.answers + packet.additionals + packet.authorities val txtRecords = dnsResourceRecords.filter { it.type == ResourceRecordType.TXT.value.toInt() }.map { it to it.getDataReader().readTXTRecord() } val aRecords = dnsResourceRecords.filter { it.type == ResourceRecordType.A.value.toInt() }.map { it to it.getDataReader().readARecord() } @@ -99,35 +104,33 @@ class ServiceRecordAggregator { aaaaRecords.forEach { builder.appendLine("AAAA ${it.first.name} ${it.first.type} ${it.first.clazz} TTL ${it.first.timeToLive}: ${it.second.address}") } Logger.i(TAG, "$builder")*/ - val currentServices: MutableList - ptrRecords.forEach { record -> - val cachedPtrRecord = _cachedPtrRecords.getOrPut(record.first.name) { mutableListOf() } - val newPtrRecord = CachedDnsPtrRecord(Date(System.currentTimeMillis() + record.first.timeToLive.toLong() * 1000L), record.second.domainName) - cachedPtrRecord.replaceOrAdd(newPtrRecord) { it.target == record.second.domainName } - } - - aRecords.forEach { aRecord -> - val cachedARecord = _cachedAddressRecords.getOrPut(aRecord.first.name) { mutableListOf() } - val newARecord = CachedDnsAddressRecord(Date(System.currentTimeMillis() + aRecord.first.timeToLive.toLong() * 1000L), aRecord.second.address) - cachedARecord.replaceOrAdd(newARecord) { it.address == newARecord.address } - } - - aaaaRecords.forEach { aaaaRecord -> - val cachedAaaaRecord = _cachedAddressRecords.getOrPut(aaaaRecord.first.name) { mutableListOf() } - val newAaaaRecord = CachedDnsAddressRecord(Date(System.currentTimeMillis() + aaaaRecord.first.timeToLive.toLong() * 1000L), aaaaRecord.second.address) - cachedAaaaRecord.replaceOrAdd(newAaaaRecord) { it.address == newAaaaRecord.address } - } - - txtRecords.forEach { txtRecord -> - _cachedTxtRecords[txtRecord.first.name] = CachedDnsTxtRecord(Date(System.currentTimeMillis() + txtRecord.first.timeToLive.toLong() * 1000L), txtRecord.second.texts) - } - - srvRecords.forEach { srvRecord -> - _cachedSrvRecords[srvRecord.first.name] = CachedDnsSrvRecord(Date(System.currentTimeMillis() + srvRecord.first.timeToLive.toLong() * 1000L), srvRecord.second) - } - - //TODO: Maybe this can be debounced? synchronized(this._currentServices) { + ptrRecords.forEach { record -> + val cachedPtrRecord = _cachedPtrRecords.getOrPut(record.first.name) { mutableListOf() } + val newPtrRecord = CachedDnsPtrRecord(Date(System.currentTimeMillis() + record.first.timeToLive.toLong() * 1000L), record.second.domainName) + cachedPtrRecord.replaceOrAdd(newPtrRecord) { it.target == record.second.domainName } + } + + aRecords.forEach { aRecord -> + val cachedARecord = _cachedAddressRecords.getOrPut(aRecord.first.name) { mutableListOf() } + val newARecord = CachedDnsAddressRecord(Date(System.currentTimeMillis() + aRecord.first.timeToLive.toLong() * 1000L), aRecord.second.address) + cachedARecord.replaceOrAdd(newARecord) { it.address == newARecord.address } + } + + aaaaRecords.forEach { aaaaRecord -> + val cachedAaaaRecord = _cachedAddressRecords.getOrPut(aaaaRecord.first.name) { mutableListOf() } + val newAaaaRecord = CachedDnsAddressRecord(Date(System.currentTimeMillis() + aaaaRecord.first.timeToLive.toLong() * 1000L), aaaaRecord.second.address) + cachedAaaaRecord.replaceOrAdd(newAaaaRecord) { it.address == newAaaaRecord.address } + } + + txtRecords.forEach { txtRecord -> + _cachedTxtRecords[txtRecord.first.name] = CachedDnsTxtRecord(Date(System.currentTimeMillis() + txtRecord.first.timeToLive.toLong() * 1000L), txtRecord.second.texts) + } + + srvRecords.forEach { srvRecord -> + _cachedSrvRecords[srvRecord.first.name] = CachedDnsSrvRecord(Date(System.currentTimeMillis() + srvRecord.first.timeToLive.toLong() * 1000L), srvRecord.second) + } + currentServices = getCurrentServices() this._currentServices.clear() this._currentServices.addAll(currentServices) diff --git a/app/src/main/java/com/futo/platformplayer/others/PlatformLinkMovementMethod.kt b/app/src/main/java/com/futo/platformplayer/others/PlatformLinkMovementMethod.kt index 9faf9433..d524b7cf 100644 --- a/app/src/main/java/com/futo/platformplayer/others/PlatformLinkMovementMethod.kt +++ b/app/src/main/java/com/futo/platformplayer/others/PlatformLinkMovementMethod.kt @@ -59,7 +59,7 @@ class PlatformLinkMovementMethod(private val _context: Context) : LinkMovementMe if (linkPressed && pressedLinks != null) { val dx = event.x - downX val dy = event.y - downY - if (Math.abs(dx) <= touchSlop && Math.abs(dy) <= touchSlop) { + if (Math.abs(dx) <= touchSlop && Math.abs(dy) <= touchSlop && isTouchInside(widget, event)) { runBlocking { for (link in pressedLinks!!) { Logger.i(TAG) { "Link clicked '${link.url}'." } @@ -101,7 +101,7 @@ class PlatformLinkMovementMethod(private val _context: Context) : LinkMovementMe } } - return super.onTouchEvent(widget, buffer, event) + return false } private fun findLinksAtTouchPosition(widget: TextView, buffer: Spannable, event: MotionEvent): Array { @@ -114,6 +114,10 @@ class PlatformLinkMovementMethod(private val _context: Context) : LinkMovementMe return buffer.getSpans(off, off, URLSpan::class.java) } + private fun isTouchInside(widget: TextView, event: MotionEvent): Boolean { + return event.x >= 0 && event.x <= widget.width && event.y >= 0 && event.y <= widget.height + } + companion object { const val TAG = "PlatformLinkMovementMethod" } diff --git a/app/src/main/java/com/futo/platformplayer/views/behavior/NonScrollingTextView.kt b/app/src/main/java/com/futo/platformplayer/views/behavior/NonScrollingTextView.kt index 402940ad..cf599176 100644 --- a/app/src/main/java/com/futo/platformplayer/views/behavior/NonScrollingTextView.kt +++ b/app/src/main/java/com/futo/platformplayer/views/behavior/NonScrollingTextView.kt @@ -76,7 +76,7 @@ class NonScrollingTextView : androidx.appcompat.widget.AppCompatTextView { if (linkPressed && _lastTouchedLinks != null) { val dx = event.x - downX val dy = event.y - downY - if (Math.abs(dx) <= touchSlop && Math.abs(dy) <= touchSlop) { + if (Math.abs(dx) <= touchSlop && Math.abs(dy) <= touchSlop && isTouchInside(event)) { runBlocking { for (link in _lastTouchedLinks!!) { Logger.i(PlatformLinkMovementMethod.TAG) { "Link clicked '${link.url}'." } @@ -119,7 +119,11 @@ class NonScrollingTextView : androidx.appcompat.widget.AppCompatTextView { } } - return super.onTouchEvent(event) + return false + } + + private fun isTouchInside(event: MotionEvent): Boolean { + return event.x >= 0 && event.x <= width && event.y >= 0 && event.y <= height } companion object {