mirror of
https://github.com/divkit/divkit.git
synced 2026-05-07 20:02:32 +00:00
fix multithreading crashes
commit_hash:4646c8e9f15726e6cb5214e54291a816a8de7916
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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<OverflowMenuSubscriber.Listener>()
|
||||
private val divDataChangedObservers = mutableListOf<DivDataChangedObserver>()
|
||||
private val persistentDivDataObservers = ObserverList<PersistentDivDataObserver>()
|
||||
private val viewToDivBindings = Collections.synchronizedMap(WeakHashMap<View, Div>())
|
||||
private val viewToDivBindings = WeakHashMap<View, Div>()
|
||||
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 {
|
||||
|
||||
+4
-3
@@ -13,19 +13,20 @@ internal class PagerIndicatorConnector @Inject constructor() {
|
||||
val pagerDiv: DivPager,
|
||||
)
|
||||
|
||||
private val mutex = Any()
|
||||
private val pagers = mutableMapOf<DivPager, DivPagerView>()
|
||||
private val indicators = mutableListOf<IndicatorData>()
|
||||
|
||||
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()
|
||||
}
|
||||
|
||||
+12
-8
@@ -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<Throwable>, warnings: List<Throwable>) -> 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<ErrorObserver>()
|
||||
private val runtimeErrors = mutableListOf<Throwable>()
|
||||
private var parsingErrors = emptyList<Throwable>()
|
||||
@@ -24,7 +28,7 @@ internal class ErrorCollector(
|
||||
private var errors = mutableListOf<Throwable>()
|
||||
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<Throwable> = warnings.listIterator()
|
||||
fun getWarnings(): Iterable<Throwable> = 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()
|
||||
}
|
||||
|
||||
@@ -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()))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 <K, V> arrayMap(): MutableMap<K, V> = ArrayMap<K, V>()
|
||||
@@ -63,3 +64,20 @@ public inline fun <T> List<T>?.compareNullableWith(other: List<T>?, comparator:
|
||||
|
||||
return compareWith(other, comparator)
|
||||
}
|
||||
|
||||
@InternalApi
|
||||
public fun <K, V> WeakHashMap<out K, V>.toMapSafe(): Map<K, V> {
|
||||
if (isEmpty()) {
|
||||
return emptyMap()
|
||||
}
|
||||
|
||||
val result = mutableMapOf<K, V>()
|
||||
val iterator = entries.iterator()
|
||||
try {
|
||||
while (iterator.hasNext()) {
|
||||
val entry = iterator.next()
|
||||
result[entry.key] = entry.value
|
||||
}
|
||||
} catch (e: NoSuchElementException) { }
|
||||
return result
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user