Skip to content

Commit a7fe247

Browse files
authored
Fix flaky test, ensure job suspension where expected by the test (#4204)
Fixes #3596
1 parent b286646 commit a7fe247

File tree

1 file changed

+57
-12
lines changed

1 file changed

+57
-12
lines changed

kotlinx-coroutines-core/jvm/test/ThreadContextElementTest.kt

+57-12
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package kotlinx.coroutines
22

3+
import kotlinx.coroutines.flow.*
34
import kotlinx.coroutines.testing.*
45
import org.junit.Test
6+
import java.util.concurrent.CopyOnWriteArrayList
7+
import java.util.concurrent.ExecutorService
8+
import java.util.concurrent.Executors
59
import kotlin.coroutines.*
610
import kotlin.test.*
7-
import kotlinx.coroutines.flow.*
811

912
class ThreadContextElementTest : TestBase() {
1013

@@ -155,39 +158,81 @@ class ThreadContextElementTest : TestBase() {
155158
}
156159
}
157160

158-
class JobCaptor(val capturees: ArrayList<Job> = ArrayList()) : ThreadContextElement<Unit> {
161+
class JobCaptor(val capturees: MutableList<String> = CopyOnWriteArrayList()) : ThreadContextElement<Unit> {
159162

160163
companion object Key : CoroutineContext.Key<MyElement>
161164

162165
override val key: CoroutineContext.Key<*> get() = Key
163166

164167
override fun updateThreadContext(context: CoroutineContext) {
165-
capturees.add(context.job)
168+
capturees.add("Update: ${context.job}")
166169
}
167170

168171
override fun restoreThreadContext(context: CoroutineContext, oldState: Unit) {
172+
capturees.add("Restore: ${context.job}")
169173
}
170174
}
171175

176+
/**
177+
* For stability of the test, it is important to make sure that
178+
* the parent job actually suspends when calling
179+
* `withContext(dispatcher2 + CoroutineName("dispatched"))`.
180+
*
181+
* Here this requirement is fulfilled by forcing execution on a single thread.
182+
* However, dispatching is performed with two non-equal dispatchers to force dispatching.
183+
*
184+
* Suspend of the parent coroutine [kotlinx.coroutines.DispatchedCoroutine.trySuspend] is out of the control of the test,
185+
* while being executed concurrently with resume of the child coroutine [kotlinx.coroutines.DispatchedCoroutine.tryResume].
186+
*/
172187
@Test
173188
fun testWithContextJobAccess() = runTest {
189+
val executor = Executors.newSingleThreadExecutor()
190+
// Emulate non-equal dispatchers
191+
val executor1 = object : ExecutorService by executor {}
192+
val executor2 = object : ExecutorService by executor {}
193+
val dispatcher1 = executor1.asCoroutineDispatcher()
194+
val dispatcher2 = executor2.asCoroutineDispatcher()
174195
val captor = JobCaptor()
175-
val manuallyCaptured = ArrayList<Job>()
176-
runBlocking(captor) {
177-
manuallyCaptured += coroutineContext.job
196+
val manuallyCaptured = mutableListOf<String>()
197+
198+
fun registerUpdate(job: Job?) = manuallyCaptured.add("Update: $job")
199+
fun registerRestore(job: Job?) = manuallyCaptured.add("Restore: $job")
200+
201+
var rootJob: Job? = null
202+
runBlocking(captor + dispatcher1) {
203+
rootJob = coroutineContext.job
204+
registerUpdate(rootJob)
205+
var undispatchedJob: Job? = null
178206
withContext(CoroutineName("undispatched")) {
179-
manuallyCaptured += coroutineContext.job
180-
withContext(Dispatchers.IO) {
181-
manuallyCaptured += coroutineContext.job
207+
undispatchedJob = coroutineContext.job
208+
registerUpdate(undispatchedJob)
209+
// These 2 restores and the corresponding next 2 updates happen only if the following `withContext`
210+
// call actually suspends.
211+
registerRestore(undispatchedJob)
212+
registerRestore(rootJob)
213+
// Without forcing of single backing thread the code inside `withContext`
214+
// may already complete at the moment when the parent coroutine decides
215+
// whether it needs to suspend or not.
216+
var dispatchedJob: Job? = null
217+
withContext(dispatcher2 + CoroutineName("dispatched")) {
218+
dispatchedJob = coroutineContext.job
219+
registerUpdate(dispatchedJob)
182220
}
221+
registerRestore(dispatchedJob)
183222
// Context restored, captured again
184-
manuallyCaptured += coroutineContext.job
223+
registerUpdate(undispatchedJob)
185224
}
225+
registerRestore(undispatchedJob)
186226
// Context restored, captured again
187-
manuallyCaptured += coroutineContext.job
227+
registerUpdate(rootJob)
188228
}
229+
registerRestore(rootJob)
189230

190-
assertEquals(manuallyCaptured, captor.capturees)
231+
// Restores may be called concurrently to the update calls in other threads, so their order is not checked.
232+
val expected = manuallyCaptured.filter { it.startsWith("Update: ") }.joinToString(separator = "\n")
233+
val actual = captor.capturees.filter { it.startsWith("Update: ") }.joinToString(separator = "\n")
234+
assertEquals(expected, actual)
235+
executor.shutdownNow()
191236
}
192237

193238
@Test

0 commit comments

Comments
 (0)