Skip to content

Commit e043aec

Browse files
dzinaddzinad
committed
NAVAND-777: fix wrong child dispatcher
1 parent d1e509c commit e043aec

File tree

6 files changed

+138
-18
lines changed

6 files changed

+138
-18
lines changed

‎libnavigation-core/src/main/java/com/mapbox/navigation/core/internal/utils/CoroutineUtils.kt‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,21 @@ package com.mapbox.navigation.core.internal.utils
33
import kotlinx.coroutines.CoroutineScope
44
import kotlinx.coroutines.Job
55
import kotlinx.coroutines.SupervisorJob
6+
import kotlinx.coroutines.job
67
import kotlin.coroutines.CoroutineContext
7-
import kotlin.coroutines.EmptyCoroutineContext
88

99
internal object CoroutineUtils {
1010

11-
fun createChildScope(
11+
fun createScope(
1212
parentJob: Job,
13-
additionalContext: CoroutineContext = EmptyCoroutineContext
14-
): CoroutineScope = CoroutineScope(SupervisorJob(parentJob) + additionalContext)
13+
additionalContext: CoroutineContext
14+
): CoroutineScope {
15+
return CoroutineScope(SupervisorJob(parentJob) + additionalContext)
16+
}
17+
18+
fun createChildScope(
19+
parentScope: CoroutineScope
20+
): CoroutineScope = CoroutineScope(
21+
SupervisorJob(parentScope.coroutineContext.job) + parentScope.coroutineContext.minusKey(Job)
22+
)
1523
}

‎libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshController.kt‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import kotlinx.coroutines.CancellationException
1111
import kotlinx.coroutines.CoroutineScope
1212
import kotlinx.coroutines.cancel
1313
import kotlinx.coroutines.delay
14-
import kotlinx.coroutines.job
1514
import kotlinx.coroutines.launch
1615

1716
@OptIn(ExperimentalPreviewMapboxNavigationAPI::class)
@@ -39,8 +38,7 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
3938
RetryRouteRefreshStrategy(maxAttemptsCount = MAX_RETRY_COUNT)
4039
)
4140

42-
private var plannedRefreshScope =
43-
CoroutineUtils.createChildScope(parentScope.coroutineContext.job)
41+
private var plannedRefreshScope = CoroutineUtils.createChildScope(parentScope)
4442
private var paused = false
4543
var routesToRefresh: List<NavigationRoute>? = null
4644
private set
@@ -151,7 +149,7 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
151149

152150
private fun recreateScope() {
153151
plannedRefreshScope.cancel()
154-
plannedRefreshScope = CoroutineUtils.createChildScope(parentScope.coroutineContext.job)
152+
plannedRefreshScope = CoroutineUtils.createChildScope(parentScope)
155153
}
156154

157155
companion object {

‎libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/RouteRefreshControllerProvider.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,13 @@ internal object RouteRefreshControllerProvider {
5555
routeRefresherExecutor,
5656
routeRefreshOptions,
5757
stateHolder,
58-
CoroutineUtils.createChildScope(routeRefreshParentJob, immediateDispatcher),
58+
CoroutineUtils.createScope(routeRefreshParentJob, immediateDispatcher),
5959
routeRefresherResultProcessor
6060
)
6161
val immediateRouteRefreshController = ImmediateRouteRefreshController(
6262
routeRefresherExecutor,
6363
stateHolder,
64-
CoroutineUtils.createChildScope(routeRefreshParentJob, dispatcher),
64+
CoroutineUtils.createScope(routeRefreshParentJob, dispatcher),
6565
routeRefresherResultProcessor
6666
)
6767
return RouteRefreshController(

‎libnavigation-core/src/test/java/com/mapbox/navigation/core/internal/utils/CoroutineUtilsTest.kt‎

Lines changed: 120 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,53 @@
11
package com.mapbox.navigation.core.internal.utils
22

3+
import android.os.Handler
4+
import android.os.HandlerThread
5+
import kotlinx.coroutines.CoroutineScope
36
import kotlinx.coroutines.Job
47
import kotlinx.coroutines.SupervisorJob
8+
import kotlinx.coroutines.android.asCoroutineDispatcher
59
import kotlinx.coroutines.cancel
610
import kotlinx.coroutines.delay
711
import kotlinx.coroutines.launch
12+
import kotlinx.coroutines.runBlocking
813
import org.junit.After
14+
import org.junit.Assert.assertEquals
915
import org.junit.Assert.assertFalse
16+
import org.junit.Assert.assertNotNull
1017
import org.junit.Assert.assertTrue
1118
import org.junit.Before
1219
import org.junit.Test
20+
import org.junit.runner.RunWith
21+
import org.robolectric.RobolectricTestRunner
22+
import kotlin.coroutines.EmptyCoroutineContext
1323

24+
@RunWith(RobolectricTestRunner::class)
1425
class CoroutineUtilsTest {
1526

27+
private lateinit var parentThread: HandlerThread
28+
private lateinit var parentScope: CoroutineScope
1629
private lateinit var parentJob: Job
1730

1831
@Before
1932
fun setUp() {
33+
parentThread = HandlerThread("parent thread").also { it.start() }
2034
parentJob = SupervisorJob()
35+
parentScope = CoroutineScope(
36+
SupervisorJob() + Handler(parentThread.looper).asCoroutineDispatcher()
37+
)
2138
}
2239

2340
@After
2441
fun tearDown() {
42+
parentScope.cancel()
2543
parentJob.cancel()
44+
parentThread.quit()
2645
}
2746

2847
@Test
2948
fun createChildScope_parentCancellationCancelsChildren() {
30-
val scope1 = CoroutineUtils.createChildScope(parentJob)
31-
val scope2 = CoroutineUtils.createChildScope(parentJob)
49+
val scope1 = CoroutineUtils.createChildScope(parentScope)
50+
val scope2 = CoroutineUtils.createChildScope(parentScope)
3251

3352
val job1 = scope1.launch {
3453
delay(5000)
@@ -40,16 +59,78 @@ class CoroutineUtilsTest {
4059
assertFalse(job1.isCancelled)
4160
assertFalse(job2.isCancelled)
4261

43-
parentJob.cancel()
62+
parentScope.cancel()
4463

4564
assertTrue(job1.isCancelled)
4665
assertTrue(job2.isCancelled)
4766
}
4867

4968
@Test
5069
fun createChildScope_childDoesNotCancelOtherChildren() {
51-
val scope1 = CoroutineUtils.createChildScope(parentJob)
52-
val scope2 = CoroutineUtils.createChildScope(parentJob)
70+
val scope1 = CoroutineUtils.createChildScope(parentScope)
71+
val scope2 = CoroutineUtils.createChildScope(parentScope)
72+
73+
val job1 = scope1.launch {
74+
delay(5000)
75+
}
76+
val job2 = scope2.launch {
77+
delay(5000)
78+
}
79+
80+
assertFalse(job1.isCancelled)
81+
assertFalse(job2.isCancelled)
82+
83+
scope1.cancel()
84+
85+
assertTrue(job1.isCancelled)
86+
assertFalse(job2.isCancelled)
87+
}
88+
89+
@Test
90+
fun createChildScope_childrenUseParentDispatcher() = runBlocking {
91+
val childScope = CoroutineUtils.createChildScope(parentScope)
92+
93+
var childThread: Long? = null
94+
var parentThread: Long? = null
95+
val childJob = childScope.launch {
96+
childThread = Thread.currentThread().id
97+
}
98+
val parentJob = parentScope.launch {
99+
parentThread = Thread.currentThread().id
100+
}
101+
102+
parentJob.join()
103+
childJob.join()
104+
105+
assertNotNull(childThread)
106+
assertEquals(parentThread, childThread)
107+
}
108+
109+
@Test
110+
fun createScope_parentCancellationCancelsChildren() {
111+
val scope1 = CoroutineUtils.createScope(parentJob, EmptyCoroutineContext)
112+
val scope2 = CoroutineUtils.createScope(parentJob, EmptyCoroutineContext)
113+
114+
val job1 = scope1.launch {
115+
delay(5000)
116+
}
117+
val job2 = scope2.launch {
118+
delay(5000)
119+
}
120+
121+
assertFalse(job1.isCancelled)
122+
assertFalse(job2.isCancelled)
123+
124+
parentJob.cancel()
125+
126+
assertTrue(job1.isCancelled)
127+
assertTrue(job2.isCancelled)
128+
}
129+
130+
@Test
131+
fun createScope_childDoesNotCancelOtherChildren() {
132+
val scope1 = CoroutineUtils.createScope(parentJob, EmptyCoroutineContext)
133+
val scope2 = CoroutineUtils.createScope(parentJob, EmptyCoroutineContext)
53134

54135
val job1 = scope1.launch {
55136
delay(5000)
@@ -66,4 +147,38 @@ class CoroutineUtilsTest {
66147
assertTrue(job1.isCancelled)
67148
assertFalse(job2.isCancelled)
68149
}
150+
151+
@Test
152+
fun createScope_childrenUseOwnDispatcher() = runBlocking {
153+
val thread1 = HandlerThread("thread 1").also { it.start() }
154+
val thread2 = HandlerThread("thread 2").also { it.start() }
155+
try {
156+
val childScope1 = CoroutineUtils.createScope(
157+
parentJob,
158+
Handler(thread1.looper).asCoroutineDispatcher()
159+
)
160+
val childScope2 = CoroutineUtils.createScope(
161+
parentJob,
162+
Handler(thread2.looper).asCoroutineDispatcher()
163+
)
164+
165+
var childThread1: Thread? = null
166+
var childThread2: Thread? = null
167+
val childJob1 = childScope1.launch {
168+
childThread1 = Thread.currentThread()
169+
}
170+
val childJob2 = childScope2.launch {
171+
childThread2 = Thread.currentThread()
172+
}
173+
174+
childJob1.join()
175+
childJob2.join()
176+
177+
assertEquals(thread1, childThread1)
178+
assertEquals(thread2, childThread2)
179+
} finally {
180+
thread1.quit()
181+
thread2.quit()
182+
}
183+
}
69184
}

‎libnavigation-core/src/test/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshControllerTest.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import kotlinx.coroutines.CoroutineScope
2323
import kotlinx.coroutines.ExperimentalCoroutinesApi
2424
import kotlinx.coroutines.SupervisorJob
2525
import kotlinx.coroutines.cancel
26-
import kotlinx.coroutines.job
2726
import kotlinx.coroutines.test.TestCoroutineDispatcher
2827
import kotlinx.coroutines.test.TestCoroutineScope
2928
import org.junit.After
@@ -59,7 +58,7 @@ class PlannedRouteRefreshControllerTest {
5958
fun setUp() {
6059
mockkObject(RouteRefreshValidator)
6160
mockkObject(CoroutineUtils)
62-
every { CoroutineUtils.createChildScope(parentScope.coroutineContext.job) } answers {
61+
every { CoroutineUtils.createChildScope(parentScope) } answers {
6362
TestCoroutineScope(SupervisorJob() + childScopeDispatcher).also { childScope = it }
6463
}
6564
sut = PlannedRouteRefreshController(

‎libnavigation-core/src/test/java/com/mapbox/navigation/core/routerefresh/RouteRefreshIntegrationTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ internal open class RouteRefreshIntegrationTest {
100100
} returns ExpectedFactory.createValue(listOf(mockk(relaxed = true)))
101101
mockkObject(CoroutineUtils)
102102
every {
103-
CoroutineUtils.createChildScope(any(), any())
103+
CoroutineUtils.createScope(any(), any())
104104
} answers { coroutineRule.createTestScope() }
105105

106106
primaryRouteProgressDataProvider.onRouteProgressChanged(

0 commit comments

Comments
 (0)