From d16ccd33d250accb65958d622f4affef2e08c276 Mon Sep 17 00:00:00 2001 From: grechka62 Date: Fri, 20 Mar 2026 11:17:48 +0300 Subject: [PATCH] fix multithreading crashes commit_hash:4646c8e9f15726e6cb5214e54291a816a8de7916 --- CHANGELOG.md | 7 +++++ .../com/yandex/div/core/view2/Div2View.kt | 31 +++++++++++++------ .../divs/pager/PagerIndicatorConnector.kt | 7 +++-- .../div/core/view2/errors/ErrorCollector.kt | 20 +++++++----- .../yandex/div/core/view2/Div2RebindTest.kt | 4 +-- .../yandex/div/internal/util/Collections.kt | 18 +++++++++++ 6 files changed, 65 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78a3af081..4d627d60d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 32.37.1 + +# Android Client: +* Fixed `NoSuchElementException` at `Div2View.trackChildrenVisibility()` and `Div2View.discardChildrenVisibility()` calls. +* Fixed `ConcurrentModificationException` at `PagerIndicatorConnector.attach()` and `ErrorCollector.notifyObservers()` calls. + + ## 32.37.0 # Android Client: diff --git a/client/android/div/src/main/java/com/yandex/div/core/view2/Div2View.kt b/client/android/div/src/main/java/com/yandex/div/core/view2/Div2View.kt index e310ff780..dd908496f 100644 --- a/client/android/div/src/main/java/com/yandex/div/core/view2/Div2View.kt +++ b/client/android/div/src/main/java/com/yandex/div/core/view2/Div2View.kt @@ -84,10 +84,10 @@ import com.yandex.div.internal.KAssert import com.yandex.div.internal.KLog import com.yandex.div.internal.core.DivItemBuilderResult import com.yandex.div.internal.core.VariableMutationHandler -import com.yandex.div.internal.util.Clock import com.yandex.div.internal.util.UiThreadHandler.Companion.executeOnMainThreadBlocking import com.yandex.div.internal.util.hasScrollableChildUnder import com.yandex.div.internal.util.immutableCopy +import com.yandex.div.internal.util.toMapSafe import com.yandex.div.internal.widget.FrameContainerLayout import com.yandex.div.internal.widget.menu.OverflowMenuSubscriber import com.yandex.div.json.expressions.ExpressionResolver @@ -98,7 +98,6 @@ import com.yandex.div2.DivAction import com.yandex.div2.DivData import com.yandex.div2.DivPatch import com.yandex.div2.DivTransitionSelector -import java.util.Collections import java.util.UUID import java.util.WeakHashMap @@ -125,7 +124,7 @@ class Div2View private constructor( private val overflowMenuListeners = mutableListOf() private val divDataChangedObservers = mutableListOf() private val persistentDivDataObservers = ObserverList() - private val viewToDivBindings = Collections.synchronizedMap(WeakHashMap()) + private val viewToDivBindings = WeakHashMap() private val bulkActionsHandler = BulkActionHandler() private val divVideoActionHandler: DivVideoActionHandler get() = div2Component.divVideoActionHandler @@ -637,7 +636,10 @@ class Div2View private constructor( fun trackChildrenVisibility(): Unit = bindingDispatcher.runWithinBindingContext { val visibilityActionTracker = div2Component.visibilityActionTracker - viewToDivBindings.toMap().forEach { (view, div) -> + val bindingsSnapshot = synchronized(viewToDivBindings) { + viewToDivBindings.toMapSafe() + } + bindingsSnapshot.forEach { (view, div) -> view.bindingContext?.expressionResolver?.let { if (ViewCompat.isAttachedToWindow(view)) { visibilityActionTracker.trackVisibilityActionsOf(this, it, view, div) @@ -650,7 +652,10 @@ class Div2View private constructor( private fun discardChildrenVisibility() { val visibilityActionTracker = div2Component.visibilityActionTracker - viewToDivBindings.toMap().forEach { (view, div) -> + val bindingsSnapshot = synchronized(viewToDivBindings) { + viewToDivBindings.toMapSafe() + } + bindingsSnapshot.forEach { (view, div) -> view.bindingContext?.expressionResolver?.let { visibilityActionTracker.trackVisibilityActionsOf(this, it, null, div) } @@ -795,7 +800,9 @@ class Div2View private constructor( } private fun stopLoadAndSubscriptions() { - viewToDivBindings.clear() + synchronized(viewToDivBindings) { + viewToDivBindings.clear() + } cancelTooltips() // Depends on children, should be called before removing them clearSubscriptions() divDataChangedObservers.clear() @@ -1250,10 +1257,14 @@ class Div2View private constructor( } internal fun bindViewToDiv(view: View, div: Div) { - viewToDivBindings[view] = div + synchronized(viewToDivBindings) { + viewToDivBindings[view] = div + } } - internal fun takeBindingDiv(view: View) = viewToDivBindings[view] + internal fun takeBindingDiv(view: View) = synchronized(viewToDivBindings) { + viewToDivBindings[view] + } /** * @return exception if setting variable failed, null otherwise. @@ -1281,7 +1292,9 @@ class Div2View private constructor( return divVideoActionHandler.handleAction(this, divId, command, expressionResolver) } - internal fun unbindViewFromDiv(view: View): Div? = viewToDivBindings.remove(view) + internal fun unbindViewFromDiv(view: View): Div? = synchronized(viewToDivBindings) { + viewToDivBindings.remove(view) + } private fun rebind(newData: DivData, isAutoanimations: Boolean, reporter: SimpleRebindReporter) { try { diff --git a/client/android/div/src/main/java/com/yandex/div/core/view2/divs/pager/PagerIndicatorConnector.kt b/client/android/div/src/main/java/com/yandex/div/core/view2/divs/pager/PagerIndicatorConnector.kt index 055234bbd..e8744d3b1 100644 --- a/client/android/div/src/main/java/com/yandex/div/core/view2/divs/pager/PagerIndicatorConnector.kt +++ b/client/android/div/src/main/java/com/yandex/div/core/view2/divs/pager/PagerIndicatorConnector.kt @@ -13,19 +13,20 @@ internal class PagerIndicatorConnector @Inject constructor() { val pagerDiv: DivPager, ) + private val mutex = Any() private val pagers = mutableMapOf() private val indicators = mutableListOf() - internal fun submitPager(pagerView: DivPagerView, pagerDiv: DivPager) { + internal fun submitPager(pagerView: DivPagerView, pagerDiv: DivPager): Unit = synchronized(mutex) { pagers[pagerDiv] = pagerView } - internal fun submitIndicator(indicatorView: DivPagerIndicatorView, pagerDiv: DivPager) { + internal fun submitIndicator(indicatorView: DivPagerIndicatorView, pagerDiv: DivPager): Unit = synchronized(mutex) { val indicatorData = IndicatorData(indicatorView, pagerDiv) indicators.add(indicatorData) } - internal fun attach() { + internal fun attach(): Unit = synchronized(mutex) { pagers.forEach { it.value.clearChangePageCallbackForIndicators() } diff --git a/client/android/div/src/main/java/com/yandex/div/core/view2/errors/ErrorCollector.kt b/client/android/div/src/main/java/com/yandex/div/core/view2/errors/ErrorCollector.kt index 22ccc054f..0df20e709 100644 --- a/client/android/div/src/main/java/com/yandex/div/core/view2/errors/ErrorCollector.kt +++ b/client/android/div/src/main/java/com/yandex/div/core/view2/errors/ErrorCollector.kt @@ -1,21 +1,25 @@ package com.yandex.div.core.view2.errors +import androidx.annotation.AnyThread import com.yandex.div.DivDataTag import com.yandex.div.core.Disposable import com.yandex.div.core.DivErrorsReporter import com.yandex.div.core.annotations.Mockable import com.yandex.div.json.ParsingErrorLogger import com.yandex.div2.DivData -import java.lang.Exception typealias ErrorObserver = (errors: List, warnings: List) -> Unit @Mockable +@AnyThread internal class ErrorCollector( internal val divData: DivData?, private val divDataTag: DivDataTag, private val divErrorsReporter: DivErrorsReporter, ) : ParsingErrorLogger { + + private val mutex = Any() + private val observers = mutableSetOf() private val runtimeErrors = mutableListOf() private var parsingErrors = emptyList() @@ -24,7 +28,7 @@ internal class ErrorCollector( private var errors = mutableListOf() private var errorsAreValid = true - fun logError(e: Throwable) { + fun logError(e: Throwable): Unit = synchronized(mutex) { runtimeErrors.add(e) divErrorsReporter.onRuntimeError(divData, divDataTag, e) notifyObservers() @@ -32,21 +36,21 @@ internal class ErrorCollector( override fun logError(e: Exception) = logError(e as Throwable) - fun logWarning(warning: Throwable) { + fun logWarning(warning: Throwable): Unit = synchronized(mutex) { warnings.add(warning) divErrorsReporter.onWarning(divData, divDataTag, warning) notifyObservers() } - fun getWarnings(): Iterator = warnings.listIterator() + fun getWarnings(): Iterable = synchronized(mutex) { warnings.toList() } - fun cleanRuntimeWarningsAndErrors() { + fun cleanRuntimeWarningsAndErrors(): Unit = synchronized(mutex) { warnings.clear() runtimeErrors.clear() notifyObservers() } - private fun notifyObservers() { + private fun notifyObservers(): Unit = synchronized(mutex) { errorsAreValid = false if (observers.isEmpty()) return rebuildErrors() @@ -61,14 +65,14 @@ internal class ErrorCollector( errorsAreValid = true } - fun observeAndGet(observer: ErrorObserver): Disposable { + fun observeAndGet(observer: ErrorObserver): Disposable = synchronized(mutex) { observers.add(observer) rebuildErrors() observer(errors, warnings) return Disposable { observers.remove(observer) } } - fun attachParsingErrors() { + fun attachParsingErrors(): Unit = synchronized(mutex) { parsingErrors = divData?.parsingErrors ?: emptyList() notifyObservers() } diff --git a/client/android/div/src/test/java/com/yandex/div/core/view2/Div2RebindTest.kt b/client/android/div/src/test/java/com/yandex/div/core/view2/Div2RebindTest.kt index c6b9825e6..491f7b52a 100644 --- a/client/android/div/src/test/java/com/yandex/div/core/view2/Div2RebindTest.kt +++ b/client/android/div/src/test/java/com/yandex/div/core/view2/Div2RebindTest.kt @@ -131,9 +131,9 @@ class Div2RebindTest { val errorCollectors = div2View.viewComponent.errorCollectors div2View.setData(oldData, tag) - assertEquals(1, Iterators.size(errorCollectors.getOrCreate(tag, oldData).getWarnings())) + assertEquals(1, Iterators.size(errorCollectors.getOrCreate(tag, oldData).getWarnings().iterator())) div2View.setData(newData, oldData, tag) - assertEquals(0, Iterators.size(errorCollectors.getOrCreate(tag, newData).getWarnings())) + assertEquals(0, Iterators.size(errorCollectors.getOrCreate(tag, newData).getWarnings().iterator())) } } diff --git a/client/android/utils/src/main/java/com/yandex/div/internal/util/Collections.kt b/client/android/utils/src/main/java/com/yandex/div/internal/util/Collections.kt index bec6bfbcf..9c0e7168f 100644 --- a/client/android/utils/src/main/java/com/yandex/div/internal/util/Collections.kt +++ b/client/android/utils/src/main/java/com/yandex/div/internal/util/Collections.kt @@ -3,6 +3,7 @@ package com.yandex.div.internal.util import androidx.collection.ArrayMap import com.yandex.div.core.annotations.InternalApi import java.util.Collections +import java.util.WeakHashMap @InternalApi public fun arrayMap(): MutableMap = ArrayMap() @@ -63,3 +64,20 @@ public inline fun List?.compareNullableWith(other: List?, comparator: return compareWith(other, comparator) } + +@InternalApi +public fun WeakHashMap.toMapSafe(): Map { + if (isEmpty()) { + return emptyMap() + } + + val result = mutableMapOf() + val iterator = entries.iterator() + try { + while (iterator.hasNext()) { + val entry = iterator.next() + result[entry.key] = entry.value + } + } catch (e: NoSuchElementException) { } + return result +}