Skip to content

Commit

Permalink
Fix cancellation in ConstraintsSizeResolver#size()
Browse files Browse the repository at this point in the history
`suspendCoroutine` does not implement cancellation from kotlinx.coroutines, which might result in Recomposer hanging while awaiting cancellation.

See [reported Compose issue](https://issuetracker.google.com/390456358) for more details.

Test: ConstraintsSizeResolverTest
  • Loading branch information
ShikaSD committed Mar 3, 2025
1 parent 663222d commit 58ce594
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ import coil3.compose.internal.toSize
import coil3.size.Size
import coil3.size.SizeResolver
import kotlin.coroutines.Continuation
import kotlin.coroutines.cancellation.CancellationException
import kotlin.coroutines.resume
import kotlin.coroutines.suspendCoroutine
import kotlinx.coroutines.suspendCancellableCoroutine

/**
* Create a [ConstraintsSizeResolver] and remember it.
Expand All @@ -31,13 +32,19 @@ fun rememberConstraintsSizeResolver(): ConstraintsSizeResolver {
@Stable
class ConstraintsSizeResolver : SizeResolver, LayoutModifier {
private var latestConstraints = ZeroConstraints
private var continuation: Continuation<Unit>? = null
private var continuations = mutableListOf<Continuation<Unit>>()

override suspend fun size(): Size {
if (latestConstraints.isZero) {
val oldContinuation = continuation
suspendCoroutine { continuation = it }
oldContinuation?.resume(Unit)
var continuation: Continuation<Unit>? = null
try {
suspendCancellableCoroutine<Unit> {
continuation = it
continuations.add(it)
}
} finally {
continuations.remove(continuation)
}
}
return latestConstraints.toSize()
}
Expand All @@ -59,8 +66,9 @@ class ConstraintsSizeResolver : SizeResolver, LayoutModifier {
// Cache the latest constraints.
latestConstraints = constraints
if (!constraints.isZero) {
continuation?.resume(Unit)
continuation = null
val c = continuations
continuations = mutableListOf()
c.forEach { it.resume(Unit) }
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package coil3.compose

import androidx.compose.ui.unit.Constraints
import coil3.compose.internal.toSize
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.async
import kotlinx.coroutines.cancelAndJoin
import kotlinx.coroutines.test.runTest

class ConstraintsSizeResolverTest {
@Test
fun `resolver size is cancellable`() = runTest {
val resolver = ConstraintsSizeResolver()

val deferred = async(start = CoroutineStart.UNDISPATCHED) {
resolver.size()
}

deferred.cancelAndJoin()
assertEquals(true, deferred.isCompleted)
}

@Test
fun `resolver size is cancellable with two suspension points`() = runTest {
val resolver = ConstraintsSizeResolver()

val deferred1 = async(start = CoroutineStart.UNDISPATCHED) {
resolver.size()
}

val deferred2 = async(start = CoroutineStart.UNDISPATCHED) {
resolver.size()
}

deferred2.cancelAndJoin()
assertEquals(true, deferred2.isCompleted)
assertEquals(false, deferred1.isCancelled)
assertEquals(false, deferred1.isCompleted)

val c = Constraints()
resolver.setConstraints(c)
val result = deferred1.await()
assertEquals(c.toSize(), result)
}

@Test
fun `resolver size is cancellable with many suspension points`() = runTest {
val resolver = ConstraintsSizeResolver()

val deferred1 = async(start = CoroutineStart.UNDISPATCHED) {
resolver.size()
}

val deferred2 = async(start = CoroutineStart.UNDISPATCHED) {
resolver.size()
}

val deferred3 = async(start = CoroutineStart.UNDISPATCHED) {
resolver.size()
}

deferred2.cancelAndJoin()
assertEquals(true, deferred2.isCompleted)
assertEquals(false, deferred1.isCancelled)
assertEquals(false, deferred3.isCancelled)

val c = Constraints()
resolver.setConstraints(c)
val result1 = deferred1.await()
val result3 = deferred3.await()
assertEquals(c.toSize(), result1)
assertEquals(c.toSize(), result3)
}
}

0 comments on commit 58ce594

Please sign in to comment.