Skip to content

Commit ba303ea

Browse files
committed
Change ScopeProvider.coroutineScope throwing behavior when accessing it before scope is active, or after scope is inactive.
Instead of throwing at call site, we deliver `CoroutineScopeLifecycleException(message: String, cause: OutsideScopeException)` to the `CoroutineExceptionHandler` installed at `RibCoroutinesConfig.exceptionHandler`. Note `cause` is always non-null, and it's either `LifecycleNotStartedException` or `LifecycleEndedException`. ## Motivation Suppose you have an interactor that starts subscribing to an Rx stream asynchronously. ```kotlin class MyInteractor { override fun didBecomeActive() { runLater { someObservable.autoDispose(this).subscribe() } } } ``` It is possible that this Rx subscription will be triggered after interactor's already inactive. In that case, subscription will be No-Op. This is not good code for a major reason: `Interactor` is attempting to do work after it's inactive. In other words, `runLater` does not respect the interactor lifecycle. Still, considering this is some legacy code being migrated from Rx to Coroutines, the new code would look like the following. ```kotlin class MyInteractor { override fun didBecomeActive() { runLater { someObservable.autoDispose(coroutineScope).subscribe() } } } ``` Unlike the Rx counterpart, this code will sometimes fatally throw. If `ScopeProvider.coroutineScope` is called after the scope's inactive, it will throw `LifecycleEndedException` at call site. Calling `coroutineScope` outside of lifecycle bounds is always erroneous code. But in order to favour a smoother migration to Coroutines, and to avoid surprises of `val` throwing exceptions, we are changing current implementation to deliver the exception to the `CoroutineExceptionHandler` instead of throwing at call site. If some app decides to override the default behavior of crashing (in JVM/Android) in face of this erroneous code, one can customize `RibCoroutinesConfig.exceptionHandler` at app startup: ```kotlin RibCoroutinesConfig.exceptionHandler = CoroutineExceptionHandler { _, throwable -> when (throwable) { is CoroutineScopeLifecycleException -> log(throwable) // only log, don't re-throw. else -> throw throwable } } ```
1 parent 9b5cff6 commit ba303ea

File tree

5 files changed

+212
-18
lines changed

5 files changed

+212
-18
lines changed

android/gradle/libs.versions.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ javapoet = "1.11.1"
3333
jsr250 = "1.0"
3434
junit = "4.12"
3535
kotlin = "1.8.20"
36-
kotlinx-coroutines = "1.7.3"
36+
kotlinx-coroutines = "1.8.0-RC2"
3737
ktfmt = "0.43"
3838
ktlint = "0.48.2"
3939
leakcanary = "1.5.4"

android/libraries/rib-coroutines/build.gradle

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@ dependencies {
2424
api(libs.autodispose.coroutines)
2525
api(libs.coroutines.android)
2626
api(libs.coroutines.rx2)
27+
implementation(libs.autodispose.lifecycle)
2728

2829
compileOnly(libs.android.api)
2930

3031
testImplementation(project(":libraries:rib-base"))
3132
testImplementation(project(":libraries:rib-test"))
33+
testImplementation(project(":libraries:rib-coroutines-test"))
3234
testImplementation(testLibs.junit)
3335
testImplementation(testLibs.mockito)
3436
testImplementation(testLibs.mockitoKotlin)

android/libraries/rib-coroutines/src/main/kotlin/com/uber/rib/core/RibCoroutineScopes.kt

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,23 @@
1616
package com.uber.rib.core
1717

1818
import android.app.Application
19+
import com.uber.autodispose.OutsideScopeException
1920
import com.uber.autodispose.ScopeProvider
20-
import com.uber.autodispose.coroutinesinterop.asCoroutineScope
2121
import com.uber.rib.core.internal.CoroutinesFriendModuleApi
22+
import com.uber.rib.core.internal.DirectDispatcher
2223
import java.util.WeakHashMap
2324
import kotlin.coroutines.CoroutineContext
2425
import kotlin.coroutines.EmptyCoroutineContext
2526
import kotlin.reflect.KProperty
2627
import kotlinx.coroutines.CoroutineName
2728
import kotlinx.coroutines.CoroutineScope
29+
import kotlinx.coroutines.CoroutineStart
30+
import kotlinx.coroutines.Job
2831
import kotlinx.coroutines.SupervisorJob
32+
import kotlinx.coroutines.cancel
2933
import kotlinx.coroutines.job
34+
import kotlinx.coroutines.launch
35+
import kotlinx.coroutines.rx2.await
3036

3137
/**
3238
* [CoroutineScope] tied to this [ScopeProvider]. This scope will be canceled when ScopeProvider is
@@ -37,15 +43,7 @@ import kotlinx.coroutines.job
3743
*/
3844
@OptIn(CoroutinesFriendModuleApi::class)
3945
public val ScopeProvider.coroutineScope: CoroutineScope by
40-
LazyCoroutineScope<ScopeProvider> {
41-
val context: CoroutineContext =
42-
SupervisorJob() +
43-
RibDispatchers.Main.immediate +
44-
CoroutineName("${this::class.simpleName}:coroutineScope") +
45-
(RibCoroutinesConfig.exceptionHandler ?: EmptyCoroutineContext)
46-
47-
asCoroutineScope(context)
48-
}
46+
LazyCoroutineScope<ScopeProvider> { ScopeProviderCoroutineScope(this, createCoroutineContext()) }
4947

5048
/**
5149
* [CoroutineScope] tied to this [Application]. This scope will not be cancelled, it lives for the
@@ -56,15 +54,40 @@ public val ScopeProvider.coroutineScope: CoroutineScope by
5654
*/
5755
@OptIn(CoroutinesFriendModuleApi::class)
5856
public val Application.coroutineScope: CoroutineScope by
59-
LazyCoroutineScope<Application> {
60-
val context: CoroutineContext =
61-
SupervisorJob() +
62-
RibDispatchers.Main.immediate +
63-
CoroutineName("${this::class.simpleName}:coroutineScope") +
64-
(RibCoroutinesConfig.exceptionHandler ?: EmptyCoroutineContext)
57+
LazyCoroutineScope<Application> { CoroutineScope(createCoroutineContext()) }
58+
59+
private fun Any.createCoroutineContext() =
60+
SupervisorJob() +
61+
RibDispatchers.Main.immediate +
62+
CoroutineName("${this::class.simpleName}:coroutineScope") +
63+
(RibCoroutinesConfig.exceptionHandler ?: EmptyCoroutineContext)
6564

66-
CoroutineScope(context)
65+
private class ScopeProviderCoroutineScope(
66+
scopeProvider: ScopeProvider,
67+
override val coroutineContext: CoroutineContext,
68+
) : ScopeProvider by scopeProvider, CoroutineScope {
69+
init {
70+
requireNotNull(coroutineContext[Job]) { "coroutineContext must have a job in it" }
71+
cancelWhenLifecycleEnded()
6772
}
73+
}
74+
75+
@OptIn(CoroutinesFriendModuleApi::class)
76+
private fun ScopeProviderCoroutineScope.cancelWhenLifecycleEnded() {
77+
launch(DirectDispatcher, CoroutineStart.UNDISPATCHED) { awaitCompletion() }
78+
.invokeOnCompletion { e -> cancel("Lifecycle is not active", e) }
79+
}
80+
81+
private suspend inline fun ScopeProvider.awaitCompletion() {
82+
try {
83+
requestScope().await()
84+
} catch (e: OutsideScopeException) {
85+
throw CoroutineScopeLifecycleException(
86+
message = "Attempted to obtain ScopeProvider.coroutineScope out of lifecycle bounds",
87+
cause = e,
88+
)
89+
}
90+
}
6891

6992
@CoroutinesFriendModuleApi
7093
public class LazyCoroutineScope<This : Any>(private val initializer: This.() -> CoroutineScope) {
@@ -88,3 +111,9 @@ public class LazyCoroutineScope<This : Any>(private val initializer: This.() ->
88111
}
89112
}
90113
}
114+
115+
public class CoroutineScopeLifecycleException
116+
internal constructor(
117+
message: String,
118+
cause: OutsideScopeException,
119+
) : RuntimeException(message, cause)
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright (C) 2024. Uber Technologies
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.uber.rib.core.internal
17+
18+
import kotlin.coroutines.CoroutineContext
19+
import kotlinx.coroutines.CoroutineDispatcher
20+
import kotlinx.coroutines.Runnable
21+
22+
/**
23+
* A coroutine dispatcher that is not confined to any specific thread. It executes the initial
24+
* continuation of a coroutine in the current call-frame and lets the coroutine resume in whatever
25+
* thread that is used by the corresponding suspending function, without mandating any specific
26+
* threading policy.
27+
*
28+
* This dispatcher is similar to [Unconfined][com.uber.rib.core.RibDispatchers.Unconfined], with the
29+
* difference that it does not form an event-loop on nested coroutines, which implies that it has
30+
* predictable ordering of events with the tradeoff of a risk StackOverflowError.
31+
*
32+
* **This is internal API, not supposed to be used by library consumers.**
33+
*/
34+
@CoroutinesFriendModuleApi
35+
public object DirectDispatcher : CoroutineDispatcher() {
36+
override fun dispatch(context: CoroutineContext, block: Runnable) {
37+
block.run()
38+
}
39+
}
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/*
2+
* Copyright (C) 2024. Uber Technologies
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.uber.rib.core
17+
18+
import com.google.common.truth.Truth.assertThat
19+
import com.uber.autodispose.lifecycle.LifecycleEndedException
20+
import com.uber.autodispose.lifecycle.LifecycleNotStartedException
21+
import kotlin.contracts.ExperimentalContracts
22+
import kotlin.contracts.InvocationKind
23+
import kotlin.contracts.contract
24+
import kotlinx.coroutines.CoroutineExceptionHandler
25+
import kotlinx.coroutines.ExperimentalCoroutinesApi
26+
import kotlinx.coroutines.Job
27+
import kotlinx.coroutines.awaitCancellation
28+
import kotlinx.coroutines.isActive
29+
import kotlinx.coroutines.launch
30+
import kotlinx.coroutines.test.runCurrent
31+
import kotlinx.coroutines.test.runTest
32+
import org.hamcrest.CoreMatchers.instanceOf
33+
import org.junit.Before
34+
import org.junit.Rule
35+
import org.junit.Test
36+
import org.junit.rules.ExpectedException
37+
import org.junit.runner.RunWith
38+
import org.junit.runners.Parameterized
39+
import org.mockito.kotlin.mock
40+
41+
@OptIn(ExperimentalCoroutinesApi::class)
42+
@RunWith(Parameterized::class)
43+
class RibCoroutineScopesTest(
44+
private val throwWhenBeforeActive: Boolean,
45+
private val throwWhenAfterInactive: Boolean,
46+
) {
47+
@get:Rule val ribCoroutinesRule = RibCoroutinesRule()
48+
49+
@get:Rule val exceptionRule: ExpectedException = ExpectedException.none()
50+
51+
private val interactor = TestInteractor()
52+
53+
@Before
54+
fun setUp() {
55+
RibCoroutinesConfig.exceptionHandler = CoroutineExceptionHandler { _, throwable ->
56+
when (throwable) {
57+
is CoroutineScopeLifecycleException -> if (shouldThrow(throwable)) throw throwable
58+
else -> throw throwable
59+
}
60+
}
61+
}
62+
63+
@Test
64+
fun coroutineScope_whenCalledBeforeActive_throwsCoroutineScopeLifecycleException() = runTest {
65+
if (throwWhenBeforeActive) {
66+
exceptionRule.expect(CoroutineScopeLifecycleException::class.java)
67+
exceptionRule.expectCause(instanceOf(LifecycleNotStartedException::class.java))
68+
}
69+
assertThat(interactor.coroutineScope.isActive).isFalse()
70+
}
71+
72+
@Test
73+
fun coroutineScope_whenCalledAfterInactive_throwsCoroutineScopeLifecycleException() = runTest {
74+
if (throwWhenAfterInactive) {
75+
exceptionRule.expect(CoroutineScopeLifecycleException::class.java)
76+
exceptionRule.expectCause(instanceOf(LifecycleEndedException::class.java))
77+
}
78+
interactor.attachAndDetach {}
79+
assertThat(interactor.coroutineScope.isActive).isFalse()
80+
}
81+
82+
@Test
83+
fun coroutineScope_whenCalledWhileActive_cancelsWhenInactive() = runTest {
84+
var launched = false
85+
val job: Job
86+
interactor.attachAndDetach {
87+
job =
88+
coroutineScope.launch {
89+
launched = true
90+
awaitCancellation()
91+
}
92+
runCurrent()
93+
assertThat(launched).isTrue()
94+
assertThat(job.isActive).isTrue()
95+
}
96+
assertThat(job.isCancelled).isTrue()
97+
}
98+
99+
private fun shouldThrow(e: CoroutineScopeLifecycleException): Boolean =
100+
(throwWhenBeforeActive && e.cause is LifecycleNotStartedException) ||
101+
(throwWhenAfterInactive && e.cause is LifecycleEndedException)
102+
103+
companion object {
104+
@JvmStatic
105+
@Parameterized.Parameters(name = "throwWhenBeforeActive = {0}, throwWhenAfterInactive = {1}")
106+
fun data() =
107+
listOf(
108+
arrayOf(true, true),
109+
arrayOf(true, false),
110+
arrayOf(false, true),
111+
arrayOf(false, false),
112+
)
113+
}
114+
}
115+
116+
private class TestInteractor : Interactor<Unit, Router<*>>()
117+
118+
@OptIn(ExperimentalContracts::class)
119+
private inline fun TestInteractor.attachAndDetach(block: TestInteractor.() -> Unit) {
120+
contract { callsInPlace(block, InvocationKind.EXACTLY_ONCE) }
121+
InteractorHelper.attach(this, Unit, mock(), null)
122+
block()
123+
InteractorHelper.detach(this)
124+
}

0 commit comments

Comments
 (0)