From 0e96440de551e4f160096ebe6a435782efa02b6f Mon Sep 17 00:00:00 2001 From: severn-everett Date: Mon, 10 Jan 2022 14:34:58 +0100 Subject: [PATCH 01/29] Upgraded SLF4J in eponymous integration module to 1.7.32 and updated deprecated Gradle dependency declarations (#3088) --- integration/kotlinx-coroutines-slf4j/build.gradle.kts | 8 ++++---- integration/kotlinx-coroutines-slf4j/src/MDCContext.kt | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/integration/kotlinx-coroutines-slf4j/build.gradle.kts b/integration/kotlinx-coroutines-slf4j/build.gradle.kts index a341eefe13..3552333311 100644 --- a/integration/kotlinx-coroutines-slf4j/build.gradle.kts +++ b/integration/kotlinx-coroutines-slf4j/build.gradle.kts @@ -3,10 +3,10 @@ */ dependencies { - compile("org.slf4j:slf4j-api:1.7.25") - testCompile("io.github.microutils:kotlin-logging:1.5.4") - testRuntime("ch.qos.logback:logback-classic:1.2.3") - testRuntime("ch.qos.logback:logback-core:1.2.3") + implementation("org.slf4j:slf4j-api:1.7.32") + testImplementation("io.github.microutils:kotlin-logging:2.1.0") + testRuntimeOnly("ch.qos.logback:logback-classic:1.2.7") + testRuntimeOnly("ch.qos.logback:logback-core:1.2.7") } externalDocumentationLink( diff --git a/integration/kotlinx-coroutines-slf4j/src/MDCContext.kt b/integration/kotlinx-coroutines-slf4j/src/MDCContext.kt index 9528f2b22d..9f165fdd50 100644 --- a/integration/kotlinx-coroutines-slf4j/src/MDCContext.kt +++ b/integration/kotlinx-coroutines-slf4j/src/MDCContext.kt @@ -43,6 +43,7 @@ public class MDCContext( /** * The value of [MDC] context map. */ + @Suppress("MemberVisibilityCanBePrivate") public val contextMap: MDCContextMap = MDC.getCopyOfContextMap() ) : ThreadContextElement, AbstractCoroutineContextElement(Key) { /** From ed3445f9df22355cde5432c5efd7ce80adc3c603 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Mon, 10 Jan 2022 17:57:01 +0300 Subject: [PATCH 02/29] Improve Flow documentation (#3127) Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com> --- kotlinx-coroutines-core/common/src/flow/Flow.kt | 5 ++++- kotlinx-coroutines-core/common/src/flow/SharedFlow.kt | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/flow/Flow.kt b/kotlinx-coroutines-core/common/src/flow/Flow.kt index 07ff5392c7..3520c48b42 100644 --- a/kotlinx-coroutines-core/common/src/flow/Flow.kt +++ b/kotlinx-coroutines-core/common/src/flow/Flow.kt @@ -108,7 +108,7 @@ import kotlin.coroutines.* * val myFlow = flow { * // GlobalScope.launch { // is prohibited * // launch(Dispatchers.IO) { // is prohibited - * // withContext(CoroutineName("myFlow")) // is prohibited + * // withContext(CoroutineName("myFlow")) { // is prohibited * emit(1) // OK * coroutineScope { * emit(2) // OK -- still the same coroutine @@ -191,6 +191,9 @@ public interface Flow { * * To ensure the context preservation property, it is not recommended implementing this method directly. * Instead, [AbstractFlow] can be used as the base type to properly ensure flow's properties. + * + * All default flow implementations ensure context preservation and exception transparency properties on a best-effort basis + * and throw [IllegalStateException] if a violation was detected. */ public suspend fun collect(collector: FlowCollector) } diff --git a/kotlinx-coroutines-core/common/src/flow/SharedFlow.kt b/kotlinx-coroutines-core/common/src/flow/SharedFlow.kt index 43709b33c5..beb4255c09 100644 --- a/kotlinx-coroutines-core/common/src/flow/SharedFlow.kt +++ b/kotlinx-coroutines-core/common/src/flow/SharedFlow.kt @@ -115,7 +115,7 @@ import kotlin.native.concurrent.* * ### Implementation notes * * Shared flow implementation uses a lock to ensure thread-safety, but suspending collector and emitter coroutines are - * resumed outside of this lock to avoid dead-locks when using unconfined coroutines. Adding new subscribers + * resumed outside of this lock to avoid deadlocks when using unconfined coroutines. Adding new subscribers * has `O(1)` amortized cost, but emitting has `O(N)` cost, where `N` is the number of subscribers. * * ### Not stable for inheritance @@ -132,13 +132,13 @@ public interface SharedFlow : Flow { /** * Accepts the given [collector] and [emits][FlowCollector.emit] values into it. - * This method should never be used directly. To emit values from a shared flow into a specific collector, either `collector.emitAll(flow)` or `collect { ... }` extension - * should be used. + * To emit values from a shared flow into a specific collector, either `collector.emitAll(flow)` or `collect { ... }` + * SAM-conversion can be used. * * **A shared flow never completes**. A call to [Flow.collect] or any other terminal operator * on a shared flow never completes normally. * - * @see [Flow.collect] + * @see [Flow.collect] for implementation and inheritance details. */ override suspend fun collect(collector: FlowCollector): Nothing } From 881cf6849e8fd9ca25c140a15521663c7ec964a1 Mon Sep 17 00:00:00 2001 From: Ilya Matveev Date: Tue, 11 Jan 2022 21:30:04 +0700 Subject: [PATCH 03/29] Make multithreadingSupported a property with a getter (#3130) * Make multithreadingSupported a property with a getter This change fixes an issue with the initialization order observed when lazy initialization is disabled (-Xir-property-lazy-initialization= disabled compiler flag) and the new MM is used. The problem is the following: the initialization of DefaultDelay happens before the initialization of multithreadingSupported. Thus the initialization of DefaultDelay gets an uninitialized value of multithreadingSupported and behaves as if the old MM is used. Related issue: #KT-50491. --- kotlinx-coroutines-core/native/src/internal/Concurrent.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kotlinx-coroutines-core/native/src/internal/Concurrent.kt b/kotlinx-coroutines-core/native/src/internal/Concurrent.kt index 1db6c3bd86..f6e18dd5fc 100644 --- a/kotlinx-coroutines-core/native/src/internal/Concurrent.kt +++ b/kotlinx-coroutines-core/native/src/internal/Concurrent.kt @@ -32,6 +32,7 @@ internal open class SuppressSupportingThrowableImpl : Throwable() { } } -@SharedImmutable +// getter instead of a property due to the bug in the initialization dependencies tracking with '-Xir-property-lazy-initialization=disabled' that Ktor uses @OptIn(ExperimentalStdlibApi::class) -internal val multithreadingSupported: Boolean = kotlin.native.isExperimentalMM() +internal val multithreadingSupported: Boolean + get() = kotlin.native.isExperimentalMM() From a3f249175faac92eff44cb184793b791957738de Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Fri, 14 Jan 2022 18:31:38 +0300 Subject: [PATCH 04/29] Explicitly describe undispatched behaviour in runInterruptible (#3139) Addresses #3109 --- kotlinx-coroutines-core/jvm/src/Interruptible.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/kotlinx-coroutines-core/jvm/src/Interruptible.kt b/kotlinx-coroutines-core/jvm/src/Interruptible.kt index b873eadf4a..0bded76517 100644 --- a/kotlinx-coroutines-core/jvm/src/Interruptible.kt +++ b/kotlinx-coroutines-core/jvm/src/Interruptible.kt @@ -8,7 +8,7 @@ import kotlinx.atomicfu.* import kotlin.coroutines.* /** - * Calls the specified [block] with a given coroutine context in a interruptible manner. + * Calls the specified [block] with a given coroutine context in an interruptible manner. * The blocking code block will be interrupted and this function will throw [CancellationException] * if the coroutine is cancelled. * @@ -30,6 +30,11 @@ import kotlin.coroutines.* * suspend fun BlockingQueue.awaitTake(): T = * runInterruptible(Dispatchers.IO) { queue.take() } * ``` + * + * `runInterruptible` uses [withContext] as an underlying mechanism for switching context, + * meaning that the supplied [block] is invoked in an [undispatched][CoroutineStart.UNDISPATCHED] + * manner directly by the caller if [CoroutineDispatcher] from the current [coroutineContext][currentCoroutineContext] + * is the same as the one supplied in [context]. */ public suspend fun runInterruptible( context: CoroutineContext = EmptyCoroutineContext, From ce02a3fde5657c9b72d44cdba73f1a76e21bca2d Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Mon, 17 Jan 2022 14:03:31 +0300 Subject: [PATCH 05/29] Remove ignored test and its obsolete dependency with ASM 5.0 (#3140) --- kotlinx-coroutines-core/build.gradle | 1 - .../jvm/test/ContinuationSerializationTest.kt | 94 ------------------- 2 files changed, 95 deletions(-) delete mode 100644 kotlinx-coroutines-core/jvm/test/ContinuationSerializationTest.kt diff --git a/kotlinx-coroutines-core/build.gradle b/kotlinx-coroutines-core/build.gradle index 72c893bc17..487527b22b 100644 --- a/kotlinx-coroutines-core/build.gradle +++ b/kotlinx-coroutines-core/build.gradle @@ -185,7 +185,6 @@ kotlin.sourceSets { jvmTest.dependencies { api "org.jetbrains.kotlinx:lincheck:$lincheck_version" api "org.jetbrains.kotlinx:kotlinx-knit-test:$knit_version" - api "com.esotericsoftware:kryo:4.0.0" implementation project(":android-unit-tests") } } diff --git a/kotlinx-coroutines-core/jvm/test/ContinuationSerializationTest.kt b/kotlinx-coroutines-core/jvm/test/ContinuationSerializationTest.kt deleted file mode 100644 index 12941fb1ae..0000000000 --- a/kotlinx-coroutines-core/jvm/test/ContinuationSerializationTest.kt +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. - */ - -package kotlinx.coroutines - - -import com.esotericsoftware.kryo.* -import com.esotericsoftware.kryo.io.* -import kotlinx.atomicfu.* -import org.junit.Test -import org.objenesis.strategy.* -import java.io.* -import kotlin.coroutines.* -import kotlin.coroutines.intrinsics.* -import kotlin.reflect.* -import kotlin.test.* - -@Ignore -class ContinuationSerializationTest : TestBase() { - - companion object { - @JvmStatic - var flag = false - } - -// private val atomicInt = atomic(1) - - private val kryo = - Kryo().also { it.instantiatorStrategy = Kryo.DefaultInstantiatorStrategy(StdInstantiatorStrategy()) } - - private var storage: ByteArrayOutputStream = ByteArrayOutputStream() - - @Test - fun testSafeContinuationSerDe() = testSerization(::serialize, { - it.javaClass.getDeclaredField("result").apply { - isAccessible = true - set(it, COROUTINE_SUSPENDED) - } - }) - - @Test - fun testUnsafeContinuationSerDe() = testSerization(::serializeUnsafe, {}) - -// @Test -// fun testCancellableContinuationSerDe() = testSerization(::serializeCancellable, { -// it.javaClass.superclass.getDeclaredField("_decision").apply { -// isAccessible = true -// set(it, atomicInt) -// } -// }) - - private suspend fun serialize() = suspendCoroutine { cont -> - Output(storage).use { - kryo.writeClassAndObject(it, cont) - } - } - - private suspend fun serializeCancellable() = suspendCancellableCoroutine { cont -> - Output(storage).use { - kryo.writeClassAndObject(it, cont) - } - } - - private suspend fun serializeUnsafe() = suspendCoroutineUninterceptedOrReturn { cont -> - Output(storage).use { - kryo.writeClassAndObject(it, cont) - } - } - - private fun testSerization(serialize: KSuspendFunction0, patcher: (Continuation) -> Unit) = - runBlocking { - launch(Unconfined) { - expect(1) - serialize() - flag = true - } - - val cont = deserialise() - patcher(cont) - expect(2) - cont.resume(Unit) - finish(3) - assertTrue(flag) - } - - @Suppress("UNCHECKED_CAST") - private fun deserialise(): Continuation { - val input = Input(ByteArrayInputStream(storage.toByteArray())) - input.use { - return kryo.readClassAndObject(it) as Continuation - } - } -} From 9169d091bbb02e4c13a7dcaf8f2086b79f3b51ee Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Mon, 17 Jan 2022 14:49:55 +0300 Subject: [PATCH 06/29] Rollback #2972, but leave a compatibility option with 1.6.0 (#3131) The approach from 1.6.0 has proven itself as unstable and multiple hard-to-understand bugs have been reported: * JavaFx timer doesn't really work outside the main thread * The frequent initialization pattern "runBlocking { doSomethingThatMayCallDelay() }" used on the main thread during startup now silently deadlocks * The latter issue was reported both by Android and internal JB Compose users * The provided workaround with system property completely switches off the desired behaviour that e.g. Compose may rely on, potentially introducing new sources of invalid behaviour The original benefits does not outweigh these pitfalls, so the decision is to revert this changes in the minor release Fixes #3113 Fixes #3106 --- kotlinx-coroutines-core/jvm/src/DefaultExecutor.kt | 2 +- .../test/HandlerDispatcherTest.kt | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/kotlinx-coroutines-core/jvm/src/DefaultExecutor.kt b/kotlinx-coroutines-core/jvm/src/DefaultExecutor.kt index 1b14748041..c993bc2324 100644 --- a/kotlinx-coroutines-core/jvm/src/DefaultExecutor.kt +++ b/kotlinx-coroutines-core/jvm/src/DefaultExecutor.kt @@ -8,7 +8,7 @@ import kotlinx.coroutines.internal.* import java.util.concurrent.* import kotlin.coroutines.* -private val defaultMainDelayOptIn = systemProp("kotlinx.coroutines.main.delay", true) +private val defaultMainDelayOptIn = systemProp("kotlinx.coroutines.main.delay", false) internal actual val DefaultDelay: Delay = initializeDefaultDelay() diff --git a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt index 3c40c04b1e..af17adfc00 100644 --- a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt +++ b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt @@ -163,20 +163,19 @@ class HandlerDispatcherTest : TestBase() { } @Test - fun testDelayIsDelegatedToMain() = runTest { + fun testDelayIsNotDelegatedToMain() = runTest { val mainLooper = shadowOf(Looper.getMainLooper()) mainLooper.pause() val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) assertNull(mainMessageQueue.head) val job = launch(Dispatchers.Default, start = CoroutineStart.UNDISPATCHED) { expect(1) - delay(10_000_000) - expect(3) + delay(Long.MAX_VALUE) + expectUnreached() } expect(2) - assertNotNull(mainMessageQueue.head) - mainLooper.runOneTask() - job.join() - finish(4) + assertNull(mainMessageQueue.head) + job.cancelAndJoin() + finish(3) } } From a1f5ab8f042391d2204e8410b14d80b5133bfb18 Mon Sep 17 00:00:00 2001 From: Roman Elizarov Date: Mon, 17 Jan 2022 22:03:07 +0300 Subject: [PATCH 07/29] Optimization: resizable workers array (#3137) Instead of allocating an array of maxPoolSize (~2M) elements for the worst-case supported scenario that may never be reached in practice and takes considerable memory, allocate just an array of corePoolSize elements and grow it dynamically if needed to accommodate more workers. The data structure to make it happen must support lock-free reads for performance reasons, but it is simple since the workers array is modified exclusively under synchronization. --- .../jvm/src/internal/ResizableAtomicArray.kt | 38 +++++++++++++++++++ .../jvm/src/scheduling/CoroutineScheduler.kt | 11 +++--- .../ResizableAtomicArrayLincheckTest.kt | 27 +++++++++++++ 3 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 kotlinx-coroutines-core/jvm/src/internal/ResizableAtomicArray.kt create mode 100644 kotlinx-coroutines-core/jvm/test/lincheck/ResizableAtomicArrayLincheckTest.kt diff --git a/kotlinx-coroutines-core/jvm/src/internal/ResizableAtomicArray.kt b/kotlinx-coroutines-core/jvm/src/internal/ResizableAtomicArray.kt new file mode 100644 index 0000000000..f949d9f5ea --- /dev/null +++ b/kotlinx-coroutines-core/jvm/src/internal/ResizableAtomicArray.kt @@ -0,0 +1,38 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.internal + +import java.util.concurrent.atomic.* + +/** + * Atomic array with lock-free reads and synchronized modifications. It logically has an unbounded size, + * is implicitly filled with nulls, and is resized on updates as needed to grow. + */ +internal class ResizableAtomicArray(initialLength: Int) { + @Volatile + private var array = AtomicReferenceArray(initialLength) + + // for debug output + public fun currentLength(): Int = array.length() + + public operator fun get(index: Int): T? { + val array = this.array // volatile read + return if (index < array.length()) array[index] else null + } + + // Must not be called concurrently, e.g. always use synchronized(this) to call this function + fun setSynchronized(index: Int, value: T?) { + val curArray = this.array + val curLen = curArray.length() + if (index < curLen) { + curArray[index] = value + } else { + val newArray = AtomicReferenceArray((index + 1).coerceAtLeast(2 * curLen)) + for (i in 0 until curLen) newArray[i] = curArray[i] + newArray[index] = value + array = newArray // copy done + } + } +} diff --git a/kotlinx-coroutines-core/jvm/src/scheduling/CoroutineScheduler.kt b/kotlinx-coroutines-core/jvm/src/scheduling/CoroutineScheduler.kt index 41f759ce8a..6b7fecf553 100644 --- a/kotlinx-coroutines-core/jvm/src/scheduling/CoroutineScheduler.kt +++ b/kotlinx-coroutines-core/jvm/src/scheduling/CoroutineScheduler.kt @@ -9,7 +9,6 @@ import kotlinx.coroutines.* import kotlinx.coroutines.internal.* import java.io.* import java.util.concurrent.* -import java.util.concurrent.atomic.* import java.util.concurrent.locks.* import kotlin.math.* import kotlin.random.* @@ -261,7 +260,7 @@ internal class CoroutineScheduler( * works properly */ @JvmField - val workers = AtomicReferenceArray(maxPoolSize + 1) + val workers = ResizableAtomicArray(corePoolSize + 1) /** * Long describing state of workers in this pool. @@ -480,7 +479,7 @@ internal class CoroutineScheduler( * 3) Only then start the worker, otherwise it may miss its own creation */ val worker = Worker(newIndex) - workers[newIndex] = worker + workers.setSynchronized(newIndex, worker) require(newIndex == incrementCreatedWorkers()) worker.start() return cpuWorkers + 1 @@ -525,7 +524,7 @@ internal class CoroutineScheduler( var dormant = 0 var terminated = 0 val queueSizes = arrayListOf() - for (index in 1 until workers.length()) { + for (index in 1 until workers.currentLength()) { val worker = workers[index] ?: continue val queueSize = worker.localQueue.size when (worker.state) { @@ -838,7 +837,7 @@ internal class CoroutineScheduler( val lastIndex = decrementCreatedWorkers() if (lastIndex != oldIndex) { val lastWorker = workers[lastIndex]!! - workers[oldIndex] = lastWorker + workers.setSynchronized(oldIndex, lastWorker) lastWorker.indexInArray = oldIndex /* * Now lastWorker is available at both indices in the array, but it can @@ -852,7 +851,7 @@ internal class CoroutineScheduler( /* * 5) It is safe to clear reference from workers array now. */ - workers[lastIndex] = null + workers.setSynchronized(lastIndex, null) } state = WorkerState.TERMINATED } diff --git a/kotlinx-coroutines-core/jvm/test/lincheck/ResizableAtomicArrayLincheckTest.kt b/kotlinx-coroutines-core/jvm/test/lincheck/ResizableAtomicArrayLincheckTest.kt new file mode 100644 index 0000000000..1948a78ecc --- /dev/null +++ b/kotlinx-coroutines-core/jvm/test/lincheck/ResizableAtomicArrayLincheckTest.kt @@ -0,0 +1,27 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.lincheck + +import kotlinx.coroutines.* +import kotlinx.coroutines.internal.* +import org.jetbrains.kotlinx.lincheck.annotations.* +import org.jetbrains.kotlinx.lincheck.paramgen.* + +@Param(name = "index", gen = IntGen::class, conf = "0:4") +@Param(name = "value", gen = IntGen::class, conf = "1:5") +@OpGroupConfig(name = "sync", nonParallel = true) +class ResizableAtomicArrayLincheckTest : AbstractLincheckTest() { + private val a = ResizableAtomicArray(2) + + @Operation + fun get(@Param(name = "index") index: Int): Int? = a[index] + + @Operation(group = "sync") + fun set(@Param(name = "index") index: Int, @Param(name = "value") value: Int) { + a.setSynchronized(index, value) + } + + override fun extractState() = (0..4).map { a[it] } +} \ No newline at end of file From f44200146f25c48bd69ef0b23bf78a1401c9fa5b Mon Sep 17 00:00:00 2001 From: Chris Povirk Date: Tue, 18 Jan 2022 08:21:52 -0500 Subject: [PATCH 08/29] Update code to prepare for nullness annotations in Guava (#3026) Leverage the fact that `FutureCallback.onSuccess` can only accept `null` when `T` is `null`, to remove the unchecked casts. --- integration/kotlinx-coroutines-guava/README.md | 2 +- integration/kotlinx-coroutines-guava/build.gradle.kts | 7 ++++++- .../kotlinx-coroutines-guava/src/ListenableFuture.kt | 10 ++++------ 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/integration/kotlinx-coroutines-guava/README.md b/integration/kotlinx-coroutines-guava/README.md index 34b8e5818f..b930a6194c 100644 --- a/integration/kotlinx-coroutines-guava/README.md +++ b/integration/kotlinx-coroutines-guava/README.md @@ -62,6 +62,6 @@ Integration with Guava [ListenableFuture](https://github.com/google/guava/wiki/L -[com.google.common.util.concurrent.ListenableFuture]: https://kotlin.github.io/kotlinx.coroutines/https://google.github.io/guava/releases/28.0-jre/api/docs/com/google/common/util/concurrent/ListenableFuture.html +[com.google.common.util.concurrent.ListenableFuture]: https://kotlin.github.io/kotlinx.coroutines/https://google.github.io/guava/releases/31.0.1-jre/api/docs/com/google/common/util/concurrent/ListenableFuture.html diff --git a/integration/kotlinx-coroutines-guava/build.gradle.kts b/integration/kotlinx-coroutines-guava/build.gradle.kts index 12a6ca70b7..ebddd3c9ce 100644 --- a/integration/kotlinx-coroutines-guava/build.gradle.kts +++ b/integration/kotlinx-coroutines-guava/build.gradle.kts @@ -2,12 +2,17 @@ * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ -val guavaVersion = "28.0-jre" +val guavaVersion = "31.0.1-jre" dependencies { compile("com.google.guava:guava:$guavaVersion") } +java { + targetCompatibility = JavaVersion.VERSION_1_8 + sourceCompatibility = JavaVersion.VERSION_1_8 +} + externalDocumentationLink( url = "https://google.github.io/guava/releases/$guavaVersion/api/docs/" ) diff --git a/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt b/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt index d214cc6b1a..0820f1f101 100644 --- a/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt +++ b/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt @@ -135,10 +135,8 @@ public fun ListenableFuture.asDeferred(): Deferred { // Finally, if this isn't done yet, attach a Listener that will complete the Deferred. val deferred = CompletableDeferred() Futures.addCallback(this, object : FutureCallback { - override fun onSuccess(result: T?) { - // Here we work with flexible types, so we unchecked cast to trick the type system - @Suppress("UNCHECKED_CAST") - runCatching { deferred.complete(result as T) } + override fun onSuccess(result: T) { + runCatching { deferred.complete(result) } .onFailure { handleCoroutineException(EmptyCoroutineContext, it) } } @@ -351,7 +349,7 @@ private class JobListenableFuture(private val jobToCancel: Job): ListenableFu * * To preserve Coroutine's [CancellationException], this future points to either `T` or [Cancelled]. */ - private val auxFuture = SettableFuture.create() + private val auxFuture = SettableFuture.create() /** * `true` if [auxFuture.get][ListenableFuture.get] throws [ExecutionException]. @@ -436,7 +434,7 @@ private class JobListenableFuture(private val jobToCancel: Job): ListenableFu } /** See [get()]. */ - private fun getInternal(result: Any): T = if (result is Cancelled) { + private fun getInternal(result: Any?): T = if (result is Cancelled) { throw CancellationException().initCause(result.exception) } else { // We know that `auxFuture` can contain either `T` or `Cancelled`. From 648f694c84bc0fd6bff5e9501178c0b1e8a2fb35 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Tue, 18 Jan 2022 18:30:46 +0300 Subject: [PATCH 09/29] Prevent LimitedParallelismStressTest from allocating too much memory, but still keep the pattern of the load roughly the same (#3147) --- .../jvm/test/LimitedParallelismStressTest.kt | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/kotlinx-coroutines-core/jvm/test/LimitedParallelismStressTest.kt b/kotlinx-coroutines-core/jvm/test/LimitedParallelismStressTest.kt index 4de1862b0f..58f2b6b35a 100644 --- a/kotlinx-coroutines-core/jvm/test/LimitedParallelismStressTest.kt +++ b/kotlinx-coroutines-core/jvm/test/LimitedParallelismStressTest.kt @@ -23,7 +23,7 @@ class LimitedParallelismStressTest(private val targetParallelism: Int) : TestBas @get:Rule val executor = ExecutorRule(targetParallelism * 2) - private val iterations = 100_000 * stressTestMultiplier + private val iterations = 100_000 private val parallelism = AtomicInteger(0) @@ -37,9 +37,11 @@ class LimitedParallelismStressTest(private val targetParallelism: Int) : TestBas @Test fun testLimitedExecutor() = runTest { val view = executor.limitedParallelism(targetParallelism) - repeat(iterations) { - launch(view) { - checkParallelism() + doStress { + repeat(iterations) { + launch(view) { + checkParallelism() + } } } } @@ -47,9 +49,11 @@ class LimitedParallelismStressTest(private val targetParallelism: Int) : TestBas @Test fun testLimitedDispatchersIo() = runTest { val view = Dispatchers.IO.limitedParallelism(targetParallelism) - repeat(iterations) { - launch(view) { - checkParallelism() + doStress { + repeat(iterations) { + launch(view) { + checkParallelism() + } } } } @@ -57,7 +61,7 @@ class LimitedParallelismStressTest(private val targetParallelism: Int) : TestBas @Test fun testLimitedDispatchersIoDispatchYield() = runTest { val view = Dispatchers.IO.limitedParallelism(targetParallelism) - repeat(iterations) { + doStress { launch(view) { yield() checkParallelism() @@ -68,16 +72,26 @@ class LimitedParallelismStressTest(private val targetParallelism: Int) : TestBas @Test fun testLimitedExecutorReachesTargetParallelism() = runTest { val view = executor.limitedParallelism(targetParallelism) - repeat(iterations) { - val barrier = CyclicBarrier(targetParallelism + 1) - repeat(targetParallelism) { - launch(view) { - barrier.await() + doStress { + repeat(iterations) { + val barrier = CyclicBarrier(targetParallelism + 1) + repeat(targetParallelism) { + launch(view) { + barrier.await() + } } + // Successfully awaited parallelism + 1 + barrier.await() + coroutineContext.job.children.toList().joinAll() + } + } + } + + private suspend inline fun doStress(crossinline block: suspend CoroutineScope.() -> Unit) { + repeat(stressTestMultiplier) { + coroutineScope { + block() } - // Successfully awaited parallelism + 1 - barrier.await() - coroutineContext.job.children.toList().joinAll() } } } From 5e5dcd7d5d30b51b96c5adb64560f8fd0b73828d Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Wed, 19 Jan 2022 13:32:57 +0300 Subject: [PATCH 10/29] Integrate Kover (#3141) --- build.gradle | 6 ++- buildSrc/build.gradle.kts | 1 + ...s => animalsniffer-conventions.gradle.kts} | 0 .../main/kotlin/kover-conventions.gradle.kts | 54 +++++++++++++++++++ gradle.properties | 3 +- kotlinx-coroutines-core/build.gradle | 24 ++++++++- .../build.gradle.kts | 7 +++ 7 files changed, 91 insertions(+), 4 deletions(-) rename buildSrc/src/main/kotlin/{animalsniffer-convention.gradle.kts => animalsniffer-conventions.gradle.kts} (100%) create mode 100644 buildSrc/src/main/kotlin/kover-conventions.gradle.kts diff --git a/build.gradle b/build.gradle index 636a6f57f4..d07f4767d9 100644 --- a/build.gradle +++ b/build.gradle @@ -58,6 +58,7 @@ buildscript { classpath "com.moowork.gradle:gradle-node-plugin:$gradle_node_version" classpath "org.jetbrains.kotlinx:binary-compatibility-validator:$binary_compatibility_validator_version" classpath "ru.vyarus:gradle-animalsniffer-plugin:1.5.3" // Android API check + classpath "org.jetbrains.kotlinx:kover:$kover_version" // JMH plugins classpath "com.github.jengelman.gradle.plugins:shadow:5.1.0" @@ -105,7 +106,8 @@ allprojects { } apply plugin: "binary-compatibility-validator" -apply plugin: 'base' +apply plugin: "base" +apply plugin: "kover-conventions" apiValidation { ignoredProjects += unpublished + ["kotlinx-coroutines-bom"] @@ -303,7 +305,7 @@ def publishTasks = getTasksByName("publish", true) + getTasksByName("publishNpm" task deploy(dependsOn: publishTasks) -apply plugin: 'animalsniffer-convention' +apply plugin: "animalsniffer-conventions" clean.dependsOn gradle.includedBuilds.collect { it.task(':clean') } diff --git a/buildSrc/build.gradle.kts b/buildSrc/build.gradle.kts index 8517a5aeee..2637980a24 100644 --- a/buildSrc/build.gradle.kts +++ b/buildSrc/build.gradle.kts @@ -60,4 +60,5 @@ dependencies { exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib") } implementation("ru.vyarus:gradle-animalsniffer-plugin:1.5.3") // Android API check + implementation("org.jetbrains.kotlinx:kover:${version("kover")}") } diff --git a/buildSrc/src/main/kotlin/animalsniffer-convention.gradle.kts b/buildSrc/src/main/kotlin/animalsniffer-conventions.gradle.kts similarity index 100% rename from buildSrc/src/main/kotlin/animalsniffer-convention.gradle.kts rename to buildSrc/src/main/kotlin/animalsniffer-conventions.gradle.kts diff --git a/buildSrc/src/main/kotlin/kover-conventions.gradle.kts b/buildSrc/src/main/kotlin/kover-conventions.gradle.kts new file mode 100644 index 0000000000..3a40331a8e --- /dev/null +++ b/buildSrc/src/main/kotlin/kover-conventions.gradle.kts @@ -0,0 +1,54 @@ +import kotlinx.kover.api.* +import kotlinx.kover.tasks.* + +/* + * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ +apply(plugin = "kover") + +val notCovered = sourceless + internal + unpublished + +val expectedCoverage = mutableMapOf( + // These have lower coverage in general, it can be eventually fixed + "kotlinx-coroutines-swing" to 70, + "kotlinx-coroutines-android" to 50, + "kotlinx-coroutines-javafx" to 39, // JavaFx is not tested on TC because its graphic subsystem cannot be initialized in headless mode + + // TODO figure it out, these probably should be fixed + "kotlinx-coroutines-debug" to 84, + "kotlinx-coroutines-reactive" to 65, + "kotlinx-coroutines-reactor" to 65, + "kotlinx-coroutines-rx2" to 78, + "kotlinx-coroutines-slf4j" to 81 +) + +extensions.configure { + disabledProjects = notCovered + /* + * Is explicitly enabled on TC in a separate build step. + * Examples: + * ./gradlew :p:check -- doesn't verify coverage + * ./gradlew :p:check -Pkover.enabled=true -- verifies coverage + * ./gradlew :p:koverReport -Pkover.enabled=true -- generates report + */ + isDisabled = !(properties["kover.enabled"]?.toString()?.toBoolean() ?: false) +} + +subprojects { + val projectName = name + if (projectName in notCovered) return@subprojects + tasks.withType { + rule { + bound { + /* + * 85 is our baseline that we aim to raise to 90+. + * Missing coverage is typically due to bugs in the agent + * (e.g. signatures deprecated with an error are counted), + * sometimes it's various diagnostic `toString` or `catch` for OOMs/VerificationErrors, + * but some places are definitely worth visiting. + */ + minValue = expectedCoverage[projectName] ?: 85 // COVERED_LINES_PERCENTAGE + } + } + } +} diff --git a/gradle.properties b/gradle.properties index ba143d84e0..922f786fb1 100644 --- a/gradle.properties +++ b/gradle.properties @@ -22,7 +22,8 @@ rxjava2_version=2.2.8 rxjava3_version=3.0.2 javafx_version=11.0.2 javafx_plugin_version=0.0.8 -binary_compatibility_validator_version=0.8.0-RC +binary_compatibility_validator_version=0.8.0 +kover_version=0.5.0-RC2 blockhound_version=1.0.2.RELEASE jna_version=5.9.0 diff --git a/kotlinx-coroutines-core/build.gradle b/kotlinx-coroutines-core/build.gradle index 487527b22b..ac6840d98c 100644 --- a/kotlinx-coroutines-core/build.gradle +++ b/kotlinx-coroutines-core/build.gradle @@ -256,7 +256,7 @@ static void configureJvmForLincheck(task) { task.minHeapSize = '1g' task.maxHeapSize = '4g' // we may need more space for building an interleaving tree in the model checking mode task.jvmArgs = ['--add-opens', 'java.base/jdk.internal.misc=ALL-UNNAMED', // required for transformation - '--add-exports', 'java.base/jdk.internal.util=ALL-UNNAMED'] // in the model checking mode + '--add-exports', 'java.base/jdk.internal.util=ALL-UNNAMED'] // in the model checking mode task.systemProperty 'kotlinx.coroutines.semaphore.segmentSize', '2' task.systemProperty 'kotlinx.coroutines.semaphore.maxSpinCycles', '1' // better for the model checking mode } @@ -265,6 +265,28 @@ static void configureJvmForLincheck(task) { task moreTest(dependsOn: [jvmStressTest, jvmLincheckTest]) check.dependsOn moreTest +tasks.jvmLincheckTest { + kover { + enabled = false // Always disabled, lincheck doesn't really support coverage + } +} + +def commonKoverExcludes = + ["kotlinx.coroutines.test.*", // Deprecated package for removal + "kotlinx.coroutines.debug.*", // Tested by debug module + "kotlinx.coroutines.channels.ChannelsKt__DeprecatedKt.*", // Deprecated + "kotlinx.coroutines.scheduling.LimitingDispatcher", // Deprecated + "kotlinx.coroutines.scheduling.ExperimentalCoroutineDispatcher" // Deprecated + ] + +tasks.koverHtmlReport { + excludes = commonKoverExcludes +} + +tasks.koverVerify { + excludes = commonKoverExcludes +} + task testsJar(type: Jar, dependsOn: jvmTestClasses) { classifier = 'tests' from compileTestKotlinJvm.destinationDir diff --git a/ui/kotlinx-coroutines-android/build.gradle.kts b/ui/kotlinx-coroutines-android/build.gradle.kts index 9cec1dc9f0..145ad9f6b4 100644 --- a/ui/kotlinx-coroutines-android/build.gradle.kts +++ b/ui/kotlinx-coroutines-android/build.gradle.kts @@ -2,6 +2,8 @@ * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ +import kotlinx.kover.api.* + configurations { create("r8") } @@ -103,3 +105,8 @@ open class RunR8 : JavaExec() { } } +tasks.withType { + extensions.configure { + excludes = excludes + listOf("com.android.*", "android.*") // Exclude robolectric-generated classes + } +} From eac7bba54b2c8f49cff57f3e14b090797c95d56a Mon Sep 17 00:00:00 2001 From: Krzysztof Date: Fri, 21 Jan 2022 11:40:56 +0100 Subject: [PATCH 11/29] docs: typo in conflate docs (#3151) --- kotlinx-coroutines-core/common/src/flow/operators/Context.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kotlinx-coroutines-core/common/src/flow/operators/Context.kt b/kotlinx-coroutines-core/common/src/flow/operators/Context.kt index ace01e1d44..8ed5606b83 100644 --- a/kotlinx-coroutines-core/common/src/flow/operators/Context.kt +++ b/kotlinx-coroutines-core/common/src/flow/operators/Context.kt @@ -171,7 +171,7 @@ public fun Flow.buffer(capacity: Int = BUFFERED): Flow = buffer(capaci * ``` * * Note that `conflate` operator is a shortcut for [buffer] with `capacity` of [Channel.CONFLATED][Channel.CONFLATED], - * with is, in turn, a shortcut to a buffer that only keeps the latest element as + * which is, in turn, a shortcut to a buffer that only keeps the latest element as * created by `buffer(onBufferOverflow = `[`BufferOverflow.DROP_OLDEST`][BufferOverflow.DROP_OLDEST]`)`. * * ### Operator fusion From 3e7466a86436a68afc06cc50a24d514da3c36311 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Mon, 24 Jan 2022 18:50:35 +0300 Subject: [PATCH 12/29] Fix typo in CoroutineDispatcher documentation Fixes #3110 --- kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt b/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt index c717d559e9..71b7ec726f 100644 --- a/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt +++ b/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt @@ -84,7 +84,7 @@ public abstract class CoroutineDispatcher : * // At most 2 threads will be processing images as it is really slow and CPU-intensive * private val imageProcessingDispatcher = backgroundDispatcher.limitedParallelism(2) * // At most 3 threads will be processing JSON to avoid image processing starvation - * private val imageProcessingDispatcher = backgroundDispatcher.limitedParallelism(3) + * private val jsonProcessingDispatcher = backgroundDispatcher.limitedParallelism(3) * // At most 1 thread will be doing IO * private val fileWriterDispatcher = backgroundDispatcher.limitedParallelism(1) * ``` From 8136179efb3c2d752c4be409698eb8d425af23e7 Mon Sep 17 00:00:00 2001 From: Doug Marques Date: Tue, 25 Jan 2022 17:25:33 +1000 Subject: [PATCH 13/29] Fixing typo: `distinctUtilChanged` to `distinctUntilChanged` (#3154) --- kotlinx-coroutines-core/common/src/flow/operators/Distinct.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kotlinx-coroutines-core/common/src/flow/operators/Distinct.kt b/kotlinx-coroutines-core/common/src/flow/operators/Distinct.kt index 3166786bec..f211a1b2bf 100644 --- a/kotlinx-coroutines-core/common/src/flow/operators/Distinct.kt +++ b/kotlinx-coroutines-core/common/src/flow/operators/Distinct.kt @@ -15,7 +15,7 @@ import kotlin.native.concurrent.* /** * Returns flow where all subsequent repetitions of the same value are filtered out. * - * Note that any instance of [StateFlow] already behaves as if `distinctUtilChanged` operator is + * Note that any instance of [StateFlow] already behaves as if `distinctUntilChanged` operator is * applied to it, so applying `distinctUntilChanged` to a `StateFlow` has no effect. * See [StateFlow] documentation on Operator Fusion. * Also, repeated application of `distinctUntilChanged` operator on any flow has no effect. From b5b254b1f84876ebae20486d1539f9cf4d5003cb Mon Sep 17 00:00:00 2001 From: OliverO2 Date: Thu, 27 Jan 2022 14:17:32 +0100 Subject: [PATCH 14/29] Fix example code in comment of CopyableThreadContextElement (#3157) --- .../jvm/src/ThreadContextElement.kt | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/kotlinx-coroutines-core/jvm/src/ThreadContextElement.kt b/kotlinx-coroutines-core/jvm/src/ThreadContextElement.kt index 1b825cef01..1a960699c7 100644 --- a/kotlinx-coroutines-core/jvm/src/ThreadContextElement.kt +++ b/kotlinx-coroutines-core/jvm/src/ThreadContextElement.kt @@ -97,26 +97,25 @@ public interface ThreadContextElement : CoroutineContext.Element { * is in a coroutine: * * ``` - * class TraceContextElement(val traceData: TraceData?) : CopyableThreadContextElement { - * companion object Key : CoroutineContext.Key - * override val key: CoroutineContext.Key - * get() = Key + * class TraceContextElement(private val traceData: TraceData?) : CopyableThreadContextElement { + * companion object Key : CoroutineContext.Key + * override val key: CoroutineContext.Key = Key * * override fun updateThreadContext(context: CoroutineContext): TraceData? { * val oldState = traceThreadLocal.get() - * traceThreadLocal.set(data) + * traceThreadLocal.set(traceData) * return oldState * } * - * override fun restoreThreadContext(context: CoroutineContext, oldData: TraceData?) { + * override fun restoreThreadContext(context: CoroutineContext, oldState: TraceData?) { * traceThreadLocal.set(oldState) * } * - * override fun copyForChildCoroutine(): CopyableThreadContextElement { + * override fun copyForChildCoroutine(): CopyableThreadContextElement { * // Copy from the ThreadLocal source of truth at child coroutine launch time. This makes * // ThreadLocal writes between resumption of the parent coroutine and the launch of the * // child coroutine visible to the child. - * return CopyForChildCoroutineElement(traceThreadLocal.get()) + * return TraceContextElement(traceThreadLocal.get()?.copy()) * } * } * ``` From 649d03e971e287355468dce0dcd3b9ffe9adf964 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Tue, 1 Feb 2022 12:29:11 +0300 Subject: [PATCH 15/29] =?UTF-8?q?Confine=20context-specific=20state=20to?= =?UTF-8?q?=20the=20thread=20in=20UndispatchedCoroutine=E2=80=A6=20(#3155)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Confine context-specific state to the thread in UndispatchedCoroutine in order to avoid state interference when the coroutine is updated concurrently. Concurrency is inevitable in this scenario: when the coroutine that has UndispatchedCoroutine as its completion suspends, we have to clear the thread context, but while we are doing so, concurrent resume of the coroutine could've happened that also ends up in save/clear/update context Fixes #2930 --- .../jvm/src/CoroutineContext.kt | 32 +++++---- .../jvm/test/ThreadLocalStressTest.kt | 72 +++++++++++++++++++ 2 files changed, 92 insertions(+), 12 deletions(-) create mode 100644 kotlinx-coroutines-core/jvm/test/ThreadLocalStressTest.kt diff --git a/kotlinx-coroutines-core/jvm/src/CoroutineContext.kt b/kotlinx-coroutines-core/jvm/src/CoroutineContext.kt index d562207f8b..6291ea2b97 100644 --- a/kotlinx-coroutines-core/jvm/src/CoroutineContext.kt +++ b/kotlinx-coroutines-core/jvm/src/CoroutineContext.kt @@ -107,7 +107,7 @@ internal tailrec fun CoroutineStackFrame.undispatchedCompletion(): UndispatchedC /** * Marker indicating that [UndispatchedCoroutine] exists somewhere up in the stack. - * Used as a performance optimization to avoid stack walking where it is not nesessary. + * Used as a performance optimization to avoid stack walking where it is not necessary. */ private object UndispatchedMarker: CoroutineContext.Element, CoroutineContext.Key { override val key: CoroutineContext.Key<*> @@ -120,26 +120,34 @@ internal actual class UndispatchedCoroutineactual constructor ( uCont: Continuation ) : ScopeCoroutine(if (context[UndispatchedMarker] == null) context + UndispatchedMarker else context, uCont) { - private var savedContext: CoroutineContext? = null - private var savedOldValue: Any? = null + /* + * The state is thread-local because this coroutine can be used concurrently. + * Scenario of usage (withContinuationContext): + * val state = saveThreadContext(ctx) + * try { + * invokeSmthWithThisCoroutineAsCompletion() // Completion implies that 'afterResume' will be called + * // COROUTINE_SUSPENDED is returned + * } finally { + * thisCoroutine().clearThreadContext() // Concurrently the "smth" could've been already resumed on a different thread + * // and it also calls saveThreadContext and clearThreadContext + * } + */ + private var threadStateToRecover = ThreadLocal>() fun saveThreadContext(context: CoroutineContext, oldValue: Any?) { - savedContext = context - savedOldValue = oldValue + threadStateToRecover.set(context to oldValue) } fun clearThreadContext(): Boolean { - if (savedContext == null) return false - savedContext = null - savedOldValue = null + if (threadStateToRecover.get() == null) return false + threadStateToRecover.set(null) return true } override fun afterResume(state: Any?) { - savedContext?.let { context -> - restoreThreadContext(context, savedOldValue) - savedContext = null - savedOldValue = null + threadStateToRecover.get()?.let { (ctx, value) -> + restoreThreadContext(ctx, value) + threadStateToRecover.set(null) } // resume undispatched -- update context but stay on the same dispatcher val result = recoverResult(state, uCont) diff --git a/kotlinx-coroutines-core/jvm/test/ThreadLocalStressTest.kt b/kotlinx-coroutines-core/jvm/test/ThreadLocalStressTest.kt new file mode 100644 index 0000000000..f9941d0215 --- /dev/null +++ b/kotlinx-coroutines-core/jvm/test/ThreadLocalStressTest.kt @@ -0,0 +1,72 @@ +/* + * Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines + +import kotlin.test.* + + +class ThreadLocalStressTest : TestBase() { + + private val threadLocal = ThreadLocal() + + // See the comment in doStress for the machinery + @Test + fun testStress() = runTest { + repeat (100 * stressTestMultiplierSqrt) { + withContext(Dispatchers.Default) { + repeat(100) { + launch { + doStress(null) + } + } + } + } + } + + @Test + fun testStressWithOuterValue() = runTest { + repeat (100 * stressTestMultiplierSqrt) { + withContext(Dispatchers.Default + threadLocal.asContextElement("bar")) { + repeat(100) { + launch { + doStress("bar") + } + } + } + } + } + + private suspend fun doStress(expectedValue: String?) { + assertEquals(expectedValue, threadLocal.get()) + try { + /* + * Here we are using very specific code-path to trigger the execution we want to. + * The bug, in general, has a larger impact, but this particular code pinpoints it: + * + * 1) We use _undispatched_ withContext with thread element + * 2) We cancel the coroutine + * 3) We use 'suspendCancellableCoroutineReusable' that does _postponed_ cancellation check + * which makes the reproduction of this race pretty reliable. + * + * Now the following code path is likely to be triggered: + * + * T1 from within 'withContinuationContext' method: + * Finds 'oldValue', finds undispatched completion, invokes its 'block' argument. + * 'block' is this coroutine, it goes to 'trySuspend', checks for postponed cancellation and *dispatches* it. + * The execution stops _right_ before 'undispatchedCompletion.clearThreadContext()'. + * + * T2 now executes the dispatched cancellation and concurrently mutates the state of the undispatched completion. + * All bets are off, now both threads can leave the thread locals state inconsistent. + */ + withContext(threadLocal.asContextElement("foo")) { + yield() + cancel() + suspendCancellableCoroutineReusable { } + } + } finally { + assertEquals(expectedValue, threadLocal.get()) + } + } +} From 76989f739796f409fb3147939e086c3d1af725bf Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Tue, 1 Feb 2022 13:51:36 +0300 Subject: [PATCH 16/29] =?UTF-8?q?Coverage:=20improve=20test=20coverage,=20?= =?UTF-8?q?disable=20deprecations,=20add=20missing=20test=E2=80=A6=20(#316?= =?UTF-8?q?1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses #3156 --- .../main/kotlin/kover-conventions.gradle.kts | 14 +- .../test/MDCContextTest.kt | 3 +- kotlinx-coroutines-debug/build.gradle | 12 + kotlinx-coroutines-debug/test/ToStringTest.kt | 11 + .../build.gradle.kts | 14 ++ .../build.gradle.kts | 12 + .../src/HandlerDispatcher.kt | 2 +- .../test/AndroidExceptionPreHandlerTest.kt | 35 +++ .../test/HandlerDispatcherAsyncTest.kt | 145 ++++++++++++ .../test/HandlerDispatcherTest.kt | 214 +++++++----------- 10 files changed, 325 insertions(+), 137 deletions(-) create mode 100644 ui/kotlinx-coroutines-android/test/AndroidExceptionPreHandlerTest.kt create mode 100644 ui/kotlinx-coroutines-android/test/HandlerDispatcherAsyncTest.kt diff --git a/buildSrc/src/main/kotlin/kover-conventions.gradle.kts b/buildSrc/src/main/kotlin/kover-conventions.gradle.kts index 3a40331a8e..125ddb19ea 100644 --- a/buildSrc/src/main/kotlin/kover-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/kover-conventions.gradle.kts @@ -10,16 +10,12 @@ val notCovered = sourceless + internal + unpublished val expectedCoverage = mutableMapOf( // These have lower coverage in general, it can be eventually fixed - "kotlinx-coroutines-swing" to 70, - "kotlinx-coroutines-android" to 50, + "kotlinx-coroutines-swing" to 70, // awaitFrame is not tested "kotlinx-coroutines-javafx" to 39, // JavaFx is not tested on TC because its graphic subsystem cannot be initialized in headless mode - // TODO figure it out, these probably should be fixed - "kotlinx-coroutines-debug" to 84, - "kotlinx-coroutines-reactive" to 65, + // Re-evaluate this along with Kover update where deprecated with error+ functions are not considered as uncovered: IDEA-287459 "kotlinx-coroutines-reactor" to 65, - "kotlinx-coroutines-rx2" to 78, - "kotlinx-coroutines-slf4j" to 81 + "kotlinx-coroutines-rx2" to 78 ) extensions.configure { @@ -51,4 +47,8 @@ subprojects { } } } + + tasks.withType { + htmlReportDir.set(file(rootProject.buildDir.toString() + "/kover/" + project.name + "/html")) + } } diff --git a/integration/kotlinx-coroutines-slf4j/test/MDCContextTest.kt b/integration/kotlinx-coroutines-slf4j/test/MDCContextTest.kt index 7d18359c5d..532c47e9ed 100644 --- a/integration/kotlinx-coroutines-slf4j/test/MDCContextTest.kt +++ b/integration/kotlinx-coroutines-slf4j/test/MDCContextTest.kt @@ -102,9 +102,10 @@ class MDCContextTest : TestBase() { val mainDispatcher = kotlin.coroutines.coroutineContext[ContinuationInterceptor]!! withContext(Dispatchers.Default + MDCContext()) { assertEquals("myValue", MDC.get("myKey")) + assertEquals("myValue", coroutineContext[MDCContext]?.contextMap?.get("myKey")) withContext(mainDispatcher) { assertEquals("myValue", MDC.get("myKey")) } } } -} \ No newline at end of file +} diff --git a/kotlinx-coroutines-debug/build.gradle b/kotlinx-coroutines-debug/build.gradle index 4830670d24..c01e70463f 100644 --- a/kotlinx-coroutines-debug/build.gradle +++ b/kotlinx-coroutines-debug/build.gradle @@ -51,3 +51,15 @@ shadowJar { configurations = [project.configurations.shadowDeps] relocate('net.bytebuddy', 'kotlinx.coroutines.repackaged.net.bytebuddy') } + +def commonKoverExcludes = + // Never used, safety mechanism + ["kotlinx.coroutines.debug.internal.NoOpProbesKt"] + +tasks.koverHtmlReport { + excludes = commonKoverExcludes +} + +tasks.koverVerify { + excludes = commonKoverExcludes +} diff --git a/kotlinx-coroutines-debug/test/ToStringTest.kt b/kotlinx-coroutines-debug/test/ToStringTest.kt index 0a9e84efad..0ea412b55d 100644 --- a/kotlinx-coroutines-debug/test/ToStringTest.kt +++ b/kotlinx-coroutines-debug/test/ToStringTest.kt @@ -8,6 +8,7 @@ import kotlinx.coroutines.* import kotlinx.coroutines.channels.* import org.junit.* import org.junit.Test +import java.io.* import kotlin.coroutines.* import kotlin.test.* @@ -105,6 +106,8 @@ class ToStringTest : TestBase() { expect(6) assertEquals(expected, DebugProbes.jobToString(root).trimEnd().trimStackTrace().trimPackage()) assertEquals(expected, DebugProbes.scopeToString(CoroutineScope(root)).trimEnd().trimStackTrace().trimPackage()) + assertEquals(expected, printToString { DebugProbes.printScope(CoroutineScope(root), it) }.trimEnd().trimStackTrace().trimPackage()) + assertEquals(expected, printToString { DebugProbes.printJob(root, it) }.trimEnd().trimStackTrace().trimPackage()) root.cancelAndJoin() finish(7) @@ -145,4 +148,12 @@ class ToStringTest : TestBase() { } } } + + private inline fun printToString(block: (PrintStream) -> Unit): String { + val baos = ByteArrayOutputStream() + val ps = PrintStream(baos) + block(ps) + ps.close() + return baos.toString() + } } diff --git a/reactive/kotlinx-coroutines-reactive/build.gradle.kts b/reactive/kotlinx-coroutines-reactive/build.gradle.kts index 128d4d86ab..b624069c60 100644 --- a/reactive/kotlinx-coroutines-reactive/build.gradle.kts +++ b/reactive/kotlinx-coroutines-reactive/build.gradle.kts @@ -34,3 +34,17 @@ tasks.check { externalDocumentationLink( url = "https://www.reactive-streams.org/reactive-streams-$reactiveStreamsVersion-javadoc/" ) + +val commonKoverExcludes = listOf( + "kotlinx.coroutines.reactive.FlowKt", // Deprecated + "kotlinx.coroutines.reactive.FlowKt__MigrationKt", // Deprecated + "kotlinx.coroutines.reactive.ConvertKt" // Deprecated +) + +tasks.koverHtmlReport { + excludes = commonKoverExcludes +} + +tasks.koverVerify { + excludes = commonKoverExcludes +} diff --git a/reactive/kotlinx-coroutines-reactor/build.gradle.kts b/reactive/kotlinx-coroutines-reactor/build.gradle.kts index 03af7f4fda..5abf3862dc 100644 --- a/reactive/kotlinx-coroutines-reactor/build.gradle.kts +++ b/reactive/kotlinx-coroutines-reactor/build.gradle.kts @@ -27,3 +27,15 @@ tasks { externalDocumentationLink( url = "https://projectreactor.io/docs/core/$reactorVersion/api/" ) + +val commonKoverExcludes = listOf( + "kotlinx.coroutines.reactor.FlowKt" // Deprecated +) + +tasks.koverHtmlReport { + excludes = commonKoverExcludes +} + +tasks.koverVerify { + excludes = commonKoverExcludes +} diff --git a/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt b/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt index ffd5df060c..5e33169dd1 100644 --- a/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt +++ b/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt @@ -190,7 +190,7 @@ public suspend fun awaitFrame(): Long { postFrameCallback(choreographer, cont) } } - // post into looper thread thread to figure it out + // post into looper thread to figure it out return suspendCancellableCoroutine { cont -> Dispatchers.Main.dispatch(EmptyCoroutineContext, Runnable { updateChoreographerAndPostFrameCallback(cont) diff --git a/ui/kotlinx-coroutines-android/test/AndroidExceptionPreHandlerTest.kt b/ui/kotlinx-coroutines-android/test/AndroidExceptionPreHandlerTest.kt new file mode 100644 index 0000000000..4aa44eceaf --- /dev/null +++ b/ui/kotlinx-coroutines-android/test/AndroidExceptionPreHandlerTest.kt @@ -0,0 +1,35 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.android + +import kotlinx.coroutines.* +import org.junit.Test +import org.junit.runner.* +import org.robolectric.* +import org.robolectric.annotation.* +import kotlin.test.* + +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, sdk = [27]) +class AndroidExceptionPreHandlerTest : TestBase() { + @Test + fun testUnhandledException() = runTest { + val previousHandler = Thread.getDefaultUncaughtExceptionHandler() + try { + Thread.setDefaultUncaughtExceptionHandler { _, e -> + expect(3) + assertIs(e) + } + expect(1) + GlobalScope.launch(Dispatchers.Main) { + expect(2) + throw TestException() + }.join() + finish(4) + } finally { + Thread.setDefaultUncaughtExceptionHandler(previousHandler) + } + } +} diff --git a/ui/kotlinx-coroutines-android/test/HandlerDispatcherAsyncTest.kt b/ui/kotlinx-coroutines-android/test/HandlerDispatcherAsyncTest.kt new file mode 100644 index 0000000000..7b03e771f9 --- /dev/null +++ b/ui/kotlinx-coroutines-android/test/HandlerDispatcherAsyncTest.kt @@ -0,0 +1,145 @@ +/* + * Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.android + +import android.os.* +import kotlinx.coroutines.* +import org.junit.Test +import org.junit.runner.* +import org.robolectric.* +import org.robolectric.Shadows.* +import org.robolectric.annotation.* +import org.robolectric.shadows.* +import org.robolectric.util.* +import java.util.concurrent.* +import kotlin.test.* + +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, sdk = [28]) +class HandlerDispatcherAsyncTest : TestBase() { + + /** + * Because [Dispatchers.Main] is a singleton, we cannot vary its initialization behavior. As a + * result we only test its behavior on the newest API level and assert that it uses async + * messages. We rely on the other tests to exercise the variance of the mechanism that the main + * dispatcher uses to ensure it has correct behavior on all API levels. + */ + @Test + fun mainIsAsync() = runTest { + ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 28) + + val mainLooper = shadowOf(Looper.getMainLooper()) + mainLooper.pause() + val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) + + val job = launch(Dispatchers.Main) { + expect(2) + } + + val message = mainMessageQueue.head + assertTrue(message.isAsynchronous) + job.join(mainLooper) + } + + @Test + fun asyncMessagesApi14() = runTest { + ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 14) + + val main = Looper.getMainLooper().asHandler(async = true).asCoroutineDispatcher() + + val mainLooper = shadowOf(Looper.getMainLooper()) + mainLooper.pause() + val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) + + val job = launch(main) { + expect(2) + } + + val message = mainMessageQueue.head + assertFalse(message.isAsynchronous) + job.join(mainLooper) + } + + @Test + fun asyncMessagesApi16() = runTest { + ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 16) + + val main = Looper.getMainLooper().asHandler(async = true).asCoroutineDispatcher() + + val mainLooper = shadowOf(Looper.getMainLooper()) + mainLooper.pause() + val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) + + val job = launch(main) { + expect(2) + } + + val message = mainMessageQueue.head + assertTrue(message.isAsynchronous) + job.join(mainLooper) + } + + @Test + fun asyncMessagesApi28() = runTest { + ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 28) + + val main = Looper.getMainLooper().asHandler(async = true).asCoroutineDispatcher() + + val mainLooper = shadowOf(Looper.getMainLooper()) + mainLooper.pause() + val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) + + val job = launch(main) { + expect(2) + } + + val message = mainMessageQueue.head + assertTrue(message.isAsynchronous) + job.join(mainLooper) + } + + @Test + fun noAsyncMessagesIfNotRequested() = runTest { + ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 28) + + val main = Looper.getMainLooper().asHandler(async = false).asCoroutineDispatcher() + + val mainLooper = shadowOf(Looper.getMainLooper()) + mainLooper.pause() + val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) + + val job = launch(main) { + expect(2) + } + + val message = mainMessageQueue.head + assertFalse(message.isAsynchronous) + job.join(mainLooper) + } + + @Test + fun testToString() { + ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 28) + val main = Looper.getMainLooper().asHandler(async = true).asCoroutineDispatcher("testName") + assertEquals("testName", main.toString()) + assertEquals("testName.immediate", main.immediate.toString()) + assertEquals("testName.immediate", main.immediate.immediate.toString()) + } + + private suspend fun Job.join(mainLooper: ShadowLooper) { + expect(1) + mainLooper.unPause() + join() + finish(3) + } + + // TODO compile against API 23+ so this can be invoked without reflection. + private val Looper.queue: MessageQueue + get() = Looper::class.java.getDeclaredMethod("getQueue").invoke(this) as MessageQueue + + // TODO compile against API 22+ so this can be invoked without reflection. + private val Message.isAsynchronous: Boolean + get() = Message::class.java.getDeclaredMethod("isAsynchronous").invoke(this) as Boolean +} diff --git a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt index af17adfc00..24758444b0 100644 --- a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt +++ b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ package kotlinx.coroutines.android @@ -9,173 +9,131 @@ import kotlinx.coroutines.* import org.junit.Test import org.junit.runner.* import org.robolectric.* -import org.robolectric.Shadows.* import org.robolectric.annotation.* import org.robolectric.shadows.* -import org.robolectric.util.* +import java.util.concurrent.* import kotlin.test.* @RunWith(RobolectricTestRunner::class) @Config(manifest = Config.NONE, sdk = [28]) class HandlerDispatcherTest : TestBase() { - - /** - * Because [Dispatchers.Main] is a singleton, we cannot vary its initialization behavior. As a - * result we only test its behavior on the newest API level and assert that it uses async - * messages. We rely on the other tests to exercise the variance of the mechanism that the main - * dispatcher uses to ensure it has correct behavior on all API levels. - */ @Test - fun mainIsAsync() = runTest { - ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 28) - - val mainLooper = shadowOf(Looper.getMainLooper()) - mainLooper.pause() - val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) - - val job = launch(Dispatchers.Main) { + fun testImmediateDispatcherYield() = runBlocking(Dispatchers.Main) { + expect(1) + // launch in the immediate dispatcher + launch(Dispatchers.Main.immediate) { expect(2) + yield() + expect(4) } - - val message = mainMessageQueue.head - assertTrue(message.isAsynchronous) - job.join(mainLooper) + expect(3) // after yield + yield() // yield back + finish(5) } @Test - fun asyncMessagesApi14() = runTest { - ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 14) - - val main = Looper.getMainLooper().asHandler(async = true).asCoroutineDispatcher() - - val mainLooper = shadowOf(Looper.getMainLooper()) - mainLooper.pause() - val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) - - val job = launch(main) { - expect(2) - } - - val message = mainMessageQueue.head - assertFalse(message.isAsynchronous) - job.join(mainLooper) + fun testMainDispatcherToString() { + assertEquals("Dispatchers.Main", Dispatchers.Main.toString()) + assertEquals("Dispatchers.Main.immediate", Dispatchers.Main.immediate.toString()) } @Test - fun asyncMessagesApi16() = runTest { - ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 16) - - val main = Looper.getMainLooper().asHandler(async = true).asCoroutineDispatcher() - - val mainLooper = shadowOf(Looper.getMainLooper()) + fun testDefaultDelayIsNotDelegatedToMain() = runTest { + val mainLooper = Shadows.shadowOf(Looper.getMainLooper()) mainLooper.pause() - val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) + assertFalse { mainLooper.scheduler.areAnyRunnable() } - val job = launch(main) { - expect(2) + val job = launch(Dispatchers.Default, start = CoroutineStart.UNDISPATCHED) { + expect(1) + delay(Long.MAX_VALUE) + expectUnreached() } - - val message = mainMessageQueue.head - assertTrue(message.isAsynchronous) - job.join(mainLooper) + expect(2) + assertEquals(0, mainLooper.scheduler.size()) + job.cancelAndJoin() + finish(3) } @Test - fun asyncMessagesApi28() = runTest { - ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 28) - - val main = Looper.getMainLooper().asHandler(async = true).asCoroutineDispatcher() - - val mainLooper = shadowOf(Looper.getMainLooper()) + fun testWithTimeoutIsDelegatedToMain() = runTest { + val mainLooper = Shadows.shadowOf(Looper.getMainLooper()) mainLooper.pause() - val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) - - val job = launch(main) { - expect(2) + assertFalse { mainLooper.scheduler.areAnyRunnable() } + val job = launch(Dispatchers.Main, start = CoroutineStart.UNDISPATCHED) { + withTimeout(1) { + expect(1) + hang { expect(3) } + } + expectUnreached() } - - val message = mainMessageQueue.head - assertTrue(message.isAsynchronous) - job.join(mainLooper) + expect(2) + assertEquals(1, mainLooper.scheduler.size()) + // Schedule cancellation + mainLooper.runToEndOfTasks() + job.join() + finish(4) } @Test - fun noAsyncMessagesIfNotRequested() = runTest { - ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 28) - - val main = Looper.getMainLooper().asHandler(async = false).asCoroutineDispatcher() - - val mainLooper = shadowOf(Looper.getMainLooper()) + fun testDelayDelegatedToMain() = runTest { + val mainLooper = Shadows.shadowOf(Looper.getMainLooper()) mainLooper.pause() - val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) - - val job = launch(main) { - expect(2) + val job = launch(Dispatchers.Main, start = CoroutineStart.UNDISPATCHED) { + expect(1) + delay(1) + expect(3) } - - val message = mainMessageQueue.head - assertFalse(message.isAsynchronous) - job.join(mainLooper) + expect(2) + assertEquals(1, mainLooper.scheduler.size()) + // Schedule cancellation + mainLooper.runToEndOfTasks() + job.join() + finish(4) } @Test - fun testToString() { - ReflectionHelpers.setStaticField(Build.VERSION::class.java, "SDK_INT", 28) - val main = Looper.getMainLooper().asHandler(async = true).asCoroutineDispatcher("testName") - assertEquals("testName", main.toString()) - assertEquals("testName.immediate", main.immediate.toString()) - assertEquals("testName.immediate", main.immediate.immediate.toString()) - } + fun testAwaitFrame() = runTest { + doTestAwaitFrame() - private suspend fun Job.join(mainLooper: ShadowLooper) { - expect(1) - mainLooper.unPause() - join() - finish(3) - } + reset() - // TODO compile against API 23+ so this can be invoked without reflection. - private val Looper.queue: MessageQueue - get() = Looper::class.java.getDeclaredMethod("getQueue").invoke(this) as MessageQueue - - // TODO compile against API 22+ so this can be invoked without reflection. - private val Message.isAsynchronous: Boolean - get() = Message::class.java.getDeclaredMethod("isAsynchronous").invoke(this) as Boolean - - @Test - fun testImmediateDispatcherYield() = runBlocking(Dispatchers.Main) { - expect(1) - // launch in the immediate dispatcher - launch(Dispatchers.Main.immediate) { - expect(2) - yield() - expect(4) - } - expect(3) // after yield - yield() // yield back - finish(5) + // Now the second test: we cannot test it separately because we're caching choreographer in HandlerDispatcher + doTestAwaitWithDetectedChoreographer() } - @Test - fun testMainDispatcherToString() { - assertEquals("Dispatchers.Main", Dispatchers.Main.toString()) - assertEquals("Dispatchers.Main.immediate", Dispatchers.Main.immediate.toString()) + private fun CoroutineScope.doTestAwaitFrame() { + ShadowChoreographer.setPostFrameCallbackDelay(100) + val mainLooper = Shadows.shadowOf(Looper.getMainLooper()) + mainLooper.pause() + launch(Dispatchers.Main, start = CoroutineStart.UNDISPATCHED) { + expect(1) + awaitFrame() + expect(5) + } + expect(2) + // Run choreographer detection + mainLooper.runOneTask() + expect(3) + mainLooper.scheduler.advanceBy(50, TimeUnit.MILLISECONDS) + expect(4) + mainLooper.scheduler.advanceBy(51, TimeUnit.MILLISECONDS) + finish(6) } - @Test - fun testDelayIsNotDelegatedToMain() = runTest { - val mainLooper = shadowOf(Looper.getMainLooper()) - mainLooper.pause() - val mainMessageQueue = shadowOf(Looper.getMainLooper().queue) - assertNull(mainMessageQueue.head) - val job = launch(Dispatchers.Default, start = CoroutineStart.UNDISPATCHED) { + private fun CoroutineScope.doTestAwaitWithDetectedChoreographer() { + ShadowChoreographer.reset() + ShadowChoreographer.setPostFrameCallbackDelay(100) + val mainLooper = Shadows.shadowOf(Looper.getMainLooper()) + launch(Dispatchers.Main, start = CoroutineStart.UNDISPATCHED) { expect(1) - delay(Long.MAX_VALUE) - expectUnreached() + awaitFrame() + expect(4) } + // Run choreographer detection expect(2) - assertNull(mainMessageQueue.head) - job.cancelAndJoin() - finish(3) + mainLooper.scheduler.advanceBy(50, TimeUnit.MILLISECONDS) + expect(3) + mainLooper.scheduler.advanceBy(51, TimeUnit.MILLISECONDS) + finish(5) } } From ff80ba2f8eeadf73396e100b08e0df138d2c8618 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Tue, 1 Feb 2022 16:51:01 +0300 Subject: [PATCH 17/29] Introduce private DiagnosticCoroutineContextException and add it to the original exception prior to passing it to the Thread.currentThread().uncaughtExceptionHandler() (#3170) Fixes #3153 --- .../jvm/src/CoroutineExceptionHandlerImpl.kt | 21 +++++++++++++++++++ .../CoroutineExceptionHandlerJvmTest.kt | 12 +++++++++++ 2 files changed, 33 insertions(+) diff --git a/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt index 4c8c81b8db..dd39210d25 100644 --- a/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt +++ b/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt @@ -27,6 +27,24 @@ internal actual fun initializeDefaultExceptionHandlers() { CoroutineExceptionHandler } +/** + * Private exception without stacktrace that is added to suppressed exceptions of the original exception + * when it is reported to the last-ditch current thread 'uncaughtExceptionHandler'. + * + * The purpose of this exception is to add an otherwise inaccessible diagnostic information and to + * be able to poke the failing coroutine context in the debugger. + */ +private class DiagnosticCoroutineContextException(private val context: CoroutineContext) : RuntimeException() { + override fun getLocalizedMessage(): String { + return context.toString() + } + + override fun fillInStackTrace(): Throwable { + // Prevent Android <= 6.0 bug, #1866 + stackTrace = emptyArray() + return this + } +} internal actual fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable) { // use additional extension handlers @@ -42,5 +60,8 @@ internal actual fun handleCoroutineExceptionImpl(context: CoroutineContext, exce // use thread's handler val currentThread = Thread.currentThread() + // addSuppressed is never user-defined and cannot normally throw with the only exception being OOM + // we do ignore that just in case to definitely deliver the exception + runCatching { exception.addSuppressed(DiagnosticCoroutineContextException(context)) } currentThread.uncaughtExceptionHandler.uncaughtException(currentThread, exception) } diff --git a/kotlinx-coroutines-core/jvm/test/exceptions/CoroutineExceptionHandlerJvmTest.kt b/kotlinx-coroutines-core/jvm/test/exceptions/CoroutineExceptionHandlerJvmTest.kt index cea9713f4a..2095f14886 100644 --- a/kotlinx-coroutines-core/jvm/test/exceptions/CoroutineExceptionHandlerJvmTest.kt +++ b/kotlinx-coroutines-core/jvm/test/exceptions/CoroutineExceptionHandlerJvmTest.kt @@ -39,4 +39,16 @@ class CoroutineExceptionHandlerJvmTest : TestBase() { finish(3) } + + @Test + fun testLastDitchHandlerContainsContextualInformation() = runBlocking { + expect(1) + GlobalScope.launch(CoroutineName("last-ditch")) { + expect(2) + throw TestException() + }.join() + assertTrue(caughtException is TestException) + assertContains(caughtException.suppressed[0].toString(), "last-ditch") + finish(3) + } } From 70d0ec7b81e7a1fa4f6ad9de9b0b38c98e34611b Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Fri, 11 Feb 2022 02:36:27 -0800 Subject: [PATCH 18/29] Update Dokka to 1.6.10 (#3184) * Update Dokka to 1.6.10 --- build.gradle | 2 -- buildSrc/build.gradle.kts | 1 - buildSrc/settings.gradle.kts | 1 - gradle.properties | 2 +- gradle/dokka.gradle.kts | 6 +----- 5 files changed, 2 insertions(+), 10 deletions(-) diff --git a/build.gradle b/build.gradle index d07f4767d9..bbf24f21d8 100644 --- a/build.gradle +++ b/build.gradle @@ -46,7 +46,6 @@ buildscript { mavenCentral() maven { url "https://plugins.gradle.org/m2/" } maven { url "https://maven.pkg.jetbrains.space/kotlin/p/kotlin/dev" } - maven { url 'https://maven.pkg.jetbrains.space/kotlin/p/dokka/dev' } mavenLocal() } @@ -128,7 +127,6 @@ allprojects { google() mavenCentral() maven { url "https://maven.pkg.jetbrains.space/kotlin/p/kotlin/dev" } - maven { url 'https://maven.pkg.jetbrains.space/kotlin/p/dokka/dev' } } } diff --git a/buildSrc/build.gradle.kts b/buildSrc/build.gradle.kts index 2637980a24..f4b64b3560 100644 --- a/buildSrc/build.gradle.kts +++ b/buildSrc/build.gradle.kts @@ -19,7 +19,6 @@ repositories { maven("https://plugins.gradle.org/m2") } maven("https://maven.pkg.jetbrains.space/kotlin/p/kotlin/dev") - maven("https://maven.pkg.jetbrains.space/kotlin/p/dokka/dev") if (buildSnapshotTrain) { mavenLocal() } diff --git a/buildSrc/settings.gradle.kts b/buildSrc/settings.gradle.kts index e30c3ee597..c2e859f65d 100644 --- a/buildSrc/settings.gradle.kts +++ b/buildSrc/settings.gradle.kts @@ -4,7 +4,6 @@ pluginManagement { val build_snapshot_train: String? by settings repositories { - maven(url = "https://maven.pkg.jetbrains.space/kotlin/p/dokka/dev/") val cacheRedirectorEnabled = System.getenv("CACHE_REDIRECTOR")?.toBoolean() == true if (cacheRedirectorEnabled) { println("Redirecting repositories for buildSrc buildscript") diff --git a/gradle.properties b/gradle.properties index 922f786fb1..7794ba71aa 100644 --- a/gradle.properties +++ b/gradle.properties @@ -14,7 +14,7 @@ atomicfu_version=0.17.0 knit_version=0.3.0 html_version=0.7.2 lincheck_version=2.14 -dokka_version=1.6.0-dev-138 +dokka_version=1.6.10 byte_buddy_version=1.10.9 reactor_version=3.4.1 reactive_streams_version=1.0.3 diff --git a/gradle/dokka.gradle.kts b/gradle/dokka.gradle.kts index a4926f7e61..8be4fbe231 100644 --- a/gradle/dokka.gradle.kts +++ b/gradle/dokka.gradle.kts @@ -37,7 +37,7 @@ tasks.withType(DokkaTaskPartial::class).configureEach { packageListUrl.set(rootProject.projectDir.toPath().resolve("site/stdlib.package.list").toUri().toURL()) } - if (project.name != "kotlinx-coroutines-core" && project.name != "kotlinx-coroutines-test") { + if (!project.isMultiplatform) { dependsOn(project.configurations["compileClasspath"]) doFirst { // resolve classpath only during execution @@ -66,10 +66,6 @@ if (project.name == "kotlinx-coroutines-core") { val jvmMain by getting { makeLinkMapping(project.file("jvm")) } - - configureEach { - classpath.from(project.configurations["jvmCompileClasspath"].files) - } } } } From ed09ff858385a4e68fdcd7f126b6d02addc91798 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Wed, 16 Feb 2022 01:13:32 -0800 Subject: [PATCH 19/29] Update Kover to 0.5.0 (#3183) * Update Kover to 0.5.0 * Update robolectric to workaround https://github.com/robolectric/robolectric/issues/5456 and then workaround all the update consequences --- buildSrc/build.gradle.kts | 6 +++- buildSrc/src/main/kotlin/UnpackAar.kt | 33 ++++++++++++++++++- .../main/kotlin/kover-conventions.gradle.kts | 8 ++--- gradle.properties | 4 +-- .../build.gradle.kts | 31 ++++------------- .../build.gradle.kts | 3 +- .../android-unit-tests/build.gradle.kts | 9 ++++- .../tests/CustomizedRobolectricTest.kt | 3 +- .../ordered/tests/FirstRobolectricTest.kt | 1 + .../build.gradle.kts | 12 +++++-- .../test/AndroidExceptionPreHandlerTest.kt | 1 + .../test/DisabledHandlerTest.kt | 1 + .../test/HandlerDispatcherAsyncTest.kt | 1 + .../test/HandlerDispatcherTest.kt | 2 +- 14 files changed, 76 insertions(+), 39 deletions(-) diff --git a/buildSrc/build.gradle.kts b/buildSrc/build.gradle.kts index f4b64b3560..eaa03f2f15 100644 --- a/buildSrc/build.gradle.kts +++ b/buildSrc/build.gradle.kts @@ -59,5 +59,9 @@ dependencies { exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib") } implementation("ru.vyarus:gradle-animalsniffer-plugin:1.5.3") // Android API check - implementation("org.jetbrains.kotlinx:kover:${version("kover")}") + implementation("org.jetbrains.kotlinx:kover:${version("kover")}") { + exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk8") + exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk7") + exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib") + } } diff --git a/buildSrc/src/main/kotlin/UnpackAar.kt b/buildSrc/src/main/kotlin/UnpackAar.kt index b3152d7ab0..afe2627a3d 100644 --- a/buildSrc/src/main/kotlin/UnpackAar.kt +++ b/buildSrc/src/main/kotlin/UnpackAar.kt @@ -2,18 +2,49 @@ * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ +import org.gradle.api.* import org.gradle.api.artifacts.transform.InputArtifact import org.gradle.api.artifacts.transform.TransformAction import org.gradle.api.artifacts.transform.TransformOutputs import org.gradle.api.artifacts.transform.TransformParameters +import org.gradle.api.attributes.* import org.gradle.api.file.FileSystemLocation import org.gradle.api.provider.Provider +import org.gradle.kotlin.dsl.* import java.io.File import java.nio.file.Files import java.util.zip.ZipEntry import java.util.zip.ZipFile -// TODO move back to kotlinx-coroutines-play-services when it's migrated to the kts +// Attributes used by aar dependencies +val artifactType = Attribute.of("artifactType", String::class.java) +val unpackedAar = Attribute.of("unpackedAar", Boolean::class.javaObjectType) + +fun Project.configureAar() = configurations.configureEach { + afterEvaluate { + if (isCanBeResolved && !isCanBeConsumed) { + attributes.attribute(unpackedAar, true) // request all AARs to be unpacked + } + } +} + +fun DependencyHandlerScope.configureAarUnpacking() { + attributesSchema { + attribute(unpackedAar) + } + + artifactTypes { + create("aar") { + attributes.attribute(unpackedAar, false) + } + } + + registerTransform(UnpackAar::class.java) { + from.attribute(unpackedAar, false).attribute(artifactType, "aar") + to.attribute(unpackedAar, true).attribute(artifactType, "jar") + } +} + @Suppress("UnstableApiUsage") abstract class UnpackAar : TransformAction { @get:InputArtifact diff --git a/buildSrc/src/main/kotlin/kover-conventions.gradle.kts b/buildSrc/src/main/kotlin/kover-conventions.gradle.kts index 125ddb19ea..052e2bb684 100644 --- a/buildSrc/src/main/kotlin/kover-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/kover-conventions.gradle.kts @@ -13,10 +13,8 @@ val expectedCoverage = mutableMapOf( "kotlinx-coroutines-swing" to 70, // awaitFrame is not tested "kotlinx-coroutines-javafx" to 39, // JavaFx is not tested on TC because its graphic subsystem cannot be initialized in headless mode - // Re-evaluate this along with Kover update where deprecated with error+ functions are not considered as uncovered: IDEA-287459 - "kotlinx-coroutines-reactor" to 65, - "kotlinx-coroutines-rx2" to 78 -) + // Reactor has lower coverage in general due to various fatal error handling features + "kotlinx-coroutines-reactor" to 75) extensions.configure { disabledProjects = notCovered @@ -28,6 +26,8 @@ extensions.configure { * ./gradlew :p:koverReport -Pkover.enabled=true -- generates report */ isDisabled = !(properties["kover.enabled"]?.toString()?.toBoolean() ?: false) + // TODO remove when updating Kover to version 0.5.x + intellijEngineVersion.set("1.0.657") } subprojects { diff --git a/gradle.properties b/gradle.properties index 7794ba71aa..e06f587df7 100644 --- a/gradle.properties +++ b/gradle.properties @@ -23,14 +23,14 @@ rxjava3_version=3.0.2 javafx_version=11.0.2 javafx_plugin_version=0.0.8 binary_compatibility_validator_version=0.8.0 -kover_version=0.5.0-RC2 +kover_version=0.5.0 blockhound_version=1.0.2.RELEASE jna_version=5.9.0 # Android versions android_version=4.1.1.4 androidx_annotation_version=1.1.0 -robolectric_version=4.0.2 +robolectric_version=4.4 baksmali_version=2.2.7 # JS diff --git a/integration/kotlinx-coroutines-play-services/build.gradle.kts b/integration/kotlinx-coroutines-play-services/build.gradle.kts index 59f3b0bd5a..9f8a128703 100644 --- a/integration/kotlinx-coroutines-play-services/build.gradle.kts +++ b/integration/kotlinx-coroutines-play-services/build.gradle.kts @@ -4,36 +4,17 @@ val tasksVersion = "16.0.1" -val artifactType = Attribute.of("artifactType", String::class.java) -val unpackedAar = Attribute.of("unpackedAar", Boolean::class.javaObjectType) - -configurations.configureEach { - afterEvaluate { - if (isCanBeResolved) { - attributes.attribute(unpackedAar, true) // request all AARs to be unpacked - } - } -} +project.configureAar() dependencies { - attributesSchema { - attribute(unpackedAar) - } - - artifactTypes { - create("aar") { - attributes.attribute(unpackedAar, false) - } - } - - registerTransform(UnpackAar::class.java) { - from.attribute(unpackedAar, false).attribute(artifactType, "aar") - to.attribute(unpackedAar, true).attribute(artifactType, "jar") - } - + configureAarUnpacking() api("com.google.android.gms:play-services-tasks:$tasksVersion") { exclude(group="com.android.support") } + + // Required by robolectric + testImplementation("androidx.test:core:1.2.0") + testImplementation("androidx.test:monitor:1.2.0") } externalDocumentationLink( diff --git a/reactive/kotlinx-coroutines-reactor/build.gradle.kts b/reactive/kotlinx-coroutines-reactor/build.gradle.kts index 5abf3862dc..1a36ccec28 100644 --- a/reactive/kotlinx-coroutines-reactor/build.gradle.kts +++ b/reactive/kotlinx-coroutines-reactor/build.gradle.kts @@ -29,7 +29,8 @@ externalDocumentationLink( ) val commonKoverExcludes = listOf( - "kotlinx.coroutines.reactor.FlowKt" // Deprecated + "kotlinx.coroutines.reactor.FlowKt", // Deprecated + "kotlinx.coroutines.reactor.ConvertKt\$asFlux$1" // Deprecated ) tasks.koverHtmlReport { diff --git a/ui/kotlinx-coroutines-android/android-unit-tests/build.gradle.kts b/ui/kotlinx-coroutines-android/android-unit-tests/build.gradle.kts index 2fd0b81479..625ce728b1 100644 --- a/ui/kotlinx-coroutines-android/android-unit-tests/build.gradle.kts +++ b/ui/kotlinx-coroutines-android/android-unit-tests/build.gradle.kts @@ -2,10 +2,17 @@ * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ +project.configureAar() + dependencies { - kotlinCompilerPluginClasspathMain(project(":kotlinx-coroutines-core")) + configureAarUnpacking() + testImplementation("com.google.android:android:${version("android")}") testImplementation("org.robolectric:robolectric:${version("robolectric")}") + // Required by robolectric + testImplementation("androidx.test:core:1.2.0") + testImplementation("androidx.test:monitor:1.2.0") + testImplementation(project(":kotlinx-coroutines-test")) testImplementation(project(":kotlinx-coroutines-android")) } diff --git a/ui/kotlinx-coroutines-android/android-unit-tests/test/ordered/tests/CustomizedRobolectricTest.kt b/ui/kotlinx-coroutines-android/android-unit-tests/test/ordered/tests/CustomizedRobolectricTest.kt index bcc12d5441..676ee4310d 100644 --- a/ui/kotlinx-coroutines-android/android-unit-tests/test/ordered/tests/CustomizedRobolectricTest.kt +++ b/ui/kotlinx-coroutines-android/android-unit-tests/test/ordered/tests/CustomizedRobolectricTest.kt @@ -26,6 +26,7 @@ class InitMainDispatcherBeforeRobolectricTestRunner(testClass: Class<*>) : Robol @Config(manifest = Config.NONE, sdk = [28]) @RunWith(InitMainDispatcherBeforeRobolectricTestRunner::class) +@LooperMode(LooperMode.Mode.LEGACY) class CustomizedRobolectricTest : TestBase() { @Test fun testComponent() { @@ -52,4 +53,4 @@ class CustomizedRobolectricTest : TestBase() { mainLooper.unPause() assertTrue(component.launchCompleted) } -} \ No newline at end of file +} diff --git a/ui/kotlinx-coroutines-android/android-unit-tests/test/ordered/tests/FirstRobolectricTest.kt b/ui/kotlinx-coroutines-android/android-unit-tests/test/ordered/tests/FirstRobolectricTest.kt index eab6fc17fb..99744f897f 100644 --- a/ui/kotlinx-coroutines-android/android-unit-tests/test/ordered/tests/FirstRobolectricTest.kt +++ b/ui/kotlinx-coroutines-android/android-unit-tests/test/ordered/tests/FirstRobolectricTest.kt @@ -15,6 +15,7 @@ import kotlin.test.* @RunWith(RobolectricTestRunner::class) @Config(manifest = Config.NONE, sdk = [28]) +@LooperMode(LooperMode.Mode.LEGACY) open class FirstRobolectricTest { @Test fun testComponent() { diff --git a/ui/kotlinx-coroutines-android/build.gradle.kts b/ui/kotlinx-coroutines-android/build.gradle.kts index 145ad9f6b4..7618c529f7 100644 --- a/ui/kotlinx-coroutines-android/build.gradle.kts +++ b/ui/kotlinx-coroutines-android/build.gradle.kts @@ -10,16 +10,24 @@ configurations { repositories { mavenCentral() - jcenter() // https://youtrack.jetbrains.com/issue/IDEA-261387 } + +project.configureAar() + dependencies { + configureAarUnpacking() + compileOnly("com.google.android:android:${version("android")}") compileOnly("androidx.annotation:annotation:${version("androidx_annotation")}") testImplementation("com.google.android:android:${version("android")}") testImplementation("org.robolectric:robolectric:${version("robolectric")}") - testImplementation("org.smali:baksmali:${version("baksmali")}") + // Required by robolectric + testImplementation("androidx.test:core:1.2.0") + testImplementation("androidx.test:monitor:1.2.0") + + testImplementation("org.smali:baksmali:${version("baksmali")}") "r8"("com.android.tools.build:builder:7.1.0-alpha01") } diff --git a/ui/kotlinx-coroutines-android/test/AndroidExceptionPreHandlerTest.kt b/ui/kotlinx-coroutines-android/test/AndroidExceptionPreHandlerTest.kt index 4aa44eceaf..1220797009 100644 --- a/ui/kotlinx-coroutines-android/test/AndroidExceptionPreHandlerTest.kt +++ b/ui/kotlinx-coroutines-android/test/AndroidExceptionPreHandlerTest.kt @@ -13,6 +13,7 @@ import kotlin.test.* @RunWith(RobolectricTestRunner::class) @Config(manifest = Config.NONE, sdk = [27]) +@LooperMode(LooperMode.Mode.LEGACY) class AndroidExceptionPreHandlerTest : TestBase() { @Test fun testUnhandledException() = runTest { diff --git a/ui/kotlinx-coroutines-android/test/DisabledHandlerTest.kt b/ui/kotlinx-coroutines-android/test/DisabledHandlerTest.kt index a1f0a03d4a..a5b5ec95ee 100644 --- a/ui/kotlinx-coroutines-android/test/DisabledHandlerTest.kt +++ b/ui/kotlinx-coroutines-android/test/DisabledHandlerTest.kt @@ -13,6 +13,7 @@ import org.robolectric.annotation.* @RunWith(RobolectricTestRunner::class) @Config(manifest = Config.NONE, sdk = [28]) +@LooperMode(LooperMode.Mode.LEGACY) class DisabledHandlerTest : TestBase() { private var delegateToSuper = false diff --git a/ui/kotlinx-coroutines-android/test/HandlerDispatcherAsyncTest.kt b/ui/kotlinx-coroutines-android/test/HandlerDispatcherAsyncTest.kt index 7b03e771f9..c2091f339f 100644 --- a/ui/kotlinx-coroutines-android/test/HandlerDispatcherAsyncTest.kt +++ b/ui/kotlinx-coroutines-android/test/HandlerDispatcherAsyncTest.kt @@ -18,6 +18,7 @@ import kotlin.test.* @RunWith(RobolectricTestRunner::class) @Config(manifest = Config.NONE, sdk = [28]) +@LooperMode(LooperMode.Mode.LEGACY) class HandlerDispatcherAsyncTest : TestBase() { /** diff --git a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt index 24758444b0..fe97ae8d27 100644 --- a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt +++ b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt @@ -16,6 +16,7 @@ import kotlin.test.* @RunWith(RobolectricTestRunner::class) @Config(manifest = Config.NONE, sdk = [28]) +@LooperMode(LooperMode.Mode.LEGACY) class HandlerDispatcherTest : TestBase() { @Test fun testImmediateDispatcherYield() = runBlocking(Dispatchers.Main) { @@ -121,7 +122,6 @@ class HandlerDispatcherTest : TestBase() { } private fun CoroutineScope.doTestAwaitWithDetectedChoreographer() { - ShadowChoreographer.reset() ShadowChoreographer.setPostFrameCallbackDelay(100) val mainLooper = Shadows.shadowOf(Looper.getMainLooper()) launch(Dispatchers.Main, start = CoroutineStart.UNDISPATCHED) { From 1029ad1f1ad3e18c15fa7445757f10a59d949a9c Mon Sep 17 00:00:00 2001 From: Roman Elizarov Date: Wed, 16 Feb 2022 13:39:23 +0300 Subject: [PATCH 20/29] docs: Clarify MutableSharedFlow.emit/tryEmit on subscribers and overflow (#3185) The details on how buffering conceptually works and what happens without subscribers are buried deep inside the SharedFlow docs. This clarification puts the important information right into the doc of emit and tryEmit methods. Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com> --- .../common/src/flow/SharedFlow.kt | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/flow/SharedFlow.kt b/kotlinx-coroutines-core/common/src/flow/SharedFlow.kt index beb4255c09..0a291f258f 100644 --- a/kotlinx-coroutines-core/common/src/flow/SharedFlow.kt +++ b/kotlinx-coroutines-core/common/src/flow/SharedFlow.kt @@ -167,8 +167,15 @@ public interface SharedFlow : Flow { */ public interface MutableSharedFlow : SharedFlow, FlowCollector { /** - * Emits a [value] to this shared flow, suspending on buffer overflow if the shared flow was created - * with the default [BufferOverflow.SUSPEND] strategy. + * Emits a [value] to this shared flow, suspending on buffer overflow. + * + * This call can suspend only when the [BufferOverflow] strategy is + * [SUSPEND][BufferOverflow.SUSPEND] **and** there are subscribers collecting this shared flow. + * + * If there are no subscribers, the buffer is not used. + * Instead, the most recently emitted value is simply stored into + * the replay cache if one was configured, displacing the older elements there, + * or dropped if no replay cache was configured. * * See [tryEmit] for a non-suspending variant of this function. * @@ -179,12 +186,16 @@ public interface MutableSharedFlow : SharedFlow, FlowCollector { /** * Tries to emit a [value] to this shared flow without suspending. It returns `true` if the value was - * emitted successfully. When this function returns `false`, it means that the call to a plain [emit] - * function will suspend until there is a buffer space available. + * emitted successfully (see below). When this function returns `false`, it means that a call to a plain [emit] + * function would suspend until there is buffer space available. + * + * This call can return `false` only when the [BufferOverflow] strategy is + * [SUSPEND][BufferOverflow.SUSPEND] **and** there are subscribers collecting this shared flow. * - * A shared flow configured with a [BufferOverflow] strategy other than [SUSPEND][BufferOverflow.SUSPEND] - * (either [DROP_OLDEST][BufferOverflow.DROP_OLDEST] or [DROP_LATEST][BufferOverflow.DROP_LATEST]) never - * suspends on [emit], and thus `tryEmit` to such a shared flow always returns `true`. + * If there are no subscribers, the buffer is not used. + * Instead, the most recently emitted value is simply stored into + * the replay cache if one was configured, displacing the older elements there, + * or dropped if no replay cache was configured. In any case, `tryEmit` returns `true`. * * This method is **thread-safe** and can be safely invoked from concurrent coroutines without * external synchronization. From 70ae22b3f0cd718a9ef4f8b2d3147372c2324bd9 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Thu, 17 Feb 2022 02:27:01 -0800 Subject: [PATCH 21/29] Revert "Eagerly load CoroutineExceptionHandler and load corresponding service (#2957)" (#3190) This reverts commit 0b65246a32a81da724fc920d9f62f1670dff7d35. --- .../common/src/CoroutineExceptionHandler.kt | 8 +------- kotlinx-coroutines-core/common/src/Job.kt | 8 +------- .../js/src/CoroutineExceptionHandlerImpl.kt | 4 ---- .../jvm/src/CoroutineExceptionHandlerImpl.kt | 5 ----- .../native/src/CoroutineExceptionHandlerImpl.kt | 4 ---- 5 files changed, 2 insertions(+), 27 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt b/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt index 819f205b17..49923a92e7 100644 --- a/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt +++ b/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt @@ -1,19 +1,13 @@ /* * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ + package kotlinx.coroutines import kotlin.coroutines.* -import kotlin.jvm.* internal expect fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable) -/** - * JVM kludge: trigger loading of all the classes and service loading - * **before** any exception occur because it may be OOM, SOE or VerifyError - */ -internal expect fun initializeDefaultExceptionHandlers() - /** * Helper function for coroutine builder implementations to handle uncaught and unexpected exceptions in coroutines, * that could not be otherwise handled in a normal way through structured concurrency, saving them to a future, and diff --git a/kotlinx-coroutines-core/common/src/Job.kt b/kotlinx-coroutines-core/common/src/Job.kt index 085ef7e8af..31d90eeef0 100644 --- a/kotlinx-coroutines-core/common/src/Job.kt +++ b/kotlinx-coroutines-core/common/src/Job.kt @@ -113,13 +113,7 @@ public interface Job : CoroutineContext.Element { /** * Key for [Job] instance in the coroutine context. */ - public companion object Key : CoroutineContext.Key { - init { - // `Job` will necessarily be accessed early, so this is as good a place as any for the - // initialization logic that we want to happen as soon as possible - initializeDefaultExceptionHandlers() - } - } + public companion object Key : CoroutineContext.Key // ------------ state query ------------ diff --git a/kotlinx-coroutines-core/js/src/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/js/src/CoroutineExceptionHandlerImpl.kt index a4d671dc65..54a65e10a6 100644 --- a/kotlinx-coroutines-core/js/src/CoroutineExceptionHandlerImpl.kt +++ b/kotlinx-coroutines-core/js/src/CoroutineExceptionHandlerImpl.kt @@ -6,10 +6,6 @@ package kotlinx.coroutines import kotlin.coroutines.* -internal actual fun initializeDefaultExceptionHandlers() { - // Do nothing -} - internal actual fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable) { // log exception console.error(exception) diff --git a/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt index dd39210d25..4259092e78 100644 --- a/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt +++ b/kotlinx-coroutines-core/jvm/src/CoroutineExceptionHandlerImpl.kt @@ -22,11 +22,6 @@ private val handlers: List = ServiceLoader.load( CoroutineExceptionHandler::class.java.classLoader ).iterator().asSequence().toList() -internal actual fun initializeDefaultExceptionHandlers() { - // Load CEH and handlers - CoroutineExceptionHandler -} - /** * Private exception without stacktrace that is added to suppressed exceptions of the original exception * when it is reported to the last-ditch current thread 'uncaughtExceptionHandler'. diff --git a/kotlinx-coroutines-core/native/src/CoroutineExceptionHandlerImpl.kt b/kotlinx-coroutines-core/native/src/CoroutineExceptionHandlerImpl.kt index d97743b4bf..434813dc29 100644 --- a/kotlinx-coroutines-core/native/src/CoroutineExceptionHandlerImpl.kt +++ b/kotlinx-coroutines-core/native/src/CoroutineExceptionHandlerImpl.kt @@ -7,10 +7,6 @@ package kotlinx.coroutines import kotlin.coroutines.* import kotlin.native.* -internal actual fun initializeDefaultExceptionHandlers() { - // Do nothing -} - @OptIn(ExperimentalStdlibApi::class) internal actual fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable) { // log exception From d5f852c3345438d9b8196a59af3482afc50d857b Mon Sep 17 00:00:00 2001 From: rechee Date: Sun, 19 Apr 2020 00:58:52 -0700 Subject: [PATCH 22/29] Provide a way to use CoroutineDispatcher as an Rx's Scheduler. Fixes #968 Fixes #548 --- .../api/kotlinx-coroutines-rx2.api | 4 +- .../kotlinx-coroutines-rx2/src/RxScheduler.kt | 139 +++++- .../test/SchedulerStressTest.kt | 87 ++++ .../test/SchedulerTest.kt | 470 +++++++++++++++++- .../api/kotlinx-coroutines-rx3.api | 4 +- .../kotlinx-coroutines-rx3/src/RxScheduler.kt | 138 ++++- .../test/SchedulerStressTest.kt | 87 ++++ .../test/SchedulerTest.kt | 468 ++++++++++++++++- 8 files changed, 1379 insertions(+), 18 deletions(-) create mode 100644 reactive/kotlinx-coroutines-rx2/test/SchedulerStressTest.kt create mode 100644 reactive/kotlinx-coroutines-rx3/test/SchedulerStressTest.kt diff --git a/reactive/kotlinx-coroutines-rx2/api/kotlinx-coroutines-rx2.api b/reactive/kotlinx-coroutines-rx2/api/kotlinx-coroutines-rx2.api index 7cc594496e..c2d1c4bf1d 100644 --- a/reactive/kotlinx-coroutines-rx2/api/kotlinx-coroutines-rx2.api +++ b/reactive/kotlinx-coroutines-rx2/api/kotlinx-coroutines-rx2.api @@ -69,7 +69,9 @@ public final class kotlinx/coroutines/rx2/RxObservableKt { } public final class kotlinx/coroutines/rx2/RxSchedulerKt { - public static final fun asCoroutineDispatcher (Lio/reactivex/Scheduler;)Lkotlinx/coroutines/rx2/SchedulerCoroutineDispatcher; + public static final fun asCoroutineDispatcher (Lio/reactivex/Scheduler;)Lkotlinx/coroutines/CoroutineDispatcher; + public static final synthetic fun asCoroutineDispatcher (Lio/reactivex/Scheduler;)Lkotlinx/coroutines/rx2/SchedulerCoroutineDispatcher; + public static final fun asScheduler (Lkotlinx/coroutines/CoroutineDispatcher;)Lio/reactivex/Scheduler; } public final class kotlinx/coroutines/rx2/RxSingleKt { diff --git a/reactive/kotlinx-coroutines-rx2/src/RxScheduler.kt b/reactive/kotlinx-coroutines-rx2/src/RxScheduler.kt index 0262fc12f7..d7d5f6cfbf 100644 --- a/reactive/kotlinx-coroutines-rx2/src/RxScheduler.kt +++ b/reactive/kotlinx-coroutines-rx2/src/RxScheduler.kt @@ -4,16 +4,143 @@ package kotlinx.coroutines.rx2 -import io.reactivex.Scheduler +import io.reactivex.* +import io.reactivex.disposables.* +import io.reactivex.plugins.* +import kotlinx.atomicfu.* import kotlinx.coroutines.* -import java.util.concurrent.TimeUnit -import kotlin.coroutines.CoroutineContext +import kotlinx.coroutines.channels.* +import java.util.concurrent.* +import kotlin.coroutines.* /** * Converts an instance of [Scheduler] to an implementation of [CoroutineDispatcher] * and provides native support of [delay] and [withTimeout]. */ -public fun Scheduler.asCoroutineDispatcher(): SchedulerCoroutineDispatcher = SchedulerCoroutineDispatcher(this) +public fun Scheduler.asCoroutineDispatcher(): CoroutineDispatcher = + if (this is DispatcherScheduler) { + dispatcher + } else { + SchedulerCoroutineDispatcher(this) + } + +@Deprecated(level = DeprecationLevel.HIDDEN, message = "Since 1.4.2, binary compatibility with earlier versions") +@JvmName("asCoroutineDispatcher") +public fun Scheduler.asCoroutineDispatcher0(): SchedulerCoroutineDispatcher = + SchedulerCoroutineDispatcher(this) + +/** + * Converts an instance of [CoroutineDispatcher] to an implementation of [Scheduler]. + */ +public fun CoroutineDispatcher.asScheduler(): Scheduler = + if (this is SchedulerCoroutineDispatcher) { + scheduler + } else { + DispatcherScheduler(this) + } + +private class DispatcherScheduler(@JvmField val dispatcher: CoroutineDispatcher) : Scheduler() { + + private val schedulerJob = SupervisorJob() + + /** + * The scope for everything happening in this [DispatcherScheduler]. + * + * Running tasks, too, get launched under this scope, because [shutdown] should cancel the running tasks as well. + */ + private val scope = CoroutineScope(schedulerJob + dispatcher) + + /** + * The counter of created workers, for their pretty-printing. + */ + private val workerCounter = atomic(1L) + + override fun scheduleDirect(block: Runnable, delay: Long, unit: TimeUnit): Disposable = + scope.scheduleTask(block, unit.toMillis(delay)) { task -> + Runnable { scope.launch { task() } } + } + + override fun createWorker(): Worker = DispatcherWorker(workerCounter.getAndIncrement(), dispatcher, schedulerJob) + + override fun shutdown() { + schedulerJob.cancel() + } + + private class DispatcherWorker( + private val counter: Long, + private val dispatcher: CoroutineDispatcher, + parentJob: Job + ) : Worker() { + + private val workerJob = SupervisorJob(parentJob) + private val workerScope = CoroutineScope(workerJob + dispatcher) + private val blockChannel = Channel Unit>(Channel.UNLIMITED) + + init { + workerScope.launch { + blockChannel.consumeEach { + it() + } + } + } + + override fun schedule(block: Runnable, delay: Long, unit: TimeUnit): Disposable = + workerScope.scheduleTask(block, unit.toMillis(delay)) { task -> + Runnable { blockChannel.trySend(task) } + } + + override fun isDisposed(): Boolean = !workerScope.isActive + + override fun dispose() { + blockChannel.close() + workerJob.cancel() + } + + override fun toString(): String = "$dispatcher (worker $counter, ${if (isDisposed) "disposed" else "active"})" + } + + override fun toString(): String = dispatcher.toString() +} + +private typealias Task = suspend () -> Unit + +/** + * Schedule [block] so that an adapted version of it, wrapped in [adaptForScheduling], executes after [delayMillis] + * milliseconds. + */ +private fun CoroutineScope.scheduleTask( + block: Runnable, + delayMillis: Long, + adaptForScheduling: (Task) -> Runnable +): Disposable { + val ctx = coroutineContext + var handle: DisposableHandle? = null + val disposable = Disposables.fromRunnable { + // null if delay <= 0 + handle?.dispose() + } + val decoratedBlock = RxJavaPlugins.onSchedule(block) + suspend fun task() { + if (disposable.isDisposed) return + try { + runInterruptible { + decoratedBlock.run() + } + } catch (e: Throwable) { + handleUndeliverableException(e, ctx) + } + } + + val toSchedule = adaptForScheduling(::task) + if (!isActive) return Disposables.disposed() + if (delayMillis <= 0) { + toSchedule.run() + } else { + @Suppress("INVISIBLE_MEMBER") + ctx.delay.invokeOnTimeout(delayMillis, toSchedule, ctx).let { handle = it } + } + return disposable +} /** * Implements [CoroutineDispatcher] on top of an arbitrary [Scheduler]. @@ -45,8 +172,10 @@ public class SchedulerCoroutineDispatcher( /** @suppress */ override fun toString(): String = scheduler.toString() + /** @suppress */ override fun equals(other: Any?): Boolean = other is SchedulerCoroutineDispatcher && other.scheduler === scheduler + /** @suppress */ override fun hashCode(): Int = System.identityHashCode(scheduler) -} +} \ No newline at end of file diff --git a/reactive/kotlinx-coroutines-rx2/test/SchedulerStressTest.kt b/reactive/kotlinx-coroutines-rx2/test/SchedulerStressTest.kt new file mode 100644 index 0000000000..ea33a9fa58 --- /dev/null +++ b/reactive/kotlinx-coroutines-rx2/test/SchedulerStressTest.kt @@ -0,0 +1,87 @@ +/* + * Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.rx2 + +import kotlinx.coroutines.* +import org.junit.* +import java.util.concurrent.* + +class SchedulerStressTest : TestBase() { + @Before + fun setup() { + ignoreLostThreads("RxCachedThreadScheduler-", "RxCachedWorkerPoolEvictor-", "RxSchedulerPurge-") + } + + /** + * Test that we don't get an OOM if we schedule many jobs at once. + * It's expected that if you don't dispose you'd see an OOM error. + */ + @Test + fun testSchedulerDisposed(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableDisposed(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerDisposed(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + val worker = scheduler.createWorker() + testRunnableDisposed(worker::schedule) + } + + private suspend fun testRunnableDisposed(block: RxSchedulerBlockNoDelay) { + val n = 2000 * stressTestMultiplier + repeat(n) { + val a = ByteArray(1000000) //1MB + val disposable = block(Runnable { + keepMe(a) + expectUnreached() + }) + disposable.dispose() + yield() // allow the scheduled task to observe that it was disposed + } + } + + /** + * Test function that holds a reference. Used for testing OOM situations + */ + private fun keepMe(a: ByteArray) { + Thread.sleep(a.size / (a.size + 1) + 10L) + } + + /** + * Test that we don't get an OOM if we schedule many delayed jobs at once. It's expected that if you don't dispose that you'd + * see a OOM error. + */ + @Test + fun testSchedulerDisposedDuringDelay(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableDisposedDuringDelay(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerDisposedDuringDelay(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + val worker = scheduler.createWorker() + testRunnableDisposedDuringDelay(worker::schedule) + } + + private fun testRunnableDisposedDuringDelay(block: RxSchedulerBlockWithDelay) { + val n = 2000 * stressTestMultiplier + repeat(n) { + val a = ByteArray(1000000) //1MB + val delayMillis: Long = 10 + val disposable = block(Runnable { + keepMe(a) + expectUnreached() + }, delayMillis, TimeUnit.MILLISECONDS) + disposable.dispose() + } + } +} \ No newline at end of file diff --git a/reactive/kotlinx-coroutines-rx2/test/SchedulerTest.kt b/reactive/kotlinx-coroutines-rx2/test/SchedulerTest.kt index 26dbe8f4cf..194186718d 100644 --- a/reactive/kotlinx-coroutines-rx2/test/SchedulerTest.kt +++ b/reactive/kotlinx-coroutines-rx2/test/SchedulerTest.kt @@ -4,10 +4,18 @@ package kotlinx.coroutines.rx2 -import io.reactivex.schedulers.Schedulers +import io.reactivex.* +import io.reactivex.disposables.* +import io.reactivex.plugins.* +import io.reactivex.schedulers.* import kotlinx.coroutines.* -import org.junit.Before +import kotlinx.coroutines.sync.* +import org.junit.* import org.junit.Test +import java.lang.Runnable +import java.util.concurrent.* +import java.util.concurrent.atomic.AtomicReference +import kotlin.coroutines.* import kotlin.test.* class SchedulerTest : TestBase() { @@ -17,7 +25,7 @@ class SchedulerTest : TestBase() { } @Test - fun testIoScheduler(): Unit = runBlocking { + fun testIoScheduler(): Unit = runTest { expect(1) val mainThread = Thread.currentThread() withContext(Schedulers.io().asCoroutineDispatcher()) { @@ -31,4 +39,458 @@ class SchedulerTest : TestBase() { } finish(4) } -} \ No newline at end of file + + /** Tests [toString] implementations of [CoroutineDispatcher.asScheduler] and its [Scheduler.Worker]. */ + @Test + fun testSchedulerToString() { + val name = "Dispatchers.Default" + val scheduler = Dispatchers.Default.asScheduler() + assertContains(scheduler.toString(), name) + val worker = scheduler.createWorker() + val activeWorkerName = worker.toString() + assertContains(worker.toString(), name) + worker.dispose() + val disposedWorkerName = worker.toString() + assertNotEquals(activeWorkerName, disposedWorkerName) + } + + private fun runSchedulerTest(nThreads: Int = 1, action: (Scheduler) -> Unit) { + val future = CompletableFuture() + try { + newFixedThreadPoolContext(nThreads, "test").use { dispatcher -> + RxJavaPlugins.setErrorHandler { + if (!future.completeExceptionally(it)) { + handleUndeliverableException(it, dispatcher) + } + } + action(dispatcher.asScheduler()) + } + } finally { + RxJavaPlugins.setErrorHandler(null) + } + future.complete(Unit) + future.getNow(Unit) // rethrow any encountered errors + } + + private fun ensureSeparateThread(schedule: (Runnable, Long, TimeUnit) -> Unit, scheduleNoDelay: (Runnable) -> Unit) { + val mainThread = Thread.currentThread() + val cdl1 = CountDownLatch(1) + val cdl2 = CountDownLatch(1) + expect(1) + val thread = AtomicReference(null) + fun checkThread() { + val current = Thread.currentThread() + thread.getAndSet(current)?.let { assertEquals(it, current) } + } + schedule({ + assertNotSame(mainThread, Thread.currentThread()) + checkThread() + cdl2.countDown() + }, 300, TimeUnit.MILLISECONDS) + scheduleNoDelay { + expect(2) + checkThread() + assertNotSame(mainThread, Thread.currentThread()) + cdl1.countDown() + } + cdl1.await() + cdl2.await() + finish(3) + } + + /** + * Tests [Scheduler.scheduleDirect] for [CoroutineDispatcher.asScheduler] on a single-threaded dispatcher. + */ + @Test + fun testSingleThreadedDispatcherDirect(): Unit = runSchedulerTest(1) { + ensureSeparateThread(it::scheduleDirect, it::scheduleDirect) + } + + /** + * Tests [Scheduler.Worker.schedule] for [CoroutineDispatcher.asScheduler] running its tasks on the correct thread. + */ + @Test + fun testSingleThreadedWorker(): Unit = runSchedulerTest(1) { + val worker = it.createWorker() + ensureSeparateThread(worker::schedule, worker::schedule) + } + + private fun checkCancelling(schedule: (Runnable, Long, TimeUnit) -> Disposable) { + // cancel the task before it has a chance to run. + val handle1 = schedule({ + throw IllegalStateException("should have been successfully cancelled") + }, 10_000, TimeUnit.MILLISECONDS) + handle1.dispose() + // cancel the task after it started running. + val cdl1 = CountDownLatch(1) + val cdl2 = CountDownLatch(1) + val handle2 = schedule({ + cdl1.countDown() + cdl2.await() + if (Thread.interrupted()) + throw IllegalStateException("cancelling the task should not interrupt the thread") + }, 100, TimeUnit.MILLISECONDS) + cdl1.await() + handle2.dispose() + cdl2.countDown() + } + + /** + * Test cancelling [Scheduler.scheduleDirect] for [CoroutineDispatcher.asScheduler]. + */ + @Test + fun testCancellingDirect(): Unit = runSchedulerTest { + checkCancelling(it::scheduleDirect) + } + + /** + * Test cancelling [Scheduler.Worker.schedule] for [CoroutineDispatcher.asScheduler]. + */ + @Test + fun testCancellingWorker(): Unit = runSchedulerTest { + val worker = it.createWorker() + checkCancelling(worker::schedule) + } + + /** + * Test shutting down [CoroutineDispatcher.asScheduler]. + */ + @Test + fun testShuttingDown() { + val n = 5 + runSchedulerTest(nThreads = n) { scheduler -> + val cdl1 = CountDownLatch(n) + val cdl2 = CountDownLatch(1) + val cdl3 = CountDownLatch(n) + repeat(n) { + scheduler.scheduleDirect { + cdl1.countDown() + try { + cdl2.await() + } catch (e: InterruptedException) { + // this is the expected outcome + cdl3.countDown() + } + } + } + cdl1.await() + scheduler.shutdown() + if (!cdl3.await(1, TimeUnit.SECONDS)) { + cdl2.countDown() + error("the tasks were not cancelled when the scheduler was shut down") + } + } + } + + /** Tests that there are no uncaught exceptions if [Disposable.dispose] on a worker happens when tasks are present. */ + @Test + fun testDisposingWorker() = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + val worker = scheduler.createWorker() + yield() // so that the worker starts waiting on the channel + assertFalse(worker.isDisposed) + worker.dispose() + assertTrue(worker.isDisposed) + } + + /** Tests trying to use a [Scheduler.Worker]/[Scheduler] after [Scheduler.Worker.dispose]/[Scheduler.shutdown]. */ + @Test + fun testSchedulingAfterDisposing() = runSchedulerTest { + expect(1) + val worker = it.createWorker() + // use CDL to ensure that the worker has properly initialized + val cdl1 = CountDownLatch(1) + setScheduler(2, 3) + val disposable1 = worker.schedule { + cdl1.countDown() + } + cdl1.await() + expect(4) + assertFalse(disposable1.isDisposed) + setScheduler(6, -1) + // check that the worker automatically disposes of the tasks after being disposed + assertFalse(worker.isDisposed) + worker.dispose() + assertTrue(worker.isDisposed) + expect(5) + val disposable2 = worker.schedule { + expectUnreached() + } + assertTrue(disposable2.isDisposed) + setScheduler(7, 8) + // ensure that the scheduler still works + val cdl2 = CountDownLatch(1) + val disposable3 = it.scheduleDirect { + cdl2.countDown() + } + cdl2.await() + expect(9) + assertFalse(disposable3.isDisposed) + // check that the scheduler automatically disposes of the tasks after being shut down + it.shutdown() + setScheduler(10, -1) + val disposable4 = it.scheduleDirect { + expectUnreached() + } + assertTrue(disposable4.isDisposed) + RxJavaPlugins.setScheduleHandler(null) + finish(11) + } + + @Test + fun testSchedulerWithNoDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithNoDelay(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerWithNoDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithNoDelay(scheduler.createWorker()::schedule) + } + + private suspend fun testRunnableWithNoDelay(block: RxSchedulerBlockNoDelay) { + expect(1) + suspendCancellableCoroutine { + block(Runnable { + expect(2) + it.resume(Unit) + }) + } + yield() + finish(3) + } + + @Test + fun testSchedulerWithDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler::scheduleDirect, 300) + } + + @Test + fun testSchedulerWorkerWithDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler.createWorker()::schedule, 300) + } + + @Test + fun testSchedulerWithZeroDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerWithZeroDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler.createWorker()::schedule) + } + + private suspend fun testRunnableWithDelay(block: RxSchedulerBlockWithDelay, delayMillis: Long = 0) { + expect(1) + suspendCancellableCoroutine { + block({ + expect(2) + it.resume(Unit) + }, delayMillis, TimeUnit.MILLISECONDS) + } + finish(3) + } + + @Test + fun testAsSchedulerWithNegativeDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler::scheduleDirect, -1) + } + + @Test + fun testAsSchedulerWorkerWithNegativeDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler.createWorker()::schedule, -1) + } + + @Test + fun testSchedulerImmediateDispose(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableImmediateDispose(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerImmediateDispose(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableImmediateDispose(scheduler.createWorker()::schedule) + } + + private fun testRunnableImmediateDispose(block: RxSchedulerBlockNoDelay) { + val disposable = block { + expectUnreached() + } + disposable.dispose() + } + + @Test + fun testConvertDispatcherToOriginalScheduler(): Unit = runTest { + val originalScheduler = Schedulers.io() + val dispatcher = originalScheduler.asCoroutineDispatcher() + val scheduler = dispatcher.asScheduler() + assertSame(originalScheduler, scheduler) + } + + @Test + fun testConvertSchedulerToOriginalDispatcher(): Unit = runTest { + val originalDispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = originalDispatcher.asScheduler() + val dispatcher = scheduler.asCoroutineDispatcher() + assertSame(originalDispatcher, dispatcher) + } + + @Test + fun testSchedulerExpectRxPluginsCall(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableExpectRxPluginsCall(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerExpectRxPluginsCall(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableExpectRxPluginsCall(scheduler.createWorker()::schedule) + } + + private suspend fun testRunnableExpectRxPluginsCall(block: RxSchedulerBlockNoDelay) { + expect(1) + setScheduler(2, 4) + suspendCancellableCoroutine { + block(Runnable { + expect(5) + it.resume(Unit) + }) + expect(3) + } + RxJavaPlugins.setScheduleHandler(null) + finish(6) + } + + @Test + fun testSchedulerExpectRxPluginsCallWithDelay(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableExpectRxPluginsCallDelay(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerExpectRxPluginsCallWithDelay(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + val worker = scheduler.createWorker() + testRunnableExpectRxPluginsCallDelay(worker::schedule) + } + + private suspend fun testRunnableExpectRxPluginsCallDelay(block: RxSchedulerBlockWithDelay) { + expect(1) + setScheduler(2, 4) + suspendCancellableCoroutine { + block({ + expect(5) + it.resume(Unit) + }, 10, TimeUnit.MILLISECONDS) + expect(3) + } + RxJavaPlugins.setScheduleHandler(null) + finish(6) + } + + private fun setScheduler(expectedCountOnSchedule: Int, expectCountOnRun: Int) { + RxJavaPlugins.setScheduleHandler { + expect(expectedCountOnSchedule) + Runnable { + expect(expectCountOnRun) + it.run() + } + } + } + + /** + * Tests that [Scheduler.Worker] runs all work sequentially. + */ + @Test + fun testWorkerSequentialOrdering() = runTest { + expect(1) + val scheduler = Dispatchers.Default.asScheduler() + val worker = scheduler.createWorker() + val iterations = 100 + for (i in 0..iterations) { + worker.schedule { + expect(2 + i) + } + } + suspendCoroutine { + worker.schedule { + it.resume(Unit) + } + } + finish((iterations + 2) + 1) + } + + /** + * Test that ensures that delays are actually respected (tasks scheduled sooner in the future run before tasks scheduled later, + * even when the later task is submitted before the earlier one) + */ + @Test + fun testSchedulerRespectsDelays(): Unit = runTest { + val scheduler = Dispatchers.Default.asScheduler() + testRunnableRespectsDelays(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerRespectsDelays(): Unit = runTest { + val scheduler = Dispatchers.Default.asScheduler() + testRunnableRespectsDelays(scheduler.createWorker()::schedule) + } + + private suspend fun testRunnableRespectsDelays(block: RxSchedulerBlockWithDelay) { + expect(1) + val semaphore = Semaphore(2, 2) + block({ + expect(3) + semaphore.release() + }, 100, TimeUnit.MILLISECONDS) + block({ + expect(2) + semaphore.release() + }, 1, TimeUnit.MILLISECONDS) + semaphore.acquire() + semaphore.acquire() + finish(4) + } + + /** + * Tests that cancelling a runnable in one worker doesn't affect work in another scheduler. + * + * This is part of expected behavior documented. + */ + @Test + fun testMultipleWorkerCancellation(): Unit = runTest { + expect(1) + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + suspendCancellableCoroutine { + val workerOne = scheduler.createWorker() + workerOne.schedule({ + expect(3) + it.resume(Unit) + }, 50, TimeUnit.MILLISECONDS) + val workerTwo = scheduler.createWorker() + workerTwo.schedule({ + expectUnreached() + }, 1000, TimeUnit.MILLISECONDS) + workerTwo.dispose() + expect(2) + } + finish(4) + } +} + +typealias RxSchedulerBlockNoDelay = (Runnable) -> Disposable +typealias RxSchedulerBlockWithDelay = (Runnable, Long, TimeUnit) -> Disposable \ No newline at end of file diff --git a/reactive/kotlinx-coroutines-rx3/api/kotlinx-coroutines-rx3.api b/reactive/kotlinx-coroutines-rx3/api/kotlinx-coroutines-rx3.api index f6f3f1d06d..5776214b0a 100644 --- a/reactive/kotlinx-coroutines-rx3/api/kotlinx-coroutines-rx3.api +++ b/reactive/kotlinx-coroutines-rx3/api/kotlinx-coroutines-rx3.api @@ -58,7 +58,9 @@ public final class kotlinx/coroutines/rx3/RxObservableKt { } public final class kotlinx/coroutines/rx3/RxSchedulerKt { - public static final fun asCoroutineDispatcher (Lio/reactivex/rxjava3/core/Scheduler;)Lkotlinx/coroutines/rx3/SchedulerCoroutineDispatcher; + public static final fun asCoroutineDispatcher (Lio/reactivex/rxjava3/core/Scheduler;)Lkotlinx/coroutines/CoroutineDispatcher; + public static final synthetic fun asCoroutineDispatcher (Lio/reactivex/rxjava3/core/Scheduler;)Lkotlinx/coroutines/rx3/SchedulerCoroutineDispatcher; + public static final fun asScheduler (Lkotlinx/coroutines/CoroutineDispatcher;)Lio/reactivex/rxjava3/core/Scheduler; } public final class kotlinx/coroutines/rx3/RxSingleKt { diff --git a/reactive/kotlinx-coroutines-rx3/src/RxScheduler.kt b/reactive/kotlinx-coroutines-rx3/src/RxScheduler.kt index 24c3f11834..abaf02450a 100644 --- a/reactive/kotlinx-coroutines-rx3/src/RxScheduler.kt +++ b/reactive/kotlinx-coroutines-rx3/src/RxScheduler.kt @@ -4,16 +4,144 @@ package kotlinx.coroutines.rx3 -import io.reactivex.rxjava3.core.Scheduler +import io.reactivex.rxjava3.core.* +import io.reactivex.rxjava3.disposables.* +import io.reactivex.rxjava3.plugins.* +import kotlinx.atomicfu.* import kotlinx.coroutines.* -import java.util.concurrent.TimeUnit -import kotlin.coroutines.CoroutineContext +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.channels.* +import java.util.concurrent.* +import kotlin.coroutines.* /** * Converts an instance of [Scheduler] to an implementation of [CoroutineDispatcher] * and provides native support of [delay] and [withTimeout]. */ -public fun Scheduler.asCoroutineDispatcher(): SchedulerCoroutineDispatcher = SchedulerCoroutineDispatcher(this) +public fun Scheduler.asCoroutineDispatcher(): CoroutineDispatcher = + if (this is DispatcherScheduler) { + dispatcher + } else { + SchedulerCoroutineDispatcher(this) + } + +@Deprecated(level = DeprecationLevel.HIDDEN, message = "Since 1.4.2, binary compatibility with earlier versions") +@JvmName("asCoroutineDispatcher") +public fun Scheduler.asCoroutineDispatcher0(): SchedulerCoroutineDispatcher = + SchedulerCoroutineDispatcher(this) + +/** + * Converts an instance of [CoroutineDispatcher] to an implementation of [Scheduler]. + */ +public fun CoroutineDispatcher.asScheduler(): Scheduler = + if (this is SchedulerCoroutineDispatcher) { + scheduler + } else { + DispatcherScheduler(this) + } + +private class DispatcherScheduler(@JvmField val dispatcher: CoroutineDispatcher) : Scheduler() { + + private val schedulerJob = SupervisorJob() + + /** + * The scope for everything happening in this [DispatcherScheduler]. + * + * Running tasks, too, get launched under this scope, because [shutdown] should cancel the running tasks as well. + */ + private val scope = CoroutineScope(schedulerJob + dispatcher) + + /** + * The counter of created workers, for their pretty-printing. + */ + private val workerCounter = atomic(1L) + + override fun scheduleDirect(block: Runnable, delay: Long, unit: TimeUnit): Disposable = + scope.scheduleTask(block, unit.toMillis(delay)) { task -> + Runnable { scope.launch { task() } } + } + + override fun createWorker(): Worker = DispatcherWorker(workerCounter.getAndIncrement(), dispatcher, schedulerJob) + + override fun shutdown() { + schedulerJob.cancel() + } + + private class DispatcherWorker( + private val counter: Long, + private val dispatcher: CoroutineDispatcher, + parentJob: Job + ) : Worker() { + + private val workerJob = SupervisorJob(parentJob) + private val workerScope = CoroutineScope(workerJob + dispatcher) + private val blockChannel = Channel Unit>(Channel.UNLIMITED) + + init { + workerScope.launch { + blockChannel.consumeEach { + it() + } + } + } + + override fun schedule(block: Runnable, delay: Long, unit: TimeUnit): Disposable = + workerScope.scheduleTask(block, unit.toMillis(delay)) { task -> + Runnable { blockChannel.trySend(task) } + } + + override fun isDisposed(): Boolean = !workerScope.isActive + + override fun dispose() { + blockChannel.close() + workerJob.cancel() + } + + override fun toString(): String = "$dispatcher (worker $counter, ${if (isDisposed) "disposed" else "active"})" + } + + override fun toString(): String = dispatcher.toString() +} + +private typealias Task = suspend () -> Unit + +/** + * Schedule [block] so that an adapted version of it, wrapped in [adaptForScheduling], executes after [delayMillis] + * milliseconds. + */ +private fun CoroutineScope.scheduleTask( + block: Runnable, + delayMillis: Long, + adaptForScheduling: (Task) -> Runnable +): Disposable { + val ctx = coroutineContext + var handle: DisposableHandle? = null + val disposable = Disposable.fromRunnable { + // null if delay <= 0 + handle?.dispose() + } + val decoratedBlock = RxJavaPlugins.onSchedule(block) + suspend fun task() { + if (disposable.isDisposed) return + try { + runInterruptible { + decoratedBlock.run() + } + } catch (e: Throwable) { + handleUndeliverableException(e, ctx) + } + } + + val toSchedule = adaptForScheduling(::task) + if (!isActive) return Disposable.disposed() + if (delayMillis <= 0) { + toSchedule.run() + } else { + @Suppress("INVISIBLE_MEMBER") + ctx.delay.invokeOnTimeout(delayMillis, toSchedule, ctx).let { handle = it } + } + return disposable +} /** * Implements [CoroutineDispatcher] on top of an arbitrary [Scheduler]. @@ -45,8 +173,10 @@ public class SchedulerCoroutineDispatcher( /** @suppress */ override fun toString(): String = scheduler.toString() + /** @suppress */ override fun equals(other: Any?): Boolean = other is SchedulerCoroutineDispatcher && other.scheduler === scheduler + /** @suppress */ override fun hashCode(): Int = System.identityHashCode(scheduler) } diff --git a/reactive/kotlinx-coroutines-rx3/test/SchedulerStressTest.kt b/reactive/kotlinx-coroutines-rx3/test/SchedulerStressTest.kt new file mode 100644 index 0000000000..5abb511d72 --- /dev/null +++ b/reactive/kotlinx-coroutines-rx3/test/SchedulerStressTest.kt @@ -0,0 +1,87 @@ +/* + * Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.rx3 + +import kotlinx.coroutines.* +import org.junit.* +import java.util.concurrent.* + +class SchedulerStressTest : TestBase() { + @Before + fun setup() { + ignoreLostThreads("RxCachedThreadScheduler-", "RxCachedWorkerPoolEvictor-", "RxSchedulerPurge-") + } + + /** + * Test that we don't get an OOM if we schedule many jobs at once. + * It's expected that if you don't dispose you'd see an OOM error. + */ + @Test + fun testSchedulerDisposed(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableDisposed(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerDisposed(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + val worker = scheduler.createWorker() + testRunnableDisposed(worker::schedule) + } + + private suspend fun testRunnableDisposed(block: RxSchedulerBlockNoDelay) { + val n = 2000 * stressTestMultiplier + repeat(n) { + val a = ByteArray(1000000) //1MB + val disposable = block(Runnable { + keepMe(a) + expectUnreached() + }) + disposable.dispose() + yield() // allow the scheduled task to observe that it was disposed + } + } + + /** + * Test function that holds a reference. Used for testing OOM situations + */ + private fun keepMe(a: ByteArray) { + Thread.sleep(a.size / (a.size + 1) + 10L) + } + + /** + * Test that we don't get an OOM if we schedule many delayed jobs at once. It's expected that if you don't dispose that you'd + * see a OOM error. + */ + @Test + fun testSchedulerDisposedDuringDelay(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableDisposedDuringDelay(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerDisposedDuringDelay(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + val worker = scheduler.createWorker() + testRunnableDisposedDuringDelay(worker::schedule) + } + + private fun testRunnableDisposedDuringDelay(block: RxSchedulerBlockWithDelay) { + val n = 2000 * stressTestMultiplier + repeat(n) { + val a = ByteArray(1000000) //1MB + val delayMillis: Long = 10 + val disposable = block(Runnable { + keepMe(a) + expectUnreached() + }, delayMillis, TimeUnit.MILLISECONDS) + disposable.dispose() + } + } +} diff --git a/reactive/kotlinx-coroutines-rx3/test/SchedulerTest.kt b/reactive/kotlinx-coroutines-rx3/test/SchedulerTest.kt index 9e95c213d0..c966cdfd08 100644 --- a/reactive/kotlinx-coroutines-rx3/test/SchedulerTest.kt +++ b/reactive/kotlinx-coroutines-rx3/test/SchedulerTest.kt @@ -4,10 +4,18 @@ package kotlinx.coroutines.rx3 -import io.reactivex.rxjava3.schedulers.Schedulers +import io.reactivex.rxjava3.core.* +import io.reactivex.rxjava3.disposables.* +import io.reactivex.rxjava3.plugins.* +import io.reactivex.rxjava3.schedulers.* import kotlinx.coroutines.* -import org.junit.Before +import kotlinx.coroutines.sync.* +import org.junit.* import org.junit.Test +import java.lang.Runnable +import java.util.concurrent.* +import java.util.concurrent.atomic.AtomicReference +import kotlin.coroutines.* import kotlin.test.* class SchedulerTest : TestBase() { @@ -17,7 +25,7 @@ class SchedulerTest : TestBase() { } @Test - fun testIoScheduler(): Unit = runBlocking { + fun testIoScheduler(): Unit = runTest { expect(1) val mainThread = Thread.currentThread() withContext(Schedulers.io().asCoroutineDispatcher()) { @@ -31,4 +39,458 @@ class SchedulerTest : TestBase() { } finish(4) } + + /** Tests [toString] implementations of [CoroutineDispatcher.asScheduler] and its [Scheduler.Worker]. */ + @Test + fun testSchedulerToString() { + val name = "Dispatchers.Default" + val scheduler = Dispatchers.Default.asScheduler() + assertContains(scheduler.toString(), name) + val worker = scheduler.createWorker() + val activeWorkerName = worker.toString() + assertContains(worker.toString(), name) + worker.dispose() + val disposedWorkerName = worker.toString() + assertNotEquals(activeWorkerName, disposedWorkerName) + } + + private fun runSchedulerTest(nThreads: Int = 1, action: (Scheduler) -> Unit) { + val future = CompletableFuture() + try { + newFixedThreadPoolContext(nThreads, "test").use { dispatcher -> + RxJavaPlugins.setErrorHandler { + if (!future.completeExceptionally(it)) { + handleUndeliverableException(it, dispatcher) + } + } + action(dispatcher.asScheduler()) + } + } finally { + RxJavaPlugins.setErrorHandler(null) + } + future.complete(Unit) + future.getNow(Unit) // rethrow any encountered errors + } + + private fun ensureSeparateThread(schedule: (Runnable, Long, TimeUnit) -> Unit, scheduleNoDelay: (Runnable) -> Unit) { + val mainThread = Thread.currentThread() + val cdl1 = CountDownLatch(1) + val cdl2 = CountDownLatch(1) + expect(1) + val thread = AtomicReference(null) + fun checkThread() { + val current = Thread.currentThread() + thread.getAndSet(current)?.let { assertEquals(it, current) } + } + schedule({ + assertNotSame(mainThread, Thread.currentThread()) + checkThread() + cdl2.countDown() + }, 300, TimeUnit.MILLISECONDS) + scheduleNoDelay { + expect(2) + checkThread() + assertNotSame(mainThread, Thread.currentThread()) + cdl1.countDown() + } + cdl1.await() + cdl2.await() + finish(3) + } + + /** + * Tests [Scheduler.scheduleDirect] for [CoroutineDispatcher.asScheduler] on a single-threaded dispatcher. + */ + @Test + fun testSingleThreadedDispatcherDirect(): Unit = runSchedulerTest(1) { + ensureSeparateThread(it::scheduleDirect, it::scheduleDirect) + } + + /** + * Tests [Scheduler.Worker.schedule] for [CoroutineDispatcher.asScheduler] running its tasks on the correct thread. + */ + @Test + fun testSingleThreadedWorker(): Unit = runSchedulerTest(1) { + val worker = it.createWorker() + ensureSeparateThread(worker::schedule, worker::schedule) + } + + private fun checkCancelling(schedule: (Runnable, Long, TimeUnit) -> Disposable) { + // cancel the task before it has a chance to run. + val handle1 = schedule({ + throw IllegalStateException("should have been successfully cancelled") + }, 10_000, TimeUnit.MILLISECONDS) + handle1.dispose() + // cancel the task after it started running. + val cdl1 = CountDownLatch(1) + val cdl2 = CountDownLatch(1) + val handle2 = schedule({ + cdl1.countDown() + cdl2.await() + if (Thread.interrupted()) + throw IllegalStateException("cancelling the task should not interrupt the thread") + }, 100, TimeUnit.MILLISECONDS) + cdl1.await() + handle2.dispose() + cdl2.countDown() + } + + /** + * Test cancelling [Scheduler.scheduleDirect] for [CoroutineDispatcher.asScheduler]. + */ + @Test + fun testCancellingDirect(): Unit = runSchedulerTest { + checkCancelling(it::scheduleDirect) + } + + /** + * Test cancelling [Scheduler.Worker.schedule] for [CoroutineDispatcher.asScheduler]. + */ + @Test + fun testCancellingWorker(): Unit = runSchedulerTest { + val worker = it.createWorker() + checkCancelling(worker::schedule) + } + + /** + * Test shutting down [CoroutineDispatcher.asScheduler]. + */ + @Test + fun testShuttingDown() { + val n = 5 + runSchedulerTest(nThreads = n) { scheduler -> + val cdl1 = CountDownLatch(n) + val cdl2 = CountDownLatch(1) + val cdl3 = CountDownLatch(n) + repeat(n) { + scheduler.scheduleDirect { + cdl1.countDown() + try { + cdl2.await() + } catch (e: InterruptedException) { + // this is the expected outcome + cdl3.countDown() + } + } + } + cdl1.await() + scheduler.shutdown() + if (!cdl3.await(1, TimeUnit.SECONDS)) { + cdl2.countDown() + error("the tasks were not cancelled when the scheduler was shut down") + } + } + } + + /** Tests that there are no uncaught exceptions if [Disposable.dispose] on a worker happens when tasks are present. */ + @Test + fun testDisposingWorker() = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + val worker = scheduler.createWorker() + yield() // so that the worker starts waiting on the channel + assertFalse(worker.isDisposed) + worker.dispose() + assertTrue(worker.isDisposed) + } + + /** Tests trying to use a [Scheduler.Worker]/[Scheduler] after [Scheduler.Worker.dispose]/[Scheduler.shutdown]. */ + @Test + fun testSchedulingAfterDisposing() = runSchedulerTest { + expect(1) + val worker = it.createWorker() + // use CDL to ensure that the worker has properly initialized + val cdl1 = CountDownLatch(1) + setScheduler(2, 3) + val disposable1 = worker.schedule { + cdl1.countDown() + } + cdl1.await() + expect(4) + assertFalse(disposable1.isDisposed) + setScheduler(6, -1) + // check that the worker automatically disposes of the tasks after being disposed + assertFalse(worker.isDisposed) + worker.dispose() + assertTrue(worker.isDisposed) + expect(5) + val disposable2 = worker.schedule { + expectUnreached() + } + assertTrue(disposable2.isDisposed) + setScheduler(7, 8) + // ensure that the scheduler still works + val cdl2 = CountDownLatch(1) + val disposable3 = it.scheduleDirect { + cdl2.countDown() + } + cdl2.await() + expect(9) + assertFalse(disposable3.isDisposed) + // check that the scheduler automatically disposes of the tasks after being shut down + it.shutdown() + setScheduler(10, -1) + val disposable4 = it.scheduleDirect { + expectUnreached() + } + assertTrue(disposable4.isDisposed) + RxJavaPlugins.setScheduleHandler(null) + finish(11) + } + + @Test + fun testSchedulerWithNoDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithNoDelay(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerWithNoDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithNoDelay(scheduler.createWorker()::schedule) + } + + private suspend fun testRunnableWithNoDelay(block: RxSchedulerBlockNoDelay) { + expect(1) + suspendCancellableCoroutine { + block(Runnable { + expect(2) + it.resume(Unit) + }) + } + yield() + finish(3) + } + + @Test + fun testSchedulerWithDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler::scheduleDirect, 300) + } + + @Test + fun testSchedulerWorkerWithDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler.createWorker()::schedule, 300) + } + + @Test + fun testSchedulerWithZeroDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerWithZeroDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler.createWorker()::schedule) + } + + private suspend fun testRunnableWithDelay(block: RxSchedulerBlockWithDelay, delayMillis: Long = 0) { + expect(1) + suspendCancellableCoroutine { + block({ + expect(2) + it.resume(Unit) + }, delayMillis, TimeUnit.MILLISECONDS) + } + finish(3) + } + + @Test + fun testAsSchedulerWithNegativeDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler::scheduleDirect, -1) + } + + @Test + fun testAsSchedulerWorkerWithNegativeDelay(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableWithDelay(scheduler.createWorker()::schedule, -1) + } + + @Test + fun testSchedulerImmediateDispose(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableImmediateDispose(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerImmediateDispose(): Unit = runTest { + val scheduler = (currentDispatcher() as CoroutineDispatcher).asScheduler() + testRunnableImmediateDispose(scheduler.createWorker()::schedule) + } + + private fun testRunnableImmediateDispose(block: RxSchedulerBlockNoDelay) { + val disposable = block { + expectUnreached() + } + disposable.dispose() + } + + @Test + fun testConvertDispatcherToOriginalScheduler(): Unit = runTest { + val originalScheduler = Schedulers.io() + val dispatcher = originalScheduler.asCoroutineDispatcher() + val scheduler = dispatcher.asScheduler() + assertSame(originalScheduler, scheduler) + } + + @Test + fun testConvertSchedulerToOriginalDispatcher(): Unit = runTest { + val originalDispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = originalDispatcher.asScheduler() + val dispatcher = scheduler.asCoroutineDispatcher() + assertSame(originalDispatcher, dispatcher) + } + + @Test + fun testSchedulerExpectRxPluginsCall(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableExpectRxPluginsCall(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerExpectRxPluginsCall(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableExpectRxPluginsCall(scheduler.createWorker()::schedule) + } + + private suspend fun testRunnableExpectRxPluginsCall(block: RxSchedulerBlockNoDelay) { + expect(1) + setScheduler(2, 4) + suspendCancellableCoroutine { + block(Runnable { + expect(5) + it.resume(Unit) + }) + expect(3) + } + RxJavaPlugins.setScheduleHandler(null) + finish(6) + } + + @Test + fun testSchedulerExpectRxPluginsCallWithDelay(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + testRunnableExpectRxPluginsCallDelay(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerExpectRxPluginsCallWithDelay(): Unit = runTest { + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + val worker = scheduler.createWorker() + testRunnableExpectRxPluginsCallDelay(worker::schedule) + } + + private suspend fun testRunnableExpectRxPluginsCallDelay(block: RxSchedulerBlockWithDelay) { + expect(1) + setScheduler(2, 4) + suspendCancellableCoroutine { + block({ + expect(5) + it.resume(Unit) + }, 10, TimeUnit.MILLISECONDS) + expect(3) + } + RxJavaPlugins.setScheduleHandler(null) + finish(6) + } + + private fun setScheduler(expectedCountOnSchedule: Int, expectCountOnRun: Int) { + RxJavaPlugins.setScheduleHandler { + expect(expectedCountOnSchedule) + Runnable { + expect(expectCountOnRun) + it.run() + } + } + } + + /** + * Tests that [Scheduler.Worker] runs all work sequentially. + */ + @Test + fun testWorkerSequentialOrdering() = runTest { + expect(1) + val scheduler = Dispatchers.Default.asScheduler() + val worker = scheduler.createWorker() + val iterations = 100 + for (i in 0..iterations) { + worker.schedule { + expect(2 + i) + } + } + suspendCoroutine { + worker.schedule { + it.resume(Unit) + } + } + finish((iterations + 2) + 1) + } + + /** + * Test that ensures that delays are actually respected (tasks scheduled sooner in the future run before tasks scheduled later, + * even when the later task is submitted before the earlier one) + */ + @Test + fun testSchedulerRespectsDelays(): Unit = runTest { + val scheduler = Dispatchers.Default.asScheduler() + testRunnableRespectsDelays(scheduler::scheduleDirect) + } + + @Test + fun testSchedulerWorkerRespectsDelays(): Unit = runTest { + val scheduler = Dispatchers.Default.asScheduler() + testRunnableRespectsDelays(scheduler.createWorker()::schedule) + } + + private suspend fun testRunnableRespectsDelays(block: RxSchedulerBlockWithDelay) { + expect(1) + val semaphore = Semaphore(2, 2) + block({ + expect(3) + semaphore.release() + }, 100, TimeUnit.MILLISECONDS) + block({ + expect(2) + semaphore.release() + }, 1, TimeUnit.MILLISECONDS) + semaphore.acquire() + semaphore.acquire() + finish(4) + } + + /** + * Tests that cancelling a runnable in one worker doesn't affect work in another scheduler. + * + * This is part of expected behavior documented. + */ + @Test + fun testMultipleWorkerCancellation(): Unit = runTest { + expect(1) + val dispatcher = currentDispatcher() as CoroutineDispatcher + val scheduler = dispatcher.asScheduler() + suspendCancellableCoroutine { + val workerOne = scheduler.createWorker() + workerOne.schedule({ + expect(3) + it.resume(Unit) + }, 50, TimeUnit.MILLISECONDS) + val workerTwo = scheduler.createWorker() + workerTwo.schedule({ + expectUnreached() + }, 1000, TimeUnit.MILLISECONDS) + workerTwo.dispose() + expect(2) + } + finish(4) + } } + +typealias RxSchedulerBlockNoDelay = (Runnable) -> Disposable +typealias RxSchedulerBlockWithDelay = (Runnable, Long, TimeUnit) -> Disposable \ No newline at end of file From 83ffd171466f840bca59561112e2e5a3dc62bde7 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Thu, 24 Feb 2022 01:25:43 -0800 Subject: [PATCH 23/29] =?UTF-8?q?Properly=20cleanup=20completion=20in=20Sa?= =?UTF-8?q?feCollector=20to=20avoid=20unintended=20memo=E2=80=A6=20(#3199)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Properly cleanup completion in SafeCollector to avoid unintended memory leak that regular coroutines (e.g. unsafe flow) are not prone to Also, FieldWalker is improved to avoid "illegal reflective access" Fixes #3197 Co-authored-by: Roman Elizarov --- .../jvm/src/flow/internal/SafeCollector.kt | 43 +++++++++++------ .../jvm/test/FieldWalker.kt | 14 ++++-- .../test/flow/SafeCollectorMemoryLeakTest.kt | 48 +++++++++++++++++++ 3 files changed, 88 insertions(+), 17 deletions(-) create mode 100644 kotlinx-coroutines-core/jvm/test/flow/SafeCollectorMemoryLeakTest.kt diff --git a/kotlinx-coroutines-core/jvm/src/flow/internal/SafeCollector.kt b/kotlinx-coroutines-core/jvm/src/flow/internal/SafeCollector.kt index ea973287a7..cad3f1aea4 100644 --- a/kotlinx-coroutines-core/jvm/src/flow/internal/SafeCollector.kt +++ b/kotlinx-coroutines-core/jvm/src/flow/internal/SafeCollector.kt @@ -29,15 +29,22 @@ internal actual class SafeCollector actual constructor( @JvmField // Note, it is non-capturing lambda, so no extra allocation during init of SafeCollector internal actual val collectContextSize = collectContext.fold(0) { count, _ -> count + 1 } + + // Either context of the last emission or wrapper 'DownstreamExceptionContext' private var lastEmissionContext: CoroutineContext? = null + // Completion if we are currently suspended or within completion body or null otherwise private var completion: Continuation? = null - // ContinuationImpl + /* + * This property is accessed in two places: + * * ContinuationImpl invokes this in its `releaseIntercepted` as `context[ContinuationInterceptor]!!` + * * When we are within a callee, it is used to create its continuation object with this collector as completion + */ override val context: CoroutineContext - get() = completion?.context ?: EmptyCoroutineContext + get() = lastEmissionContext ?: EmptyCoroutineContext override fun invokeSuspend(result: Result): Any { - result.onFailure { lastEmissionContext = DownstreamExceptionElement(it) } + result.onFailure { lastEmissionContext = DownstreamExceptionContext(it, context) } completion?.resumeWith(result as Result) return COROUTINE_SUSPENDED } @@ -59,7 +66,9 @@ internal actual class SafeCollector actual constructor( emit(uCont, value) } catch (e: Throwable) { // Save the fact that exception from emit (or even check context) has been thrown - lastEmissionContext = DownstreamExceptionElement(e) + // Note, that this can the first emit and lastEmissionContext may not be saved yet, + // hence we use `uCont.context` here. + lastEmissionContext = DownstreamExceptionContext(e, uCont.context) throw e } } @@ -72,9 +81,18 @@ internal actual class SafeCollector actual constructor( val previousContext = lastEmissionContext if (previousContext !== currentContext) { checkContext(currentContext, previousContext, value) + lastEmissionContext = currentContext } completion = uCont - return emitFun(collector as FlowCollector, value, this as Continuation) + val result = emitFun(collector as FlowCollector, value, this as Continuation) + /* + * If the callee hasn't suspended, that means that it won't (it's forbidden) call 'resumeWith` (-> `invokeSuspend`) + * and we don't have to retain a strong reference to it to avoid memory leaks. + */ + if (result != COROUTINE_SUSPENDED) { + completion = null + } + return result } private fun checkContext( @@ -82,14 +100,13 @@ internal actual class SafeCollector actual constructor( previousContext: CoroutineContext?, value: T ) { - if (previousContext is DownstreamExceptionElement) { + if (previousContext is DownstreamExceptionContext) { exceptionTransparencyViolated(previousContext, value) } checkContext(currentContext) - lastEmissionContext = currentContext } - private fun exceptionTransparencyViolated(exception: DownstreamExceptionElement, value: Any?) { + private fun exceptionTransparencyViolated(exception: DownstreamExceptionContext, value: Any?) { /* * Exception transparency ensures that if a `collect` block or any intermediate operator * throws an exception, then no more values will be received by it. @@ -122,14 +139,12 @@ internal actual class SafeCollector actual constructor( For a more detailed explanation, please refer to Flow documentation. """.trimIndent()) } - } -internal class DownstreamExceptionElement(@JvmField val e: Throwable) : CoroutineContext.Element { - companion object Key : CoroutineContext.Key - - override val key: CoroutineContext.Key<*> = Key -} +internal class DownstreamExceptionContext( + @JvmField val e: Throwable, + originalContext: CoroutineContext +) : CoroutineContext by originalContext private object NoOpContinuation : Continuation { override val context: CoroutineContext = EmptyCoroutineContext diff --git a/kotlinx-coroutines-core/jvm/test/FieldWalker.kt b/kotlinx-coroutines-core/jvm/test/FieldWalker.kt index 52bcce3c69..7b2aaf63fc 100644 --- a/kotlinx-coroutines-core/jvm/test/FieldWalker.kt +++ b/kotlinx-coroutines-core/jvm/test/FieldWalker.kt @@ -9,6 +9,7 @@ import java.lang.reflect.* import java.text.* import java.util.* import java.util.Collections.* +import java.util.concurrent.* import java.util.concurrent.atomic.* import java.util.concurrent.locks.* import kotlin.test.* @@ -26,11 +27,11 @@ object FieldWalker { // excluded/terminal classes (don't walk them) fieldsCache += listOf( Any::class, String::class, Thread::class, Throwable::class, StackTraceElement::class, - WeakReference::class, ReferenceQueue::class, AbstractMap::class, - ReentrantReadWriteLock::class, SimpleDateFormat::class + WeakReference::class, ReferenceQueue::class, AbstractMap::class, Enum::class, + ReentrantLock::class, ReentrantReadWriteLock::class, SimpleDateFormat::class, ThreadPoolExecutor::class, ) .map { it.java } - .associateWith { emptyList() } + .associateWith { emptyList() } } /* @@ -159,6 +160,13 @@ object FieldWalker { && !(it.type.isArray && it.type.componentType.isPrimitive) && it.name != "previousOut" // System.out from TestBase that we store in a field to restore later } + check(fields.isEmpty() || !type.name.startsWith("java.")) { + """ + Trying to walk trough JDK's '$type' will get into illegal reflective access on JDK 9+. + Either modify your test to avoid usage of this class or update FieldWalker code to retrieve + the captured state of this class without going through reflection (see how collections are handled). + """.trimIndent() + } fields.forEach { it.isAccessible = true } // make them all accessible result.addAll(fields) type = type.superclass diff --git a/kotlinx-coroutines-core/jvm/test/flow/SafeCollectorMemoryLeakTest.kt b/kotlinx-coroutines-core/jvm/test/flow/SafeCollectorMemoryLeakTest.kt new file mode 100644 index 0000000000..b75ec60ed5 --- /dev/null +++ b/kotlinx-coroutines-core/jvm/test/flow/SafeCollectorMemoryLeakTest.kt @@ -0,0 +1,48 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.flow + +import kotlinx.coroutines.* +import org.junit.* + +class SafeCollectorMemoryLeakTest : TestBase() { + // custom List.forEach impl to avoid using iterator (FieldWalker cannot scan it) + private inline fun List.listForEach(action: (T) -> Unit) { + for (i in indices) action(get(i)) + } + + @Test + fun testCompletionIsProperlyCleanedUp() = runBlocking { + val job = flow { + emit(listOf(239)) + expect(2) + hang {} + }.transform { l -> l.listForEach { _ -> emit(42) } } + .onEach { expect(1) } + .launchIn(this) + yield() + expect(3) + FieldWalker.assertReachableCount(0, job) { it == 239 } + job.cancelAndJoin() + finish(4) + } + + @Test + fun testCompletionIsNotCleanedUp() = runBlocking { + val job = flow { + emit(listOf(239)) + hang {} + }.transform { l -> l.listForEach { _ -> emit(42) } } + .onEach { + expect(1) + hang { finish(3) } + } + .launchIn(this) + yield() + expect(2) + FieldWalker.assertReachableCount(1, job) { it == 239 } + job.cancelAndJoin() + } +} From b545807d00425ac1838a2105a3578795eaab1167 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Thu, 24 Feb 2022 01:48:42 -0800 Subject: [PATCH 24/29] Validate that throwing tryCopy does not crash with an internal error (#3063) Motivated by #3031 --- .../StackTraceRecoveryCustomExceptionsTest.kt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryCustomExceptionsTest.kt b/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryCustomExceptionsTest.kt index dba738a8d3..d4e19040a5 100644 --- a/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryCustomExceptionsTest.kt +++ b/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryCustomExceptionsTest.kt @@ -124,4 +124,22 @@ class StackTraceRecoveryCustomExceptionsTest : TestBase() { assertTrue(ex is CopyableWithCustomMessage) assertEquals("Recovered: [OK]", ex.message) } + + @Test + fun testTryCopyThrows() = runTest { + class FailingException : Exception(), CopyableThrowable { + override fun createCopy(): FailingException? { + TODO("Not yet implemented") + } + } + + val e = FailingException() + val result = runCatching { + coroutineScope { + throw e + } + } + + assertSame(e, result.exceptionOrNull()) + } } From f9917429de77cd6a55285bd711db62fc1f6a4f8b Mon Sep 17 00:00:00 2001 From: Scott Olsen Date: Thu, 31 Mar 2022 04:59:08 -0400 Subject: [PATCH 25/29] docs: clarify section on coroutine memory consumption (#3225) Updates the section "Coroutines are light-weight" to clarify its comparison of the resource-intensiveness of coroutines and threads. The content is much the same. This commit also makes the code sample in this section runnable and omits the surrounding runBlocking block. --- docs/topics/coroutines-basics.md | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/docs/topics/coroutines-basics.md b/docs/topics/coroutines-basics.md index 885c29fc28..68ae97886f 100644 --- a/docs/topics/coroutines-basics.md +++ b/docs/topics/coroutines-basics.md @@ -245,14 +245,17 @@ Done -## Coroutines ARE light-weight +## Coroutines are light-weight -Run the following code: +Coroutines are less resource-intensive than JVM threads. Code that exhausts the +JVM's available memory when using threads can be expressed using coroutines +without hitting resource limits. For example, the following code launches +100000 distinct coroutines that each wait 5 seconds and then print a period +('.') while consuming very little memory: ```kotlin import kotlinx.coroutines.* -//sampleStart fun main() = runBlocking { repeat(100_000) { // launch a lot of coroutines launch { @@ -261,8 +264,9 @@ fun main() = runBlocking { } } } -//sampleEnd ``` + > You can get the full code [here](../../kotlinx-coroutines-core/jvm/test/guide/example-basic-06.kt). > @@ -270,10 +274,9 @@ fun main() = runBlocking { -It launches 100K coroutines and, after 5 seconds, each coroutine prints a dot. - -Now, try that with threads (remove `runBlocking`, replace `launch` with `thread`, and replace `delay` with `Thread.sleep`). -What would happen? (Most likely your code will produce some sort of out-of-memory error) +If you write the same program using threads (remove `runBlocking`, replace +`launch` with `thread`, and replace `delay` with `Thread.sleep`), it will +likely consume too much memory and throw an out-of-memory error. From 6c326e414c569c5e116e321156272bb241d2e6e0 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Thu, 31 Mar 2022 02:20:45 -0700 Subject: [PATCH 26/29] Do not mention service loading for CoroutineExceptionHandler as it is not meant to be a public mechanism (#3228) --- docs/topics/exception-handling.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/topics/exception-handling.md b/docs/topics/exception-handling.md index 9e14ff2afd..5ee55437db 100644 --- a/docs/topics/exception-handling.md +++ b/docs/topics/exception-handling.md @@ -75,12 +75,6 @@ You cannot recover from the exception in the `CoroutineExceptionHandler`. The co with the corresponding exception when the handler is called. Normally, the handler is used to log the exception, show some kind of error message, terminate, and/or restart the application. -On JVM it is possible to redefine global exception handler for all coroutines by registering [CoroutineExceptionHandler] via -[`ServiceLoader`](https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html). -Global exception handler is similar to -[`Thread.defaultUncaughtExceptionHandler`](https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#setDefaultUncaughtExceptionHandler(java.lang.Thread.UncaughtExceptionHandler)) -which is used when no more specific handlers are registered. -On Android, `uncaughtExceptionPreHandler` is installed as a global coroutine exception handler. `CoroutineExceptionHandler` is invoked only on **uncaught** exceptions — exceptions that were not handled in any other way. In particular, all _children_ coroutines (coroutines created in the context of another [Job]) delegate handling of From 8133c973bfa339d89c8df09e2889c9f884fa2b04 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Thu, 31 Mar 2022 03:48:19 -0700 Subject: [PATCH 27/29] Fix limitedParallelism implementation on K/N (#3226) The initial implementation predates new memory model and was never working on it Fixes #3223 --- .../common/src/internal/LimitedDispatcher.kt | 9 +-- .../test/LimitedParallelismSharedTest.kt | 34 +++++++++++ .../test/LimitedParallelismConcurrentTest.kt | 59 +++++++++++++++++++ ...mitedParallelismUnhandledExceptionTest.kt} | 25 +------- 4 files changed, 99 insertions(+), 28 deletions(-) create mode 100644 kotlinx-coroutines-core/common/test/LimitedParallelismSharedTest.kt create mode 100644 kotlinx-coroutines-core/concurrent/test/LimitedParallelismConcurrentTest.kt rename kotlinx-coroutines-core/jvm/test/{LimitedParallelismTest.kt => LimitedParallelismUnhandledExceptionTest.kt} (51%) diff --git a/kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt b/kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt index 892375b89f..28f37ecf1d 100644 --- a/kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt +++ b/kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt @@ -23,6 +23,9 @@ internal class LimitedDispatcher( private val queue = LockFreeTaskQueue(singleConsumer = false) + // A separate object that we can synchronize on for K/N + private val workerAllocationLock = SynchronizedObject() + @ExperimentalCoroutinesApi override fun limitedParallelism(parallelism: Int): CoroutineDispatcher { parallelism.checkParallelism() @@ -50,8 +53,7 @@ internal class LimitedDispatcher( continue } - @Suppress("CAST_NEVER_SUCCEEDS") - synchronized(this as SynchronizedObject) { + synchronized(workerAllocationLock) { --runningWorkers if (queue.size == 0) return ++runningWorkers @@ -87,8 +89,7 @@ internal class LimitedDispatcher( } private fun tryAllocateWorker(): Boolean { - @Suppress("CAST_NEVER_SUCCEEDS") - synchronized(this as SynchronizedObject) { + synchronized(workerAllocationLock) { if (runningWorkers >= parallelism) return false ++runningWorkers return true diff --git a/kotlinx-coroutines-core/common/test/LimitedParallelismSharedTest.kt b/kotlinx-coroutines-core/common/test/LimitedParallelismSharedTest.kt new file mode 100644 index 0000000000..d01e85716b --- /dev/null +++ b/kotlinx-coroutines-core/common/test/LimitedParallelismSharedTest.kt @@ -0,0 +1,34 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines + +import kotlin.test.* + +class LimitedParallelismSharedTest : TestBase() { + + @Test + fun testLimitedDefault() = runTest { + // Test that evaluates the very basic completion of tasks in limited dispatcher + // for all supported platforms. + // For more specific and concurrent tests, see 'concurrent' package. + val view = Dispatchers.Default.limitedParallelism(1) + val view2 = Dispatchers.Default.limitedParallelism(1) + val j1 = launch(view) { + while (true) { + yield() + } + } + val j2 = launch(view2) { j1.cancel() } + joinAll(j1, j2) + } + + @Test + fun testParallelismSpec() { + assertFailsWith { Dispatchers.Default.limitedParallelism(0) } + assertFailsWith { Dispatchers.Default.limitedParallelism(-1) } + assertFailsWith { Dispatchers.Default.limitedParallelism(Int.MIN_VALUE) } + Dispatchers.Default.limitedParallelism(Int.MAX_VALUE) + } +} diff --git a/kotlinx-coroutines-core/concurrent/test/LimitedParallelismConcurrentTest.kt b/kotlinx-coroutines-core/concurrent/test/LimitedParallelismConcurrentTest.kt new file mode 100644 index 0000000000..964f678e74 --- /dev/null +++ b/kotlinx-coroutines-core/concurrent/test/LimitedParallelismConcurrentTest.kt @@ -0,0 +1,59 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +import kotlinx.atomicfu.* +import kotlinx.coroutines.* +import kotlinx.coroutines.exceptions.* +import kotlin.test.* + +class LimitedParallelismConcurrentTest : TestBase() { + + private val targetParallelism = 4 + private val iterations = 100_000 + private val parallelism = atomic(0) + + private fun checkParallelism() { + val value = parallelism.incrementAndGet() + randomWait() + assertTrue { value <= targetParallelism } + parallelism.decrementAndGet() + } + + @Test + fun testLimitedExecutor() = runMtTest { + val executor = newFixedThreadPoolContext(targetParallelism, "test") + val view = executor.limitedParallelism(targetParallelism) + doStress { + repeat(iterations) { + launch(view) { + checkParallelism() + } + } + } + executor.close() + } + + private suspend inline fun doStress(crossinline block: suspend CoroutineScope.() -> Unit) { + repeat(stressTestMultiplier) { + coroutineScope { + block() + } + } + } + + @Test + fun testTaskFairness() = runMtTest { + val executor = newSingleThreadContext("test") + val view = executor.limitedParallelism(1) + val view2 = executor.limitedParallelism(1) + val j1 = launch(view) { + while (true) { + yield() + } + } + val j2 = launch(view2) { j1.cancel() } + joinAll(j1, j2) + executor.close() + } +} diff --git a/kotlinx-coroutines-core/jvm/test/LimitedParallelismTest.kt b/kotlinx-coroutines-core/jvm/test/LimitedParallelismUnhandledExceptionTest.kt similarity index 51% rename from kotlinx-coroutines-core/jvm/test/LimitedParallelismTest.kt rename to kotlinx-coroutines-core/jvm/test/LimitedParallelismUnhandledExceptionTest.kt index 30c54117a9..8d48aa43b3 100644 --- a/kotlinx-coroutines-core/jvm/test/LimitedParallelismTest.kt +++ b/kotlinx-coroutines-core/jvm/test/LimitedParallelismUnhandledExceptionTest.kt @@ -9,30 +9,7 @@ import java.util.concurrent.* import kotlin.coroutines.* import kotlin.test.* -class LimitedParallelismTest : TestBase() { - - @Test - fun testParallelismSpec() { - assertFailsWith { Dispatchers.Default.limitedParallelism(0) } - assertFailsWith { Dispatchers.Default.limitedParallelism(-1) } - assertFailsWith { Dispatchers.Default.limitedParallelism(Int.MIN_VALUE) } - Dispatchers.Default.limitedParallelism(Int.MAX_VALUE) - } - - @Test - fun testTaskFairness() = runTest { - val executor = newSingleThreadContext("test") - val view = executor.limitedParallelism(1) - val view2 = executor.limitedParallelism(1) - val j1 = launch(view) { - while (true) { - yield() - } - } - val j2 = launch(view2) { j1.cancel() } - joinAll(j1, j2) - executor.close() - } +class LimitedParallelismUnhandledExceptionTest : TestBase() { @Test fun testUnhandledException() = runTest { From a5dd74b2b325113586768b8b61af6e2833a139c2 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Mon, 4 Apr 2022 03:57:56 -0700 Subject: [PATCH 28/29] CopyableThreadContextElement implementation (#3227) New approach eagerly copies corresponding elements to avoid accidental top-level reuse and also provides merge capability in case when an element is being overwritten. Merge capability is crucial in tracing scenarios to properly preserve the state of linked thread locals Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com> --- .../api/kotlinx-coroutines-core.api | 4 +- .../common/src/Builders.common.kt | 3 +- .../common/src/CoroutineContext.common.kt | 12 +- .../js/src/CoroutineContext.kt | 4 + .../jvm/src/CoroutineContext.kt | 87 +++++++++--- .../jvm/src/ThreadContextElement.kt | 33 ++++- .../jvm/test/ThreadContextElementTest.kt | 13 +- .../test/ThreadContextMutableCopiesTest.kt | 134 ++++++++++++++++++ .../native/src/CoroutineContext.kt | 4 + 9 files changed, 259 insertions(+), 35 deletions(-) create mode 100644 kotlinx-coroutines-core/jvm/test/ThreadContextMutableCopiesTest.kt diff --git a/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api b/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api index d1fc624a5e..79f3cf4308 100644 --- a/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api +++ b/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api @@ -141,7 +141,8 @@ public final class kotlinx/coroutines/CompletionHandlerException : java/lang/Run } public abstract interface class kotlinx/coroutines/CopyableThreadContextElement : kotlinx/coroutines/ThreadContextElement { - public abstract fun copyForChildCoroutine ()Lkotlinx/coroutines/CopyableThreadContextElement; + public abstract fun copyForChild ()Lkotlinx/coroutines/CopyableThreadContextElement; + public abstract fun mergeForChild (Lkotlin/coroutines/CoroutineContext$Element;)Lkotlin/coroutines/CoroutineContext; } public final class kotlinx/coroutines/CopyableThreadContextElement$DefaultImpls { @@ -156,6 +157,7 @@ public abstract interface class kotlinx/coroutines/CopyableThrowable { } public final class kotlinx/coroutines/CoroutineContextKt { + public static final fun newCoroutineContext (Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;)Lkotlin/coroutines/CoroutineContext; public static final fun newCoroutineContext (Lkotlinx/coroutines/CoroutineScope;Lkotlin/coroutines/CoroutineContext;)Lkotlin/coroutines/CoroutineContext; } diff --git a/kotlinx-coroutines-core/common/src/Builders.common.kt b/kotlinx-coroutines-core/common/src/Builders.common.kt index a11ffe9eb4..c360724245 100644 --- a/kotlinx-coroutines-core/common/src/Builders.common.kt +++ b/kotlinx-coroutines-core/common/src/Builders.common.kt @@ -148,7 +148,8 @@ public suspend fun withContext( return suspendCoroutineUninterceptedOrReturn sc@ { uCont -> // compute new context val oldContext = uCont.context - val newContext = oldContext + context + // Copy CopyableThreadContextElement if necessary + val newContext = oldContext.newCoroutineContext(context) // always check for cancellation of new context newContext.ensureActive() // FAST PATH #1 -- new context is the same as the old one diff --git a/kotlinx-coroutines-core/common/src/CoroutineContext.common.kt b/kotlinx-coroutines-core/common/src/CoroutineContext.common.kt index da094e152d..9153f39821 100644 --- a/kotlinx-coroutines-core/common/src/CoroutineContext.common.kt +++ b/kotlinx-coroutines-core/common/src/CoroutineContext.common.kt @@ -7,11 +7,19 @@ package kotlinx.coroutines import kotlin.coroutines.* /** - * Creates a context for the new coroutine. It installs [Dispatchers.Default] when no other dispatcher or - * [ContinuationInterceptor] is specified, and adds optional support for debugging facilities (when turned on). + * Creates a context for a new coroutine. It installs [Dispatchers.Default] when no other dispatcher or + * [ContinuationInterceptor] is specified and adds optional support for debugging facilities (when turned on) + * and copyable-thread-local facilities on JVM. */ public expect fun CoroutineScope.newCoroutineContext(context: CoroutineContext): CoroutineContext +/** + * Creates a context for coroutine builder functions that do not launch a new coroutine, e.g. [withContext]. + * @suppress + */ +@InternalCoroutinesApi +public expect fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext + @PublishedApi @Suppress("PropertyName") internal expect val DefaultDelay: Delay diff --git a/kotlinx-coroutines-core/js/src/CoroutineContext.kt b/kotlinx-coroutines-core/js/src/CoroutineContext.kt index 95cb3c2964..8036c88a10 100644 --- a/kotlinx-coroutines-core/js/src/CoroutineContext.kt +++ b/kotlinx-coroutines-core/js/src/CoroutineContext.kt @@ -42,6 +42,10 @@ public actual fun CoroutineScope.newCoroutineContext(context: CoroutineContext): combined + Dispatchers.Default else combined } +public actual fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext { + return this + addedContext +} + // No debugging facilities on JS internal actual inline fun withCoroutineContext(context: CoroutineContext, countOrElement: Any?, block: () -> T): T = block() internal actual inline fun withContinuationContext(continuation: Continuation<*>, countOrElement: Any?, block: () -> T): T = block() diff --git a/kotlinx-coroutines-core/jvm/src/CoroutineContext.kt b/kotlinx-coroutines-core/jvm/src/CoroutineContext.kt index 6291ea2b97..e08b805295 100644 --- a/kotlinx-coroutines-core/jvm/src/CoroutineContext.kt +++ b/kotlinx-coroutines-core/jvm/src/CoroutineContext.kt @@ -9,36 +9,83 @@ import kotlin.coroutines.* import kotlin.coroutines.jvm.internal.CoroutineStackFrame /** - * Creates context for the new coroutine. It installs [Dispatchers.Default] when no other dispatcher nor - * [ContinuationInterceptor] is specified, and adds optional support for debugging facilities (when turned on). - * + * Creates a context for a new coroutine. It installs [Dispatchers.Default] when no other dispatcher or + * [ContinuationInterceptor] is specified and adds optional support for debugging facilities (when turned on) + * and copyable-thread-local facilities on JVM. * See [DEBUG_PROPERTY_NAME] for description of debugging facilities on JVM. */ @ExperimentalCoroutinesApi public actual fun CoroutineScope.newCoroutineContext(context: CoroutineContext): CoroutineContext { - val combined = coroutineContext.foldCopiesForChildCoroutine() + context + val combined = foldCopies(coroutineContext, context, true) val debug = if (DEBUG) combined + CoroutineId(COROUTINE_ID.incrementAndGet()) else combined return if (combined !== Dispatchers.Default && combined[ContinuationInterceptor] == null) debug + Dispatchers.Default else debug } /** - * Returns the [CoroutineContext] for a child coroutine to inherit. - * - * If any [CopyableThreadContextElement] is in the [this], calls - * [CopyableThreadContextElement.copyForChildCoroutine] on each, returning a new [CoroutineContext] - * by folding the returned copied elements into [this]. - * - * Returns [this] if `this` has zero [CopyableThreadContextElement] in it. + * Creates a context for coroutine builder functions that do not launch a new coroutine, e.g. [withContext]. + * @suppress */ -private fun CoroutineContext.foldCopiesForChildCoroutine(): CoroutineContext { - val hasToCopy = fold(false) { result, it -> - result || it is CopyableThreadContextElement<*> +@InternalCoroutinesApi +public actual fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext { + /* + * Fast-path: we only have to copy/merge if 'addedContext' (which typically has one or two elements) + * contains copyable elements. + */ + if (!addedContext.hasCopyableElements()) return this + addedContext + return foldCopies(this, addedContext, false) +} + +private fun CoroutineContext.hasCopyableElements(): Boolean = + fold(false) { result, it -> result || it is CopyableThreadContextElement<*> } + +/** + * Folds two contexts properly applying [CopyableThreadContextElement] rules when necessary. + * The rules are the following: + * * If neither context has CTCE, the sum of two contexts is returned + * * Every CTCE from the left-hand side context that does not have a matching (by key) element from right-hand side context + * is [copied][CopyableThreadContextElement.copyForChild] if [isNewCoroutine] is `true`. + * * Every CTCE from the left-hand side context that has a matching element in the right-hand side context is [merged][CopyableThreadContextElement.mergeForChild] + * * Every CTCE from the right-hand side context that hasn't been merged is copied + * * Everything else is added to the resulting context as is. + */ +private fun foldCopies(originalContext: CoroutineContext, appendContext: CoroutineContext, isNewCoroutine: Boolean): CoroutineContext { + // Do we have something to copy left-hand side? + val hasElementsLeft = originalContext.hasCopyableElements() + val hasElementsRight = appendContext.hasCopyableElements() + + // Nothing to fold, so just return the sum of contexts + if (!hasElementsLeft && !hasElementsRight) { + return originalContext + appendContext + } + + var leftoverContext = appendContext + val folded = originalContext.fold(EmptyCoroutineContext) { result, element -> + if (element !is CopyableThreadContextElement<*>) return@fold result + element + // Will this element be overwritten? + val newElement = leftoverContext[element.key] + // No, just copy it + if (newElement == null) { + // For 'withContext'-like builders we do not copy as the element is not shared + return@fold result + if (isNewCoroutine) element.copyForChild() else element + } + // Yes, then first remove the element from append context + leftoverContext = leftoverContext.minusKey(element.key) + // Return the sum + @Suppress("UNCHECKED_CAST") + return@fold result + (element as CopyableThreadContextElement).mergeForChild(newElement) } - if (!hasToCopy) return this - return fold(EmptyCoroutineContext) { combined, it -> - combined + if (it is CopyableThreadContextElement<*>) it.copyForChildCoroutine() else it + + if (hasElementsRight) { + leftoverContext = leftoverContext.fold(EmptyCoroutineContext) { result, element -> + // We're appending new context element -- we have to copy it, otherwise it may be shared with others + if (element is CopyableThreadContextElement<*>) { + return@fold result + element.copyForChild() + } + return@fold result + element + } } + return folded + leftoverContext } /** @@ -77,7 +124,7 @@ internal actual inline fun withContinuationContext(continuation: Continuatio internal fun Continuation<*>.updateUndispatchedCompletion(context: CoroutineContext, oldValue: Any?): UndispatchedCoroutine<*>? { if (this !is CoroutineStackFrame) return null /* - * Fast-path to detect whether we have unispatched coroutine at all in our stack. + * Fast-path to detect whether we have undispatched coroutine at all in our stack. * * Implementation note. * If we ever find that stackwalking for thread-locals is way too slow, here is another idea: @@ -88,8 +135,8 @@ internal fun Continuation<*>.updateUndispatchedCompletion(context: CoroutineCont * Both options should work, but it requires more careful studying of the performance * and, mostly, maintainability impact. */ - val potentiallyHasUndispatchedCorotuine = context[UndispatchedMarker] !== null - if (!potentiallyHasUndispatchedCorotuine) return null + val potentiallyHasUndispatchedCoroutine = context[UndispatchedMarker] !== null + if (!potentiallyHasUndispatchedCoroutine) return null val completion = undispatchedCompletion() completion?.saveThreadContext(context, oldValue) return completion diff --git a/kotlinx-coroutines-core/jvm/src/ThreadContextElement.kt b/kotlinx-coroutines-core/jvm/src/ThreadContextElement.kt index 1a960699c7..d2b6b6b988 100644 --- a/kotlinx-coroutines-core/jvm/src/ThreadContextElement.kt +++ b/kotlinx-coroutines-core/jvm/src/ThreadContextElement.kt @@ -80,7 +80,7 @@ public interface ThreadContextElement : CoroutineContext.Element { /** * A [ThreadContextElement] copied whenever a child coroutine inherits a context containing it. * - * When an API uses a _mutable_ `ThreadLocal` for consistency, a [CopyableThreadContextElement] + * When an API uses a _mutable_ [ThreadLocal] for consistency, a [CopyableThreadContextElement] * can give coroutines "coroutine-safe" write access to that `ThreadLocal`. * * A write made to a `ThreadLocal` with a matching [CopyableThreadContextElement] by a coroutine @@ -99,6 +99,7 @@ public interface ThreadContextElement : CoroutineContext.Element { * ``` * class TraceContextElement(private val traceData: TraceData?) : CopyableThreadContextElement { * companion object Key : CoroutineContext.Key + * * override val key: CoroutineContext.Key = Key * * override fun updateThreadContext(context: CoroutineContext): TraceData? { @@ -111,24 +112,35 @@ public interface ThreadContextElement : CoroutineContext.Element { * traceThreadLocal.set(oldState) * } * - * override fun copyForChildCoroutine(): CopyableThreadContextElement { + * override fun copyForChild(): TraceContextElement { * // Copy from the ThreadLocal source of truth at child coroutine launch time. This makes * // ThreadLocal writes between resumption of the parent coroutine and the launch of the * // child coroutine visible to the child. * return TraceContextElement(traceThreadLocal.get()?.copy()) * } + * + * override fun mergeForChild(overwritingElement: CoroutineContext.Element): CoroutineContext { + * // Merge operation defines how to handle situations when both + * // the parent coroutine has an element in the context and + * // an element with the same key was also + * // explicitly passed to the child coroutine. + * // If merging does not require special behavior, + * // the copy of the element can be returned. + * return TraceContextElement(traceThreadLocal.get()?.copy()) + * } * } * ``` * - * A coroutine using this mechanism can safely call Java code that assumes it's called using a - * `Thread`. + * A coroutine using this mechanism can safely call Java code that assumes the corresponding thread local element's + * value is installed into the target thread local. */ +@DelicateCoroutinesApi @ExperimentalCoroutinesApi public interface CopyableThreadContextElement : ThreadContextElement { /** * Returns a [CopyableThreadContextElement] to replace `this` `CopyableThreadContextElement` in the child - * coroutine's context that is under construction. + * coroutine's context that is under construction if the added context does not contain an element with the same [key]. * * This function is called on the element each time a new coroutine inherits a context containing it, * and the returned value is folded into the context given to the child. @@ -136,7 +148,16 @@ public interface CopyableThreadContextElement : ThreadContextElement { * Since this method is called whenever a new coroutine is launched in a context containing this * [CopyableThreadContextElement], implementations are performance-sensitive. */ - public fun copyForChildCoroutine(): CopyableThreadContextElement + public fun copyForChild(): CopyableThreadContextElement + + /** + * Returns a [CopyableThreadContextElement] to replace `this` `CopyableThreadContextElement` in the child + * coroutine's context that is under construction if the added context does contain an element with the same [key]. + * + * This method is invoked on the original element, accepting as the parameter + * the element that is supposed to overwrite it. + */ + public fun mergeForChild(overwritingElement: CoroutineContext.Element): CoroutineContext } /** diff --git a/kotlinx-coroutines-core/jvm/test/ThreadContextElementTest.kt b/kotlinx-coroutines-core/jvm/test/ThreadContextElementTest.kt index baba4aa8e6..ec45406bce 100644 --- a/kotlinx-coroutines-core/jvm/test/ThreadContextElementTest.kt +++ b/kotlinx-coroutines-core/jvm/test/ThreadContextElementTest.kt @@ -126,8 +126,7 @@ class ThreadContextElementTest : TestBase() { @Test fun testCopyableThreadContextElementImplementsWriteVisibility() = runTest { newFixedThreadPoolContext(nThreads = 4, name = "withContext").use { - val startData = MyData() - withContext(it + CopyForChildCoroutineElement(startData)) { + withContext(it + CopyForChildCoroutineElement(MyData())) { val forBlockData = MyData() myThreadLocal.setForBlock(forBlockData) { assertSame(myThreadLocal.get(), forBlockData) @@ -153,7 +152,7 @@ class ThreadContextElementTest : TestBase() { assertSame(myThreadLocal.get(), forBlockData) } } - assertSame(myThreadLocal.get(), startData) // Asserts value was restored. + assertNull(myThreadLocal.get()) // Asserts value was restored to its origin } } } @@ -187,7 +186,7 @@ class MyElement(val data: MyData) : ThreadContextElement { } /** - * A [ThreadContextElement] that implements copy semantics in [copyForChildCoroutine]. + * A [ThreadContextElement] that implements copy semantics in [copyForChild]. */ class CopyForChildCoroutineElement(val data: MyData?) : CopyableThreadContextElement { companion object Key : CoroutineContext.Key @@ -201,6 +200,10 @@ class CopyForChildCoroutineElement(val data: MyData?) : CopyableThreadContextEle return oldState } + override fun mergeForChild(overwritingElement: CoroutineContext.Element): CopyForChildCoroutineElement { + TODO("Not used in tests") + } + override fun restoreThreadContext(context: CoroutineContext, oldState: MyData?) { myThreadLocal.set(oldState) } @@ -216,7 +219,7 @@ class CopyForChildCoroutineElement(val data: MyData?) : CopyableThreadContextEle * will be reflected in the parent coroutine's [CopyForChildCoroutineElement] when it yields the * thread and calls [restoreThreadContext]. */ - override fun copyForChildCoroutine(): CopyableThreadContextElement { + override fun copyForChild(): CopyForChildCoroutineElement { return CopyForChildCoroutineElement(myThreadLocal.get()) } } diff --git a/kotlinx-coroutines-core/jvm/test/ThreadContextMutableCopiesTest.kt b/kotlinx-coroutines-core/jvm/test/ThreadContextMutableCopiesTest.kt new file mode 100644 index 0000000000..34e5955fd7 --- /dev/null +++ b/kotlinx-coroutines-core/jvm/test/ThreadContextMutableCopiesTest.kt @@ -0,0 +1,134 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines + +import kotlin.coroutines.* +import kotlin.test.* + +class ThreadContextMutableCopiesTest : TestBase() { + companion object { + val threadLocalData: ThreadLocal> = ThreadLocal.withInitial { ArrayList() } + } + + class MyMutableElement( + val mutableData: MutableList + ) : CopyableThreadContextElement> { + + companion object Key : CoroutineContext.Key + + override val key: CoroutineContext.Key<*> + get() = Key + + override fun updateThreadContext(context: CoroutineContext): MutableList { + val st = threadLocalData.get() + threadLocalData.set(mutableData) + return st + } + + override fun restoreThreadContext(context: CoroutineContext, oldState: MutableList) { + threadLocalData.set(oldState) + } + + override fun copyForChild(): MyMutableElement { + return MyMutableElement(ArrayList(mutableData)) + } + + override fun mergeForChild(overwritingElement: CoroutineContext.Element): MyMutableElement { + overwritingElement as MyMutableElement // <- app-specific, may be another subtype + return MyMutableElement((mutableData.toSet() + overwritingElement.mutableData).toMutableList()) + } + } + + @Test + fun testDataIsCopied() = runTest { + val root = MyMutableElement(ArrayList()) + runBlocking(root) { + val data = threadLocalData.get() + expect(1) + launch(root) { + assertNotSame(data, threadLocalData.get()) + assertEquals(data, threadLocalData.get()) + finish(2) + } + } + } + + @Test + fun testDataIsNotOverwritten() = runTest { + val root = MyMutableElement(ArrayList()) + runBlocking(root) { + expect(1) + val originalData = threadLocalData.get() + threadLocalData.get().add("X") + launch { + threadLocalData.get().add("Y") + // Note here, +root overwrites the data + launch(Dispatchers.Default + root) { + assertEquals(listOf("X", "Y"), threadLocalData.get()) + assertNotSame(originalData, threadLocalData.get()) + finish(2) + } + } + } + } + + @Test + fun testDataIsMerged() = runTest { + val root = MyMutableElement(ArrayList()) + runBlocking(root) { + expect(1) + val originalData = threadLocalData.get() + threadLocalData.get().add("X") + launch { + threadLocalData.get().add("Y") + // Note here, +root overwrites the data + launch(Dispatchers.Default + MyMutableElement(mutableListOf("Z"))) { + assertEquals(listOf("X", "Y", "Z"), threadLocalData.get()) + assertNotSame(originalData, threadLocalData.get()) + finish(2) + } + } + } + } + + @Test + fun testDataIsNotOverwrittenWithContext() = runTest { + val root = MyMutableElement(ArrayList()) + runBlocking(root) { + val originalData = threadLocalData.get() + threadLocalData.get().add("X") + expect(1) + launch { + threadLocalData.get().add("Y") + // Note here, +root overwrites the data + withContext(Dispatchers.Default + root) { + assertEquals(listOf("X", "Y"), threadLocalData.get()) + assertNotSame(originalData, threadLocalData.get()) + finish(2) + } + } + } + } + + @Test + fun testDataIsCopiedForRunBlocking() = runTest { + val root = MyMutableElement(ArrayList()) + val originalData = root.mutableData + runBlocking(root) { + assertNotSame(originalData, threadLocalData.get()) + } + } + + @Test + fun testDataIsCopiedForCoroutine() = runTest { + val root = MyMutableElement(ArrayList()) + val originalData = root.mutableData + expect(1) + launch(root) { + assertNotSame(originalData, threadLocalData.get()) + finish(2) + } + } +} diff --git a/kotlinx-coroutines-core/native/src/CoroutineContext.kt b/kotlinx-coroutines-core/native/src/CoroutineContext.kt index e1e29581a7..6e2dac1a29 100644 --- a/kotlinx-coroutines-core/native/src/CoroutineContext.kt +++ b/kotlinx-coroutines-core/native/src/CoroutineContext.kt @@ -49,6 +49,10 @@ public actual fun CoroutineScope.newCoroutineContext(context: CoroutineContext): combined + (DefaultDelay as CoroutineContext.Element) else combined } +public actual fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext { + return this + addedContext +} + // No debugging facilities on native internal actual inline fun withCoroutineContext(context: CoroutineContext, countOrElement: Any?, block: () -> T): T = block() internal actual inline fun withContinuationContext(continuation: Continuation<*>, countOrElement: Any?, block: () -> T): T = block() From 429b5d1b35d1d63cd3de52aa46df5a01118b0c05 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Fri, 1 Apr 2022 20:10:16 +0300 Subject: [PATCH 29/29] Version 1.6.1 --- CHANGES.md | 14 ++++++++++++++ README.md | 16 ++++++++-------- gradle.properties | 2 +- kotlinx-coroutines-debug/README.md | 2 +- kotlinx-coroutines-test/README.md | 2 +- ui/coroutines-guide-ui.md | 2 +- 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 424959f9a7..2aba4abf59 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,19 @@ # Change log for kotlinx.coroutines +## Version 1.6.1 + +* Rollback of time-related functions dispatching on `Dispatchers.Main`. + This behavior was introduced in 1.6.0 and then found inconvenient and erroneous (#3106, #3113). +* Reworked the newly-introduced `CopyableThreadContextElement` to solve issues uncovered after the initial release (#3227). +* Fixed a bug with `ThreadLocalElement` not being properly updated in racy scenarios (#2930). +* Reverted eager loading of default `CoroutineExceptionHandler` that triggered ANR on some devices (#3180). +* New API to convert a `CoroutineDispatcher` to a Rx scheduler (#968, #548). Thanks @recheej! +* Fixed a memory leak with the very last element emitted from `flow` builder being retained in memory (#3197). +* Fixed a bug with `limitedParallelism` on K/N with new memory model throwing `ClassCastException` (#3223). +* `CoroutineContext` is added to the exception printed to the default `CoroutineExceptionHandler` to improve debuggability (#3153). +* Static memory consumption of `Dispatchers.Default` was significantly reduced (#3137). +* Updated slf4j version in `kotlinx-coroutines-slf4j` from 1.7.25 to 1.7.32. + ## Version 1.6.0 Note that this is a full changelog relative to the 1.5.2 version. Changelog relative to 1.6.0-RC3 can be found at the end. diff --git a/README.md b/README.md index 7f7558c81e..10910f9e22 100644 --- a/README.md +++ b/README.md @@ -2,12 +2,12 @@ [![official JetBrains project](https://jb.gg/badges/official.svg)](https://confluence.jetbrains.com/display/ALL/JetBrains+on+GitHub) [![GitHub license](https://img.shields.io/badge/license-Apache%20License%202.0-blue.svg?style=flat)](https://www.apache.org/licenses/LICENSE-2.0) -[![Download](https://img.shields.io/maven-central/v/org.jetbrains.kotlinx/kotlinx-coroutines-core/1.6.0)](https://search.maven.org/artifact/org.jetbrains.kotlinx/kotlinx-coroutines-core/1.6.0/pom) -[![Kotlin](https://img.shields.io/badge/kotlin-1.6.0-blue.svg?logo=kotlin)](http://kotlinlang.org) +[![Download](https://img.shields.io/maven-central/v/org.jetbrains.kotlinx/kotlinx-coroutines-core/1.6.1)](https://search.maven.org/artifact/org.jetbrains.kotlinx/kotlinx-coroutines-core/1.6.1/pom) +[![Kotlin](https://img.shields.io/badge/kotlin-1.6.1-blue.svg?logo=kotlin)](http://kotlinlang.org) [![Slack channel](https://img.shields.io/badge/chat-slack-green.svg?logo=slack)](https://kotlinlang.slack.com/messages/coroutines/) Library support for Kotlin coroutines with [multiplatform](#multiplatform) support. -This is a companion version for the Kotlin `1.6.0` release. +This is a companion version for the Kotlin `1.6.1` release. ```kotlin suspend fun main() = coroutineScope { @@ -83,7 +83,7 @@ Add dependencies (you can also add other modules that you need): org.jetbrains.kotlinx kotlinx-coroutines-core - 1.6.0 + 1.6.1 ``` @@ -101,7 +101,7 @@ Add dependencies (you can also add other modules that you need): ```kotlin dependencies { - implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.0") + implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.1") } ``` @@ -131,7 +131,7 @@ Add [`kotlinx-coroutines-android`](ui/kotlinx-coroutines-android) module as a dependency when using `kotlinx.coroutines` on Android: ```kotlin -implementation("org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.0") +implementation("org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.1") ``` This gives you access to the Android [Dispatchers.Main] @@ -166,7 +166,7 @@ In common code that should get compiled for different platforms, you can add a d ```kotlin commonMain { dependencies { - implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.0") + implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.1") } } ``` @@ -178,7 +178,7 @@ Platform-specific dependencies are recommended to be used only for non-multiplat #### JS Kotlin/JS version of `kotlinx.coroutines` is published as -[`kotlinx-coroutines-core-js`](https://search.maven.org/artifact/org.jetbrains.kotlinx/kotlinx-coroutines-core-js/1.6.0/jar) +[`kotlinx-coroutines-core-js`](https://search.maven.org/artifact/org.jetbrains.kotlinx/kotlinx-coroutines-core-js/1.6.1/jar) (follow the link to get the dependency declaration snippet) and as [`kotlinx-coroutines-core`](https://www.npmjs.com/package/kotlinx-coroutines-core) NPM package. #### Native diff --git a/gradle.properties b/gradle.properties index e06f587df7..e91e8dadaa 100644 --- a/gradle.properties +++ b/gradle.properties @@ -3,7 +3,7 @@ # # Kotlin -version=1.6.0-SNAPSHOT +version=1.6.1-SNAPSHOT group=org.jetbrains.kotlinx kotlin_version=1.6.0 diff --git a/kotlinx-coroutines-debug/README.md b/kotlinx-coroutines-debug/README.md index 5504b31f19..78a217e692 100644 --- a/kotlinx-coroutines-debug/README.md +++ b/kotlinx-coroutines-debug/README.md @@ -61,7 +61,7 @@ stacktraces will be dumped to the console. ### Using as JVM agent Debug module can also be used as a standalone JVM agent to enable debug probes on the application startup. -You can run your application with an additional argument: `-javaagent:kotlinx-coroutines-debug-1.6.0.jar`. +You can run your application with an additional argument: `-javaagent:kotlinx-coroutines-debug-1.6.1.jar`. Additionally, on Linux and Mac OS X you can use `kill -5 $pid` command in order to force your application to print all alive coroutines. When used as Java agent, `"kotlinx.coroutines.debug.enable.creation.stack.trace"` system property can be used to control [DebugProbes.enableCreationStackTraces] along with agent startup. diff --git a/kotlinx-coroutines-test/README.md b/kotlinx-coroutines-test/README.md index 7b6cf7308c..24d2f4e207 100644 --- a/kotlinx-coroutines-test/README.md +++ b/kotlinx-coroutines-test/README.md @@ -26,7 +26,7 @@ Provided [TestDispatcher] implementations: Add `kotlinx-coroutines-test` to your project test dependencies: ``` dependencies { - testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.6.0' + testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.6.1' } ``` diff --git a/ui/coroutines-guide-ui.md b/ui/coroutines-guide-ui.md index b7e8737868..fd4fcf0f72 100644 --- a/ui/coroutines-guide-ui.md +++ b/ui/coroutines-guide-ui.md @@ -110,7 +110,7 @@ Add dependencies on `kotlinx-coroutines-android` module to the `dependencies { . `app/build.gradle` file: ```groovy -implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.0" +implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.1" ``` You can clone [kotlinx.coroutines](https://github.com/Kotlin/kotlinx.coroutines) project from GitHub onto your