Make PausingDispatcher behave like the main dispatcher when yielding
The default android main dispatcher supports yielding inside the
immediate dispatcher (see https://github.com/Kotlin/kotlinx.coroutines/commit/3ab34d808b5aa2c8673754bd43e4879b9d4af8c9).
The pausing dispatcher has not worked liked this and yield() calls are
essentially a no-op. This change fixes this and will respect yield()
calls.
Fixes: 168777346
Test: PausingDispatcherTest
Change-Id: Iffa89a264f8566e584eccc1a6ce844863cca2ad9
diff --git a/lifecycle/lifecycle-runtime-ktx/src/androidTest/java/androidx/lifecycle/Expectations.kt b/lifecycle/lifecycle-runtime-ktx/src/androidTest/java/androidx/lifecycle/Expectations.kt
index 395a840..4194da9 100644
--- a/lifecycle/lifecycle-runtime-ktx/src/androidTest/java/androidx/lifecycle/Expectations.kt
+++ b/lifecycle/lifecycle-runtime-ktx/src/androidTest/java/androidx/lifecycle/Expectations.kt
@@ -25,7 +25,7 @@
* to track execution order.
*/
class Expectations {
- private var counter = AtomicInteger(0)
+ private val counter = AtomicInteger(0)
fun expect(expected: Int) {
val order = counter.incrementAndGet()
diff --git a/lifecycle/lifecycle-runtime-ktx/src/androidTest/java/androidx/lifecycle/PausingDispatcherTest.kt b/lifecycle/lifecycle-runtime-ktx/src/androidTest/java/androidx/lifecycle/PausingDispatcherTest.kt
index 7b0d811..c9a1586 100644
--- a/lifecycle/lifecycle-runtime-ktx/src/androidTest/java/androidx/lifecycle/PausingDispatcherTest.kt
+++ b/lifecycle/lifecycle-runtime-ktx/src/androidTest/java/androidx/lifecycle/PausingDispatcherTest.kt
@@ -21,12 +21,14 @@
import androidx.test.filters.SmallTest
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineExceptionHandler
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.InternalCoroutinesApi
import kotlinx.coroutines.Job
+import kotlinx.coroutines.MainCoroutineDispatcher
import kotlinx.coroutines.asCoroutineDispatcher
import kotlinx.coroutines.async
import kotlinx.coroutines.cancelAndJoin
@@ -47,6 +49,7 @@
import java.util.concurrent.CountDownLatch
import java.util.concurrent.Executors
import java.util.concurrent.TimeUnit
+import kotlin.coroutines.CoroutineContext
@InternalCoroutinesApi
@SmallTest
@@ -74,13 +77,45 @@
@OptIn(ExperimentalCoroutinesApi::class)
@Before
fun updateMainHandlerAndDispatcher() {
- Dispatchers.setMain(mainExecutor.asCoroutineDispatcher())
+ Dispatchers.setMain(mainExecutor.asCoroutineDispatcher().asMain())
runBlocking(Dispatchers.Main) {
// extract the main thread to field for assertions
mainThread = Thread.currentThread()
}
}
+ /**
+ * Wraps the receiver as a [MainCoroutineDispatcher].
+ *
+ * This is needed because [PausingDispatcher] is uses [Dispatchers.Main.immediate] internally
+ * and simply calling [Dispatchers.setMain] with a coroutine dispatcher leads to a different
+ * behavior. Namely that [CoroutineDispatcher.isDispatchNeeded] always returns `true` which
+ * is not an accurate emulation of how the real android dispatcher behaves.
+ *
+ * The returned dispatcher will mimic the behavior of the default Android main dispatcher in
+ * that its immediate counterpart will return `false` from
+ * [CoroutineDispatcher.isDispatchNeeded] if the current thread is [mainThread].
+ *
+ * The receiver must be backed by a single thread for this to work properly.
+ *
+ * Ideally the real android coroutine dispatcher would be used but this will require a somewhat
+ * refactoring of these tests so for now this is a good trade-off.
+ */
+ private fun CoroutineDispatcher.asMain(immediate: Boolean = false): MainCoroutineDispatcher {
+ return object : MainCoroutineDispatcher() {
+ override val immediate: MainCoroutineDispatcher =
+ if (immediate) this else asMain(immediate = true)
+
+ override fun dispatch(context: CoroutineContext, block: Runnable) {
+ [email protected](context, block)
+ }
+
+ override fun isDispatchNeeded(context: CoroutineContext): Boolean {
+ return !immediate || Thread.currentThread() != mainThread
+ }
+ }
+ }
+
@OptIn(ExperimentalCoroutinesApi::class)
@After
fun clearHandlerAndDispatcher() {
@@ -124,8 +159,10 @@
assertThat(result).isEqualTo(3)
}
+ @OptIn(ExperimentalCoroutinesApi::class)
@Test
fun yieldTest() {
+ Dispatchers.resetMain()
runBlocking(Dispatchers.Main) {
owner.whenResumed {
expectations.expect(1)
@@ -143,6 +180,44 @@
}
}
+ @OptIn(ExperimentalCoroutinesApi::class)
+ @Test
+ fun yieldImmediateTest() {
+ Dispatchers.resetMain()
+ runBlocking(Dispatchers.Main.immediate) {
+ val job = owner.lifecycleScope.launchWhenResumed {
+ expectations.expect(1)
+ yield()
+ expectations.expect(3)
+ }
+ expectations.expect(2)
+ yield()
+ expectations.expect(4)
+ expectations.expectTotal(4)
+ assertThat(job.isActive).isFalse()
+ }
+ }
+
+ @Test
+ fun yieldLoop() {
+ runBlocking(Dispatchers.Main.immediate) {
+ var condition = false
+ val job = owner.lifecycleScope.launchWhenResumed {
+ expectations.expect(1)
+ while (!condition) {
+ expectations.expect(2)
+ yield()
+ expectations.expect(3)
+ }
+ }
+ expectations.expectTotal(2)
+ condition = true
+ yield()
+ assertThat(job.isActive).isFalse()
+ expectations.expectTotal(3)
+ }
+ }
+
@Test
fun runInsideMain() {
val res = runBlocking(Dispatchers.Main) {
@@ -560,4 +635,4 @@
private fun drain() {
assertThat(taskTracker.awaitIdle(10, TimeUnit.SECONDS)).isTrue()
}
-}
\ No newline at end of file
+}
diff --git a/lifecycle/lifecycle-runtime-ktx/src/main/java/androidx/lifecycle/DispatchQueue.kt b/lifecycle/lifecycle-runtime-ktx/src/main/java/androidx/lifecycle/DispatchQueue.kt
index 6c0568a..dc00740 100644
--- a/lifecycle/lifecycle-runtime-ktx/src/main/java/androidx/lifecycle/DispatchQueue.kt
+++ b/lifecycle/lifecycle-runtime-ktx/src/main/java/androidx/lifecycle/DispatchQueue.kt
@@ -22,7 +22,7 @@
import kotlinx.coroutines.Dispatchers
import java.util.ArrayDeque
import java.util.Queue
-import kotlin.coroutines.EmptyCoroutineContext
+import kotlin.coroutines.CoroutineContext
/**
* Helper class for [PausingDispatcher] that tracks runnables which are enqueued to the dispatcher
@@ -80,19 +80,24 @@
}
@MainThread
- private fun canRun() = finished || !paused
+ fun canRun() = finished || !paused
@AnyThread
@SuppressLint("WrongThread") // false negative, we are checking the thread
- fun runOrEnqueue(runnable: Runnable) {
+ fun dispatchAndEnqueue(context: CoroutineContext, runnable: Runnable) {
with(Dispatchers.Main.immediate) {
- if (isDispatchNeeded(EmptyCoroutineContext)) {
- dispatch(
- EmptyCoroutineContext,
- Runnable {
- enqueue(runnable)
- }
- )
+ // This check is here to handle a special but important case. If for example
+ // launchWhenCreated is used while not created it's expected that it will run
+ // synchronously when the lifecycle is created. If we called `dispatch` here
+ // it launches made before the required state is reached would always be deferred
+ // which is not the intended behavior.
+ //
+ // This means that calling `yield()` while paused and then receiving `resume` right
+ // after leads to the runnable being run immediately but that is indeed intended.
+ // This could be solved by implementing `dispatchYield` in the dispatcher but it's
+ // marked as internal API.
+ if (isDispatchNeeded(context) || canRun()) {
+ dispatch(context, Runnable { enqueue(runnable) })
} else {
enqueue(runnable)
}
diff --git a/lifecycle/lifecycle-runtime-ktx/src/main/java/androidx/lifecycle/PausingDispatcher.kt b/lifecycle/lifecycle-runtime-ktx/src/main/java/androidx/lifecycle/PausingDispatcher.kt
index 592afe7..9e56422 100644
--- a/lifecycle/lifecycle-runtime-ktx/src/main/java/androidx/lifecycle/PausingDispatcher.kt
+++ b/lifecycle/lifecycle-runtime-ktx/src/main/java/androidx/lifecycle/PausingDispatcher.kt
@@ -178,7 +178,18 @@
@JvmField
internal val dispatchQueue = DispatchQueue()
- override fun dispatch(context: CoroutineContext, block: Runnable) {
- dispatchQueue.runOrEnqueue(block)
+ override fun isDispatchNeeded(context: CoroutineContext): Boolean {
+ if (Dispatchers.Main.immediate.isDispatchNeeded(context)) {
+ return true
+ }
+ // It's safe to call dispatchQueue.canRun() here because
+ // Dispatchers.Main.immediate.isDispatchNeeded returns true if we're not on the main thread
+ // If the queue is paused right now we need to dispatch so that the block is added to the
+ // the queue
+ return !dispatchQueue.canRun()
}
-}
\ No newline at end of file
+
+ override fun dispatch(context: CoroutineContext, block: Runnable) {
+ dispatchQueue.dispatchAndEnqueue(context, block)
+ }
+}
