From 8773a2691dd638cbfb097283ce682d4ec40ecfc9 Mon Sep 17 00:00:00 2001 From: Roman Elizarov Date: Mon, 12 Oct 2020 19:09:48 +0300 Subject: [PATCH] Breaking: Get rid of atomic cancellation and provide a replacement (#1937) This is a problematic for Android when Main dispatcher is cancelled on destroyed activity. Atomic nature of channels is designed to prevent loss of elements, which is really not an issue for a typical application, but creates problem when used with channels. * Internal suspendAtomicCancellableCoroutine -> suspendCancellableCoroutine * Internal suspendAtomicCancellableCoroutineReusable -> suspendCancellableCoroutineReusable * Remove atomic cancellation from docs * Ensures that flowOn does not resume downstream after cancellation. * MODE_ATOMIC_DEFAULT renamed into MODE_ATOMIC * Introduced MODE_CANCELLABLE_REUSABLE to track suspendCancellableCoroutineReusable * Better documentation for MODE_XXX constants. * Added stress test for proper handling of MODE_CANCELLABLE_REUSABLE and fixed test for #1123 bug with job.join (working in MODE_CANCELLABLE) that was not properly failing in the absence of the proper code in CancellableContinuationImpl.getResult * Added test for Flow.combine that should be fixed * Support extended invokeOnCancellation contract * Introduced internal tryResumeAtomic * Channel onUnderliveredElement is introduced as a replacement. Fixes #1265 Fixes #1813 Fixes #1915 Fixes #1936 Co-authored-by: Louis CAD Co-authored-by: Vsevolod Tolstopyatov --- .../benchmarks/tailcall/SimpleChannel.kt | 8 +- .../api/kotlinx-coroutines-core.api | 15 +- kotlinx-coroutines-core/common/src/Await.kt | 15 +- .../common/src/Builders.common.kt | 5 +- .../common/src/CancellableContinuation.kt | 189 ++++++++--- .../common/src/CancellableContinuationImpl.kt | 299 ++++++++++++------ ...tedExceptionally.kt => CompletionState.kt} | 18 +- .../common/src/CoroutineDispatcher.kt | 1 + .../common/src/Deferred.kt | 2 + kotlinx-coroutines-core/common/src/Delay.kt | 9 + kotlinx-coroutines-core/common/src/Yield.kt | 3 + .../common/src/channels/AbstractChannel.kt | 153 ++++++--- .../src/channels/ArrayBroadcastChannel.kt | 6 +- .../common/src/channels/ArrayChannel.kt | 29 +- .../common/src/channels/Channel.kt | 143 ++++++--- .../common/src/channels/Channels.common.kt | 9 +- .../src/channels/ConflatedBroadcastChannel.kt | 2 +- .../common/src/channels/ConflatedChannel.kt | 27 +- .../common/src/channels/LinkedListChannel.kt | 2 +- .../common/src/channels/Produce.kt | 6 +- .../common/src/channels/RendezvousChannel.kt | 4 +- .../common/src/flow/Builders.kt | 9 +- .../common/src/flow/Channels.kt | 3 + .../common/src/flow/operators/Context.kt | 6 +- .../common/src/internal/Atomic.kt | 7 +- .../src/internal/DispatchedContinuation.kt | 57 ++-- .../common/src/internal/DispatchedTask.kt | 99 ++++-- .../src/internal/LockFreeLinkedList.common.kt | 1 + .../src/internal/OnUndeliveredElement.kt | 43 +++ .../common/src/intrinsics/Cancellable.kt | 8 +- .../common/src/selects/Select.kt | 10 +- .../common/src/sync/Mutex.kt | 37 +-- .../common/src/sync/Semaphore.kt | 25 +- .../test/AtomicCancellationCommonTest.kt | 13 +- .../CancellableContinuationHandlersTest.kt | 72 ++++- .../test/CancellableContinuationTest.kt | 22 ++ .../common/test/CancellableResumeTest.kt | 156 ++++++++- .../common/test/DispatchedContinuationTest.kt | 78 +++++ .../test/channels/BasicOperationsTest.kt | 20 +- .../common/test/channels/BroadcastTest.kt | 4 +- .../ChannelUndeliveredElementFailureTest.kt | 143 +++++++++ .../channels/ChannelUndeliveredElementTest.kt | 104 ++++++ .../test/channels/ConflatedChannelTest.kt | 2 +- .../common/test/channels/TestChannelKind.kt | 14 +- .../common/test/flow/operators/CatchTest.kt | 7 +- .../common/test/flow/operators/CombineTest.kt | 18 +- .../common/test/flow/operators/FlowOnTest.kt | 15 + .../common/test/flow/operators/ZipTest.kt | 25 +- .../common/test/selects/SelectLoopTest.kt | 23 +- kotlinx-coroutines-core/js/src/Promise.kt | 2 + .../js/src/internal/LinkedList.kt | 2 + .../jvm/src/DebugStrings.kt | 1 + .../jvm/src/internal/LockFreeLinkedList.kt | 19 +- .../jvm/test/AtomicCancellationTest.kt | 27 +- .../jvm/test/JobStructuredJoinStressTest.kt | 41 ++- .../ReusableCancellableContinuationTest.kt | 52 +-- kotlinx-coroutines-core/jvm/test/TestBase.kt | 2 + .../BroadcastChannelMultiReceiveStressTest.kt | 13 +- .../channels/ChannelAtomicCancelStressTest.kt | 156 --------- ...annelCancelUndeliveredElementStressTest.kt | 102 ++++++ .../channels/ChannelSendReceiveStressTest.kt | 2 +- .../ChannelUndeliveredElementStressTest.kt | 255 +++++++++++++++ .../test/channels/InvokeOnCloseStressTest.kt | 2 +- .../test/channels/SimpleSendReceiveJvmTest.kt | 2 +- .../jvm/test/sync/MutexStressTest.kt | 64 ++++ .../jvm/test/sync/SemaphoreStressTest.kt | 8 +- .../native/src/internal/LinkedList.kt | 2 + .../test/CoroutinesDumpTest.kt | 26 +- .../test/DebugProbesTest.kt | 27 +- .../src/Channel.kt | 2 +- .../kotlinx-coroutines-rx2/src/RxChannel.kt | 2 +- .../kotlinx-coroutines-rx3/src/RxChannel.kt | 2 +- 72 files changed, 2098 insertions(+), 679 deletions(-) rename kotlinx-coroutines-core/common/src/{CompletedExceptionally.kt => CompletionState.kt} (78%) create mode 100644 kotlinx-coroutines-core/common/src/internal/OnUndeliveredElement.kt create mode 100644 kotlinx-coroutines-core/common/test/DispatchedContinuationTest.kt create mode 100644 kotlinx-coroutines-core/common/test/channels/ChannelUndeliveredElementFailureTest.kt create mode 100644 kotlinx-coroutines-core/common/test/channels/ChannelUndeliveredElementTest.kt delete mode 100644 kotlinx-coroutines-core/jvm/test/channels/ChannelAtomicCancelStressTest.kt create mode 100644 kotlinx-coroutines-core/jvm/test/channels/ChannelCancelUndeliveredElementStressTest.kt create mode 100644 kotlinx-coroutines-core/jvm/test/channels/ChannelUndeliveredElementStressTest.kt diff --git a/benchmarks/src/jmh/kotlin/benchmarks/tailcall/SimpleChannel.kt b/benchmarks/src/jmh/kotlin/benchmarks/tailcall/SimpleChannel.kt index c217fcae91..d961dab8d9 100644 --- a/benchmarks/src/jmh/kotlin/benchmarks/tailcall/SimpleChannel.kt +++ b/benchmarks/src/jmh/kotlin/benchmarks/tailcall/SimpleChannel.kt @@ -70,12 +70,12 @@ class NonCancellableChannel : SimpleChannel() { } class CancellableChannel : SimpleChannel() { - override suspend fun suspendReceive(): Int = suspendAtomicCancellableCoroutine { + override suspend fun suspendReceive(): Int = suspendCancellableCoroutine { consumer = it.intercepted() COROUTINE_SUSPENDED } - override suspend fun suspendSend(element: Int) = suspendAtomicCancellableCoroutine { + override suspend fun suspendSend(element: Int) = suspendCancellableCoroutine { enqueuedValue = element producer = it.intercepted() COROUTINE_SUSPENDED @@ -84,13 +84,13 @@ class CancellableChannel : SimpleChannel() { class CancellableReusableChannel : SimpleChannel() { @Suppress("INVISIBLE_MEMBER") - override suspend fun suspendReceive(): Int = suspendAtomicCancellableCoroutineReusable { + override suspend fun suspendReceive(): Int = suspendCancellableCoroutineReusable { consumer = it.intercepted() COROUTINE_SUSPENDED } @Suppress("INVISIBLE_MEMBER") - override suspend fun suspendSend(element: Int) = suspendAtomicCancellableCoroutineReusable { + override suspend fun suspendSend(element: Int) = suspendCancellableCoroutineReusable { enqueuedValue = element producer = it.intercepted() COROUTINE_SUSPENDED diff --git a/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api b/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api index 6611995361..4ccd340f52 100644 --- a/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api +++ b/kotlinx-coroutines-core/api/kotlinx-coroutines-core.api @@ -46,6 +46,7 @@ public abstract interface class kotlinx/coroutines/CancellableContinuation : kot public abstract fun resumeUndispatched (Lkotlinx/coroutines/CoroutineDispatcher;Ljava/lang/Object;)V public abstract fun resumeUndispatchedWithException (Lkotlinx/coroutines/CoroutineDispatcher;Ljava/lang/Throwable;)V public abstract fun tryResume (Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object; + public abstract fun tryResume (Ljava/lang/Object;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; public abstract fun tryResumeWithException (Ljava/lang/Throwable;)Ljava/lang/Object; } @@ -56,6 +57,8 @@ public final class kotlinx/coroutines/CancellableContinuation$DefaultImpls { public class kotlinx/coroutines/CancellableContinuationImpl : kotlin/coroutines/jvm/internal/CoroutineStackFrame, kotlinx/coroutines/CancellableContinuation { public fun (Lkotlin/coroutines/Continuation;I)V + public final fun callCancelHandler (Lkotlinx/coroutines/CancelHandler;Ljava/lang/Throwable;)V + public final fun callOnCancellation (Lkotlin/jvm/functions/Function1;Ljava/lang/Throwable;)V public fun cancel (Ljava/lang/Throwable;)Z public fun completeResume (Ljava/lang/Object;)V public fun getCallerFrame ()Lkotlin/coroutines/jvm/internal/CoroutineStackFrame; @@ -75,14 +78,12 @@ public class kotlinx/coroutines/CancellableContinuationImpl : kotlin/coroutines/ public fun resumeWith (Ljava/lang/Object;)V public fun toString ()Ljava/lang/String; public fun tryResume (Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object; + public fun tryResume (Ljava/lang/Object;Ljava/lang/Object;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; public fun tryResumeWithException (Ljava/lang/Throwable;)Ljava/lang/Object; } public final class kotlinx/coroutines/CancellableContinuationKt { public static final fun disposeOnCancellation (Lkotlinx/coroutines/CancellableContinuation;Lkotlinx/coroutines/DisposableHandle;)V - public static final fun suspendAtomicCancellableCoroutine (Lkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; - public static final fun suspendAtomicCancellableCoroutine (ZLkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; - public static synthetic fun suspendAtomicCancellableCoroutine$default (ZLkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object; public static final fun suspendCancellableCoroutine (Lkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; } @@ -272,10 +273,6 @@ public final class kotlinx/coroutines/DelayKt { public static final fun delay-p9JZ4hM (DLkotlin/coroutines/Continuation;)Ljava/lang/Object; } -public final class kotlinx/coroutines/DispatchedContinuationKt { - public static final fun resumeCancellableWith (Lkotlin/coroutines/Continuation;Ljava/lang/Object;)V -} - public final class kotlinx/coroutines/Dispatchers { public static final field INSTANCE Lkotlinx/coroutines/Dispatchers; public static final fun getDefault ()Lkotlinx/coroutines/CoroutineDispatcher; @@ -613,8 +610,10 @@ public final class kotlinx/coroutines/channels/ChannelIterator$DefaultImpls { } public final class kotlinx/coroutines/channels/ChannelKt { - public static final fun Channel (I)Lkotlinx/coroutines/channels/Channel; + public static final synthetic fun Channel (I)Lkotlinx/coroutines/channels/Channel; + public static final fun Channel (ILkotlin/jvm/functions/Function1;)Lkotlinx/coroutines/channels/Channel; public static synthetic fun Channel$default (IILjava/lang/Object;)Lkotlinx/coroutines/channels/Channel; + public static synthetic fun Channel$default (ILkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lkotlinx/coroutines/channels/Channel; } public final class kotlinx/coroutines/channels/ChannelsKt { diff --git a/kotlinx-coroutines-core/common/src/Await.kt b/kotlinx-coroutines-core/common/src/Await.kt index dd1e1771f2..7189349024 100644 --- a/kotlinx-coroutines-core/common/src/Await.kt +++ b/kotlinx-coroutines-core/common/src/Await.kt @@ -5,6 +5,7 @@ package kotlinx.coroutines import kotlinx.atomicfu.* +import kotlinx.coroutines.channels.* import kotlin.coroutines.* /** @@ -18,6 +19,8 @@ import kotlin.coroutines.* * This suspending function is cancellable. * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, * this function immediately resumes with [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. */ public suspend fun awaitAll(vararg deferreds: Deferred): List = if (deferreds.isEmpty()) emptyList() else AwaitAll(deferreds).await() @@ -33,6 +36,8 @@ public suspend fun awaitAll(vararg deferreds: Deferred): List = * This suspending function is cancellable. * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, * this function immediately resumes with [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. */ public suspend fun Collection>.awaitAll(): List = if (isEmpty()) emptyList() else AwaitAll(toTypedArray()).await() @@ -41,8 +46,11 @@ public suspend fun Collection>.awaitAll(): List = * Suspends current coroutine until all given jobs are complete. * This method is semantically equivalent to joining all given jobs one by one with `jobs.forEach { it.join() }`. * - * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, + * This suspending function is cancellable. + * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, * this function immediately resumes with [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. */ public suspend fun joinAll(vararg jobs: Job): Unit = jobs.forEach { it.join() } @@ -50,8 +58,11 @@ public suspend fun joinAll(vararg jobs: Job): Unit = jobs.forEach { it.join() } * Suspends current coroutine until all given jobs are complete. * This method is semantically equivalent to joining all given jobs one by one with `forEach { it.join() }`. * - * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, + * This suspending function is cancellable. + * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, * this function immediately resumes with [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. */ public suspend fun Collection.joinAll(): Unit = forEach { it.join() } diff --git a/kotlinx-coroutines-core/common/src/Builders.common.kt b/kotlinx-coroutines-core/common/src/Builders.common.kt index 64bff500dc..c0924a0238 100644 --- a/kotlinx-coroutines-core/common/src/Builders.common.kt +++ b/kotlinx-coroutines-core/common/src/Builders.common.kt @@ -129,8 +129,9 @@ private class LazyDeferredCoroutine( * This function uses dispatcher from the new context, shifting execution of the [block] into the * different thread if a new dispatcher is specified, and back to the original dispatcher * when it completes. Note that the result of `withContext` invocation is - * dispatched into the original context in a cancellable way, which means that if the original [coroutineContext], - * in which `withContext` was invoked, is cancelled by the time its dispatcher starts to execute the code, + * dispatched into the original context in a cancellable way with a **prompt cancellation guarantee**, + * which means that if the original [coroutineContext], in which `withContext` was invoked, + * is cancelled by the time its dispatcher starts to execute the code, * it discards the result of `withContext` and throws [CancellationException]. */ public suspend fun withContext( diff --git a/kotlinx-coroutines-core/common/src/CancellableContinuation.kt b/kotlinx-coroutines-core/common/src/CancellableContinuation.kt index f5b511cb9c..7d9315afbf 100644 --- a/kotlinx-coroutines-core/common/src/CancellableContinuation.kt +++ b/kotlinx-coroutines-core/common/src/CancellableContinuation.kt @@ -15,6 +15,8 @@ import kotlin.coroutines.intrinsics.* * When the [cancel] function is explicitly invoked, this continuation immediately resumes with a [CancellationException] or * the specified cancel cause. * + * An instance of `CancellableContinuation` is created by the [suspendCancellableCoroutine] function. + * * Cancellable continuation has three states (as subset of [Job] states): * * | **State** | [isActive] | [isCompleted] | [isCancelled] | @@ -24,11 +26,11 @@ import kotlin.coroutines.intrinsics.* * | _Canceled_ (final _completed_ state)| `false` | `true` | `true` | * * Invocation of [cancel] transitions this continuation from _active_ to _cancelled_ state, while - * invocation of [resume] or [resumeWithException] transitions it from _active_ to _resumed_ state. + * invocation of [Continuation.resume] or [Continuation.resumeWithException] transitions it from _active_ to _resumed_ state. * * A [cancelled][isCancelled] continuation implies that it is [completed][isCompleted]. * - * Invocation of [resume] or [resumeWithException] in _resumed_ state produces an [IllegalStateException], + * Invocation of [Continuation.resume] or [Continuation.resumeWithException] in _resumed_ state produces an [IllegalStateException], * but is ignored in _cancelled_ state. * * ``` @@ -41,7 +43,6 @@ import kotlin.coroutines.intrinsics.* * +-----------+ * | Cancelled | * +-----------+ - * * ``` */ public interface CancellableContinuation : Continuation { @@ -76,6 +77,14 @@ public interface CancellableContinuation : Continuation { @InternalCoroutinesApi public fun tryResume(value: T, idempotent: Any? = null): Any? + /** + * Same as [tryResume] but with [onCancellation] handler that called if and only if the value is not + * delivered to the caller because of the dispatch in the process, so that atomicity delivery + * guaranteed can be provided by having a cancellation fallback. + */ + @InternalCoroutinesApi + public fun tryResume(value: T, idempotent: Any?, onCancellation: ((cause: Throwable) -> Unit)?): Any? + /** * Tries to resume this continuation with the specified [exception] and returns a non-null object token if successful, * or `null` otherwise (it was already resumed or cancelled). When a non-null object is returned, @@ -110,8 +119,8 @@ public interface CancellableContinuation : Continuation { public fun cancel(cause: Throwable? = null): Boolean /** - * Registers a [handler] to be **synchronously** invoked on cancellation (regular or exceptional) of this continuation. - * When the continuation is already cancelled, the handler will be immediately invoked + * Registers a [handler] to be **synchronously** invoked on [cancellation][cancel] (regular or exceptional) of this continuation. + * When the continuation is already cancelled, the handler is immediately invoked * with the cancellation exception. Otherwise, the handler will be invoked as soon as this * continuation is cancelled. * @@ -120,7 +129,15 @@ public interface CancellableContinuation : Continuation { * processed as an uncaught exception in the context of the current coroutine * (see [CoroutineExceptionHandler]). * - * At most one [handler] can be installed on a continuation. + * At most one [handler] can be installed on a continuation. Attempt to call `invokeOnCancellation` second + * time produces [IllegalStateException]. + * + * This handler is also called when this continuation [resumes][Continuation.resume] normally (with a value) and then + * is cancelled while waiting to be dispatched. More generally speaking, this handler is called whenever + * the caller of [suspendCancellableCoroutine] is getting a [CancellationException]. + * + * A typical example for `invokeOnCancellation` usage is given in + * the documentation for the [suspendCancellableCoroutine] function. * * **Note**: Implementation of `CompletionHandler` must be fast, non-blocking, and thread-safe. * This `handler` can be invoked concurrently with the surrounding code. @@ -163,7 +180,7 @@ public interface CancellableContinuation : Continuation { * (see [CoroutineExceptionHandler]). * * This function shall be used when resuming with a resource that must be closed by - * code that called the corresponding suspending function, e.g.: + * code that called the corresponding suspending function, for example: * * ``` * continuation.resume(resource) { @@ -171,17 +188,119 @@ public interface CancellableContinuation : Continuation { * } * ``` * + * A more complete example and further details are given in + * the documentation for the [suspendCancellableCoroutine] function. + * * **Note**: The [onCancellation] handler must be fast, non-blocking, and thread-safe. * It can be invoked concurrently with the surrounding code. * There is no guarantee on the execution context of its invocation. */ @ExperimentalCoroutinesApi // since 1.2.0 - public fun resume(value: T, onCancellation: (cause: Throwable) -> Unit) + public fun resume(value: T, onCancellation: ((cause: Throwable) -> Unit)?) } /** * Suspends the coroutine like [suspendCoroutine], but providing a [CancellableContinuation] to - * the [block]. This function throws a [CancellationException] if the coroutine is cancelled or completed while suspended. + * the [block]. This function throws a [CancellationException] if the [Job] of the coroutine is + * cancelled or completed while it is suspended. + * + * A typical use of this function is to suspend a coroutine while waiting for a result + * from a single-shot callback API and to return the result to the caller. + * For multi-shot callback APIs see [callbackFlow][kotlinx.coroutines.flow.callbackFlow]. + * + * ``` + * suspend fun awaitCallback(): T = suspendCancellableCoroutine { continuation -> + * val callback = object : Callback { // Implementation of some callback interface + * override fun onCompleted(value: T) { + * // Resume coroutine with a value provided by the callback + * continuation.resume(value) + * } + * override fun onApiError(cause: Throwable) { + * // Resume coroutine with an exception provided by the callback + * continuation.resumeWithException(cause) + * } + * } + * // Register callback with an API + * api.register(callback) + * // Remove callback on cancellation + * continuation.invokeOnCancellation { api.unregister(callback) } + * // At this point the coroutine is suspended by suspendCancellableCoroutine until callback fires + * } + * ``` + * + * > The callback `register`/`unregister` methods provided by an external API must be thread-safe, because + * > `invokeOnCancellation` block can be called at any time due to asynchronous nature of cancellation, even + * > concurrently with the call of the callback. + * + * ### Prompt cancellation guarantee + * + * This function provides **prompt cancellation guarantee**. + * If the [Job] of the current coroutine was cancelled while this function was suspended it will not resume + * successfully. + * + * The cancellation of the coroutine's job is generally asynchronous with respect to the suspended coroutine. + * The suspended coroutine is resumed with the call it to its [Continuation.resumeWith] member function or to + * [resume][Continuation.resume] extension function. + * However, when coroutine is resumed, it does not immediately start executing, but is passed to its + * [CoroutineDispatcher] to schedule its execution when dispatcher's resources become available for execution. + * The job's cancellation can happen both before, after, and concurrently with the call to `resume`. In any + * case, prompt cancellation guarantees that the the coroutine will not resume its code successfully. + * + * If the coroutine was resumed with an exception (for example, using [Continuation.resumeWithException] extension + * function) and cancelled, then the resulting exception of the `suspendCancellableCoroutine` function is determined + * by whichever action (exceptional resume or cancellation) that happened first. + * + * ### Returning resources from a suspended coroutine + * + * As a result of a prompt cancellation guarantee, when a closeable resource + * (like open file or a handle to another native resource) is returned from a suspended coroutine as a value + * it can be lost when the coroutine is cancelled. In order to ensure that the resource can be properly closed + * in this case, the [CancellableContinuation] interface provides two functions. + * + * * [invokeOnCancellation][CancellableContinuation.invokeOnCancellation] installs a handler that is called + * whenever a suspend coroutine is being cancelled. In addition to the example at the beginning, it can be + * used to ensure that a resource that was opened before the call to + * `suspendCancellableCoroutine` or in its body is closed in case of cancellation. + * + * ``` + * suspendCancellableCoroutine { continuation -> + * val resource = openResource() // Opens some resource + * continuation.invokeOnCancellation { + * resource.close() // Ensures the resource is closed on cancellation + * } + * // ... + * } + * ``` + * + * * [resume(value) { ... }][CancellableContinuation.resume] method on a [CancellableContinuation] takes + * an optional `onCancellation` block. It can be used when resuming with a resource that must be closed by + * the code that called the corresponding suspending function. + * + * ``` + * suspendCancellableCoroutine { continuation -> + * val callback = object : Callback { // Implementation of some callback interface + * // A callback provides a reference to some closeable resource + * override fun onCompleted(resource: T) { + * // Resume coroutine with a value provided by the callback and ensure the resource is closed in case + * // when the coroutine is cancelled before the caller gets a reference to the resource. + * continuation.resume(resource) { + * resource.close() // Close the resource on cancellation + * } + * } + * // ... + * } + * ``` + * + * ### Implementation details and custom continuation interceptors + * + * The prompt cancellation guarantee is the result of a coordinated implementation inside `suspendCancellableCoroutine` + * function and the [CoroutineDispatcher] class. The coroutine dispatcher checks for the status of the [Job] immediately + * before continuing its normal execution and aborts this normal execution, calling all the corresponding + * cancellation handlers, if the job was cancelled. + * + * If a custom implementation of [ContinuationInterceptor] is used in a coroutine's context that does not extend + * [CoroutineDispatcher] class, then there is no prompt cancellation guarantee. A custom continuation interceptor + * can resume execution of a previously suspended coroutine even if its job was already cancelled. */ public suspend inline fun suspendCancellableCoroutine( crossinline block: (CancellableContinuation) -> Unit @@ -199,29 +318,10 @@ public suspend inline fun suspendCancellableCoroutine( } /** - * Suspends the coroutine like [suspendCancellableCoroutine], but with *atomic cancellation*. - * - * When the suspended function throws a [CancellationException], it means that the continuation was not resumed. - * As a side-effect of atomic cancellation, a thread-bound coroutine (to some UI thread, for example) may - * continue to execute even after it was cancelled from the same thread in the case when the continuation - * was already resumed and was posted for execution to the thread's queue. - * - * @suppress **This an internal API and should not be used from general code.** + * Suspends the coroutine similar to [suspendCancellableCoroutine], but an instance of + * [CancellableContinuationImpl] is reused. */ -@InternalCoroutinesApi -public suspend inline fun suspendAtomicCancellableCoroutine( - crossinline block: (CancellableContinuation) -> Unit -): T = - suspendCoroutineUninterceptedOrReturn { uCont -> - val cancellable = CancellableContinuationImpl(uCont.intercepted(), resumeMode = MODE_ATOMIC_DEFAULT) - block(cancellable) - cancellable.getResult() - } - -/** - * Suspends coroutine similar to [suspendAtomicCancellableCoroutine], but an instance of [CancellableContinuationImpl] is reused if possible. - */ -internal suspend inline fun suspendAtomicCancellableCoroutineReusable( +internal suspend inline fun suspendCancellableCoroutineReusable( crossinline block: (CancellableContinuation) -> Unit ): T = suspendCoroutineUninterceptedOrReturn { uCont -> val cancellable = getOrCreateCancellableContinuation(uCont.intercepted()) @@ -232,12 +332,12 @@ internal suspend inline fun suspendAtomicCancellableCoroutineReusable( internal fun getOrCreateCancellableContinuation(delegate: Continuation): CancellableContinuationImpl { // If used outside of our dispatcher if (delegate !is DispatchedContinuation) { - return CancellableContinuationImpl(delegate, resumeMode = MODE_ATOMIC_DEFAULT) + return CancellableContinuationImpl(delegate, MODE_CANCELLABLE_REUSABLE) } /* * Attempt to claim reusable instance. * - * suspendAtomicCancellableCoroutineReusable { // <- claimed + * suspendCancellableCoroutineReusable { // <- claimed * // Any asynchronous cancellation is "postponed" while this block * // is being executed * } // postponed cancellation is checked here. @@ -248,26 +348,13 @@ internal fun getOrCreateCancellableContinuation(delegate: Continuation): * thus leaking CC instance for indefinite time. * 2) Continuation was cancelled. Then we should prevent any further reuse and bail out. */ - return delegate.claimReusableCancellableContinuation()?.takeIf { it.resetState() } - ?: return CancellableContinuationImpl(delegate, MODE_ATOMIC_DEFAULT) + return delegate.claimReusableCancellableContinuation()?.takeIf { it.resetStateReusable() } + ?: return CancellableContinuationImpl(delegate, MODE_CANCELLABLE_REUSABLE) } /** - * @suppress **Deprecated** - */ -@Deprecated( - message = "holdCancellability parameter is deprecated and is no longer used", - replaceWith = ReplaceWith("suspendAtomicCancellableCoroutine(block)") -) -@InternalCoroutinesApi -public suspend inline fun suspendAtomicCancellableCoroutine( - holdCancellability: Boolean = false, - crossinline block: (CancellableContinuation) -> Unit -): T = - suspendAtomicCancellableCoroutine(block) - -/** - * Removes the specified [node] on cancellation. + * Removes the specified [node] on cancellation. This function assumes that this node is already + * removed on successful resume and does not try to remove it if the continuation is cancelled during dispatch. */ internal fun CancellableContinuation<*>.removeOnCancellation(node: LockFreeLinkedListNode) = invokeOnCancellation(handler = RemoveOnCancel(node).asHandler) @@ -288,7 +375,7 @@ public fun CancellableContinuation<*>.disposeOnCancellation(handle: DisposableHa // --------------- implementation details --------------- -private class RemoveOnCancel(private val node: LockFreeLinkedListNode) : CancelHandler() { +private class RemoveOnCancel(private val node: LockFreeLinkedListNode) : BeforeResumeCancelHandler() { override fun invoke(cause: Throwable?) { node.remove() } override fun toString() = "RemoveOnCancel[$node]" } diff --git a/kotlinx-coroutines-core/common/src/CancellableContinuationImpl.kt b/kotlinx-coroutines-core/common/src/CancellableContinuationImpl.kt index 166cb3b353..cdb1b78882 100644 --- a/kotlinx-coroutines-core/common/src/CancellableContinuationImpl.kt +++ b/kotlinx-coroutines-core/common/src/CancellableContinuationImpl.kt @@ -27,6 +27,10 @@ internal open class CancellableContinuationImpl( final override val delegate: Continuation, resumeMode: Int ) : DispatchedTask(resumeMode), CancellableContinuation, CoroutineStackFrame { + init { + assert { resumeMode != MODE_UNINITIALIZED } // invalid mode for CancellableContinuationImpl + } + public override val context: CoroutineContext = delegate.context /* @@ -88,15 +92,17 @@ internal open class CancellableContinuationImpl( private fun isReusable(): Boolean = delegate is DispatchedContinuation<*> && delegate.isReusable(this) /** - * Resets cancellability state in order to [suspendAtomicCancellableCoroutineReusable] to work. - * Invariant: used only by [suspendAtomicCancellableCoroutineReusable] in [REUSABLE_CLAIMED] state. + * Resets cancellability state in order to [suspendCancellableCoroutineReusable] to work. + * Invariant: used only by [suspendCancellableCoroutineReusable] in [REUSABLE_CLAIMED] state. */ - @JvmName("resetState") // Prettier stack traces - internal fun resetState(): Boolean { + @JvmName("resetStateReusable") // Prettier stack traces + internal fun resetStateReusable(): Boolean { + assert { resumeMode == MODE_CANCELLABLE_REUSABLE } // invalid mode for CancellableContinuationImpl assert { parentHandle !== NonDisposableHandle } val state = _state.value assert { state !is NotCompleted } - if (state is CompletedIdempotentResult) { + if (state is CompletedContinuation && state.idempotentResume != null) { + // Cannot reuse continuation that was resumed with idempotent marker detachChild() return false } @@ -129,7 +135,7 @@ internal open class CancellableContinuationImpl( private fun checkCompleted(): Boolean { val completed = isCompleted - if (resumeMode != MODE_ATOMIC_DEFAULT) return completed // Do not check postponed cancellation for non-reusable continuations + if (!resumeMode.isReusableMode) return completed // Do not check postponed cancellation for non-reusable continuations val dispatched = delegate as? DispatchedContinuation<*> ?: return completed val cause = dispatched.checkPostponedCancellation(this) ?: return completed if (!completed) { @@ -146,10 +152,26 @@ internal open class CancellableContinuationImpl( override fun takeState(): Any? = state - override fun cancelResult(state: Any?, cause: Throwable) { - if (state is CompletedWithCancellation) { - invokeHandlerSafely { - state.onCancellation(cause) + // Note: takeState does not clear the state so we don't use takenState + // and we use the actual current state where in CAS-loop + override fun cancelCompletedResult(takenState: Any?, cause: Throwable): Unit = _state.loop { state -> + when (state) { + is NotCompleted -> error("Not completed") + is CompletedExceptionally -> return // already completed exception or cancelled, nothing to do + is CompletedContinuation -> { + check(!state.cancelled) { "Must be called at most once" } + val update = state.copy(cancelCause = cause) + if (_state.compareAndSet(state, update)) { + state.invokeHandlers(this, cause) + return // done + } + } + else -> { + // completed normally without marker class, promote to CompletedContinuation in case + // if invokeOnCancellation if called later + if (_state.compareAndSet(state, CompletedContinuation(state, cancelCause = cause))) { + return // done + } } } } @@ -158,7 +180,7 @@ internal open class CancellableContinuationImpl( * Attempt to postpone cancellation for reusable cancellable continuation */ private fun cancelLater(cause: Throwable): Boolean { - if (resumeMode != MODE_ATOMIC_DEFAULT) return false + if (!resumeMode.isReusableMode) return false val dispatched = (delegate as? DispatchedContinuation<*>) ?: return false return dispatched.postponeCancellation(cause) } @@ -170,10 +192,10 @@ internal open class CancellableContinuationImpl( val update = CancelledContinuation(this, cause, handled = state is CancelHandler) if (!_state.compareAndSet(state, update)) return@loop // retry on cas failure // Invoke cancel handler if it was present - if (state is CancelHandler) invokeHandlerSafely { state.invoke(cause) } + (state as? CancelHandler)?.let { callCancelHandler(it, cause) } // Complete state update detachChildIfNonResuable() - dispatchResume(mode = MODE_ATOMIC_DEFAULT) + dispatchResume(resumeMode) // no need for additional cancellation checks return true } } @@ -185,14 +207,36 @@ internal open class CancellableContinuationImpl( detachChildIfNonResuable() } - private inline fun invokeHandlerSafely(block: () -> Unit) { + private inline fun callCancelHandlerSafely(block: () -> Unit) { + try { + block() + } catch (ex: Throwable) { + // Handler should never fail, if it does -- it is an unhandled exception + handleCoroutineException( + context, + CompletionHandlerException("Exception in invokeOnCancellation handler for $this", ex) + ) + } + } + + private fun callCancelHandler(handler: CompletionHandler, cause: Throwable?) = + /* + * :KLUDGE: We have to invoke a handler in platform-specific way via `invokeIt` extension, + * because we play type tricks on Kotlin/JS and handler is not necessarily a function there + */ + callCancelHandlerSafely { handler.invokeIt(cause) } + + fun callCancelHandler(handler: CancelHandler, cause: Throwable?) = + callCancelHandlerSafely { handler.invoke(cause) } + + fun callOnCancellation(onCancellation: (cause: Throwable) -> Unit, cause: Throwable) { try { - block() + onCancellation.invoke(cause) } catch (ex: Throwable) { // Handler should never fail, if it does -- it is an unhandled exception handleCoroutineException( context, - CompletionHandlerException("Exception in cancellation handler for $this", ex) + CompletionHandlerException("Exception in resume onCancellation handler for $this", ex) ) } } @@ -231,64 +275,75 @@ internal open class CancellableContinuationImpl( val state = this.state if (state is CompletedExceptionally) throw recoverStackTrace(state.cause, this) // if the parent job was already cancelled, then throw the corresponding cancellation exception - // otherwise, there is a race is suspendCancellableCoroutine { cont -> ... } does cont.resume(...) + // otherwise, there is a race if suspendCancellableCoroutine { cont -> ... } does cont.resume(...) // before the block returns. This getResult would return a result as opposed to cancellation // exception that should have happened if the continuation is dispatched for execution later. - if (resumeMode == MODE_CANCELLABLE) { + if (resumeMode.isCancellableMode) { val job = context[Job] if (job != null && !job.isActive) { val cause = job.getCancellationException() - cancelResult(state, cause) + cancelCompletedResult(state, cause) throw recoverStackTrace(cause, this) } } return getSuccessfulResult(state) } - override fun resumeWith(result: Result) { + override fun resumeWith(result: Result) = resumeImpl(result.toState(this), resumeMode) - } - override fun resume(value: T, onCancellation: (cause: Throwable) -> Unit) { - val cancelled = resumeImpl(CompletedWithCancellation(value, onCancellation), resumeMode) - if (cancelled != null) { - // too late to resume (was cancelled) -- call handler - invokeHandlerSafely { - onCancellation(cancelled.cause) - } - } - } + override fun resume(value: T, onCancellation: ((cause: Throwable) -> Unit)?) = + resumeImpl(value, resumeMode, onCancellation) public override fun invokeOnCancellation(handler: CompletionHandler) { - var handleCache: CancelHandler? = null + val cancelHandler = makeCancelHandler(handler) _state.loop { state -> when (state) { is Active -> { - val node = handleCache ?: makeHandler(handler).also { handleCache = it } - if (_state.compareAndSet(state, node)) return // quit on cas success + if (_state.compareAndSet(state, cancelHandler)) return // quit on cas success } is CancelHandler -> multipleHandlersError(handler, state) - is CancelledContinuation -> { + is CompletedExceptionally -> { /* - * Continuation was already cancelled, invoke directly. + * Continuation was already cancelled or completed exceptionally. * NOTE: multiple invokeOnCancellation calls with different handlers are not allowed, - * so we check to make sure that handler was installed just once. + * so we check to make sure handler was installed just once. */ if (!state.makeHandled()) multipleHandlersError(handler, state) /* + * Call the handler only if it was cancelled (not called when completed exceptionally). * :KLUDGE: We have to invoke a handler in platform-specific way via `invokeIt` extension, * because we play type tricks on Kotlin/JS and handler is not necessarily a function there */ - invokeHandlerSafely { handler.invokeIt((state as? CompletedExceptionally)?.cause) } + if (state is CancelledContinuation) { + callCancelHandler(handler, (state as? CompletedExceptionally)?.cause) + } return } + is CompletedContinuation -> { + /* + * Continuation was already completed, and might already have cancel handler. + */ + if (state.cancelHandler != null) multipleHandlersError(handler, state) + // BeforeResumeCancelHandler does not need to be called on a completed continuation + if (cancelHandler is BeforeResumeCancelHandler) return + if (state.cancelled) { + // Was already cancelled while being dispatched -- invoke the handler directly + callCancelHandler(handler, state.cancelCause) + return + } + val update = state.copy(cancelHandler = cancelHandler) + if (_state.compareAndSet(state, update)) return // quit on cas success + } else -> { /* - * Continuation was already completed, do nothing. - * NOTE: multiple invokeOnCancellation calls with different handlers are not allowed, - * but we have no way to check that it was installed just once in this case. + * Continuation was already completed normally, but might get cancelled while being dispatched. + * Change its state to CompletedContinuation, unless we have BeforeResumeCancelHandler which + * does not need to be called in this case. */ - return + if (cancelHandler is BeforeResumeCancelHandler) return + val update = CompletedContinuation(state, cancelHandler = cancelHandler) + if (_state.compareAndSet(state, update)) return // quit on cas success } } } @@ -298,7 +353,7 @@ internal open class CancellableContinuationImpl( error("It's prohibited to register multiple handlers, tried to register $handler, already has $state") } - private fun makeHandler(handler: CompletionHandler): CancelHandler = + private fun makeCancelHandler(handler: CompletionHandler): CancelHandler = if (handler is CancelHandler) handler else InvokeOnCancel(handler) private fun dispatchResume(mode: Int) { @@ -307,15 +362,39 @@ internal open class CancellableContinuationImpl( dispatch(mode) } - // returns null when successfully dispatched resumed, CancelledContinuation if too late (was already cancelled) - private fun resumeImpl(proposedUpdate: Any?, resumeMode: Int): CancelledContinuation? { + private fun resumedState( + state: NotCompleted, + proposedUpdate: Any?, + resumeMode: Int, + onCancellation: ((cause: Throwable) -> Unit)?, + idempotent: Any? + ): Any? = when { + proposedUpdate is CompletedExceptionally -> { + assert { idempotent == null } // there are no idempotent exceptional resumes + assert { onCancellation == null } // only successful results can be cancelled + proposedUpdate + } + !resumeMode.isCancellableMode && idempotent == null -> proposedUpdate // cannot be cancelled in process, all is fine + onCancellation != null || (state is CancelHandler && state !is BeforeResumeCancelHandler) || idempotent != null -> + // mark as CompletedContinuation if special cases are present: + // Cancellation handlers that shall be called after resume or idempotent resume + CompletedContinuation(proposedUpdate, state as? CancelHandler, onCancellation, idempotent) + else -> proposedUpdate // simple case -- use the value directly + } + + private fun resumeImpl( + proposedUpdate: Any?, + resumeMode: Int, + onCancellation: ((cause: Throwable) -> Unit)? = null + ) { _state.loop { state -> when (state) { is NotCompleted -> { - if (!_state.compareAndSet(state, proposedUpdate)) return@loop // retry on cas failure + val update = resumedState(state, proposedUpdate, resumeMode, onCancellation, idempotent = null) + if (!_state.compareAndSet(state, update)) return@loop // retry on cas failure detachChildIfNonResuable() - dispatchResume(resumeMode) - return null + dispatchResume(resumeMode) // dispatch resume, but it might get cancelled in process + return // done } is CancelledContinuation -> { /* @@ -323,14 +402,48 @@ internal open class CancellableContinuationImpl( * because cancellation is asynchronous and may race with resume. * Racy exceptions will be lost, too. */ - if (state.makeResumed()) return state // tried to resume just once, but was cancelled + if (state.makeResumed()) { // check if trying to resume one (otherwise error) + // call onCancellation + onCancellation?.let { callOnCancellation(it, state.cause) } + return // done + } + } + } + alreadyResumedError(proposedUpdate) // otherwise, an error (second resume attempt) + } + } + + /** + * Similar to [tryResume], but does not actually completes resume (needs [completeResume] call). + * Returns [RESUME_TOKEN] when resumed, `null` when it was already resumed or cancelled. + */ + private fun tryResumeImpl( + proposedUpdate: Any?, + idempotent: Any?, + onCancellation: ((cause: Throwable) -> Unit)? + ): Symbol? { + _state.loop { state -> + when (state) { + is NotCompleted -> { + val update = resumedState(state, proposedUpdate, resumeMode, onCancellation, idempotent) + if (!_state.compareAndSet(state, update)) return@loop // retry on cas failure + detachChildIfNonResuable() + return RESUME_TOKEN + } + is CompletedContinuation -> { + return if (idempotent != null && state.idempotentResume === idempotent) { + assert { state.result == proposedUpdate } // "Non-idempotent resume" + RESUME_TOKEN // resumed with the same token -- ok + } else { + null // resumed with a different token or non-idempotent -- too late + } } + else -> return null // cannot resume -- not active anymore } - alreadyResumedError(proposedUpdate) // otherwise -- an error (second resume attempt) } } - private fun alreadyResumedError(proposedUpdate: Any?) { + private fun alreadyResumedError(proposedUpdate: Any?): Nothing { error("Already resumed, but proposed with update $proposedUpdate") } @@ -342,7 +455,7 @@ internal open class CancellableContinuationImpl( /** * Detaches from the parent. - * Invariant: used used from [CoroutineDispatcher.releaseInterceptedContinuation] iff [isReusable] is `true` + * Invariant: used from [CoroutineDispatcher.releaseInterceptedContinuation] iff [isReusable] is `true` */ internal fun detachChild() { val handle = parentHandle @@ -351,42 +464,14 @@ internal open class CancellableContinuationImpl( } // Note: Always returns RESUME_TOKEN | null - override fun tryResume(value: T, idempotent: Any?): Any? { - _state.loop { state -> - when (state) { - is NotCompleted -> { - val update: Any? = if (idempotent == null) value else - CompletedIdempotentResult(idempotent, value) - if (!_state.compareAndSet(state, update)) return@loop // retry on cas failure - detachChildIfNonResuable() - return RESUME_TOKEN - } - is CompletedIdempotentResult -> { - return if (state.idempotentResume === idempotent) { - assert { state.result === value } // "Non-idempotent resume" - RESUME_TOKEN - } else { - null - } - } - else -> return null // cannot resume -- not active anymore - } - } - } + override fun tryResume(value: T, idempotent: Any?): Any? = + tryResumeImpl(value, idempotent, onCancellation = null) - override fun tryResumeWithException(exception: Throwable): Any? { - _state.loop { state -> - when (state) { - is NotCompleted -> { - val update = CompletedExceptionally(exception) - if (!_state.compareAndSet(state, update)) return@loop // retry on cas failure - detachChildIfNonResuable() - return RESUME_TOKEN - } - else -> return null // cannot resume -- not active anymore - } - } - } + override fun tryResume(value: T, idempotent: Any?, onCancellation: ((cause: Throwable) -> Unit)?): Any? = + tryResumeImpl(value, idempotent, onCancellation) + + override fun tryResumeWithException(exception: Throwable): Any? = + tryResumeImpl(CompletedExceptionally(exception), idempotent = null, onCancellation = null) // note: token is always RESUME_TOKEN override fun completeResume(token: Any) { @@ -407,11 +492,15 @@ internal open class CancellableContinuationImpl( @Suppress("UNCHECKED_CAST") override fun getSuccessfulResult(state: Any?): T = when (state) { - is CompletedIdempotentResult -> state.result as T - is CompletedWithCancellation -> state.result as T + is CompletedContinuation -> state.result as T else -> state as T } + // The exceptional state in CancellableContinuationImpl is stored directly and it is not recovered yet. + // The stacktrace recovery is invoked here. + override fun getExceptionalResult(state: Any?): Throwable? = + super.getExceptionalResult(state)?.let { recoverStackTrace(it, delegate) } + // For nicer debugging public override fun toString(): String = "${nameString()}(${delegate.toDebugString()}){$state}@$hexAddress" @@ -428,8 +517,20 @@ private object Active : NotCompleted { override fun toString(): String = "Active" } +/** + * Base class for all [CancellableContinuation.invokeOnCancellation] handlers to avoid an extra instance + * on JVM, yet support JS where you cannot extend from a functional type. + */ internal abstract class CancelHandler : CancelHandlerBase(), NotCompleted +/** + * Base class for all [CancellableContinuation.invokeOnCancellation] handlers that don't need to be invoked + * if continuation is cancelled after resumption, during dispatch, because the corresponding resources + * were already released before calling `resume`. This cancel handler is called only before `resume`. + * It avoids allocation of [CompletedContinuation] instance during resume on JVM. + */ +internal abstract class BeforeResumeCancelHandler : CancelHandler() + // Wrapper for lambdas, for the performance sake CancelHandler can be subclassed directly private class InvokeOnCancel( // Clashes with InvokeOnCancellation private val handler: CompletionHandler @@ -440,16 +541,18 @@ private class InvokeOnCancel( // Clashes with InvokeOnCancellation override fun toString() = "InvokeOnCancel[${handler.classSimpleName}@$hexAddress]" } -private class CompletedIdempotentResult( - @JvmField val idempotentResume: Any?, - @JvmField val result: Any? -) { - override fun toString(): String = "CompletedIdempotentResult[$result]" -} - -private class CompletedWithCancellation( +// Completed with additional metadata +private data class CompletedContinuation( @JvmField val result: Any?, - @JvmField val onCancellation: (cause: Throwable) -> Unit + @JvmField val cancelHandler: CancelHandler? = null, // installed via invokeOnCancellation + @JvmField val onCancellation: ((cause: Throwable) -> Unit)? = null, // installed via resume block + @JvmField val idempotentResume: Any? = null, + @JvmField val cancelCause: Throwable? = null ) { - override fun toString(): String = "CompletedWithCancellation[$result]" + val cancelled: Boolean get() = cancelCause != null + + fun invokeHandlers(cont: CancellableContinuationImpl<*>, cause: Throwable) { + cancelHandler?.let { cont.callCancelHandler(it, cause) } + onCancellation?.let { cont.callOnCancellation(it, cause) } + } } diff --git a/kotlinx-coroutines-core/common/src/CompletedExceptionally.kt b/kotlinx-coroutines-core/common/src/CompletionState.kt similarity index 78% rename from kotlinx-coroutines-core/common/src/CompletedExceptionally.kt rename to kotlinx-coroutines-core/common/src/CompletionState.kt index b426785bd7..f09aa3ccd9 100644 --- a/kotlinx-coroutines-core/common/src/CompletedExceptionally.kt +++ b/kotlinx-coroutines-core/common/src/CompletionState.kt @@ -9,10 +9,17 @@ import kotlinx.coroutines.internal.* import kotlin.coroutines.* import kotlin.jvm.* -internal fun Result.toState(): Any? = fold({ it }, { CompletedExceptionally(it) }) +internal fun Result.toState( + onCancellation: ((cause: Throwable) -> Unit)? = null +): Any? = fold( + onSuccess = { if (onCancellation != null) CompletedWithCancellation(it, onCancellation) else it }, + onFailure = { CompletedExceptionally(it) } +) -internal fun Result.toState(caller: CancellableContinuation<*>): Any? = fold({ it }, - { CompletedExceptionally(recoverStackTrace(it, caller)) }) +internal fun Result.toState(caller: CancellableContinuation<*>): Any? = fold( + onSuccess = { it }, + onFailure = { CompletedExceptionally(recoverStackTrace(it, caller)) } +) @Suppress("RESULT_CLASS_IN_RETURN_TYPE", "UNCHECKED_CAST") internal fun recoverResult(state: Any?, uCont: Continuation): Result = @@ -21,6 +28,11 @@ internal fun recoverResult(state: Any?, uCont: Continuation): Result = else Result.success(state as T) +internal data class CompletedWithCancellation( + @JvmField val result: Any?, + @JvmField val onCancellation: (cause: Throwable) -> Unit +) + /** * Class for an internal state of a job that was cancelled (completed exceptionally). * diff --git a/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt b/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt index 1b6e7eb00f..ab1e814b8a 100644 --- a/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt +++ b/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt @@ -4,6 +4,7 @@ package kotlinx.coroutines +import kotlinx.coroutines.internal.* import kotlin.coroutines.* /** diff --git a/kotlinx-coroutines-core/common/src/Deferred.kt b/kotlinx-coroutines-core/common/src/Deferred.kt index 72f3fde141..ff996756a3 100644 --- a/kotlinx-coroutines-core/common/src/Deferred.kt +++ b/kotlinx-coroutines-core/common/src/Deferred.kt @@ -43,6 +43,8 @@ public interface Deferred : Job { * This suspending function is cancellable. * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, this function * immediately resumes with [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. * * This function can be used in [select] invocation with [onAwait] clause. * Use [isCompleted] to check for completion of this deferred value without waiting. diff --git a/kotlinx-coroutines-core/common/src/Delay.kt b/kotlinx-coroutines-core/common/src/Delay.kt index ee28ec3b0e..adde8e433c 100644 --- a/kotlinx-coroutines-core/common/src/Delay.kt +++ b/kotlinx-coroutines-core/common/src/Delay.kt @@ -21,9 +21,12 @@ import kotlin.time.* public interface Delay { /** * Delays coroutine for a given time without blocking a thread and resumes it after a specified time. + * * This suspending function is cancellable. * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, this function * immediately resumes with [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. */ public suspend fun delay(time: Long) { if (time <= 0) return // don't delay @@ -97,9 +100,12 @@ public suspend fun awaitCancellation(): Nothing = suspendCancellableCoroutine {} /** * Delays coroutine for a given time without blocking a thread and resumes it after a specified time. + * * This suspending function is cancellable. * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, this function * immediately resumes with [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. * * If you want to delay forever (until cancellation), consider using [awaitCancellation] instead. * @@ -117,9 +123,12 @@ public suspend fun delay(timeMillis: Long) { /** * Delays coroutine for a given [duration] without blocking a thread and resumes it after the specified time. + * * This suspending function is cancellable. * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, this function * immediately resumes with [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. * * If you want to delay forever (until cancellation), consider using [awaitCancellation] instead. * diff --git a/kotlinx-coroutines-core/common/src/Yield.kt b/kotlinx-coroutines-core/common/src/Yield.kt index e0af04ddb7..0d8bd3bc2f 100644 --- a/kotlinx-coroutines-core/common/src/Yield.kt +++ b/kotlinx-coroutines-core/common/src/Yield.kt @@ -4,6 +4,7 @@ package kotlinx.coroutines +import kotlinx.coroutines.internal.* import kotlin.coroutines.* import kotlin.coroutines.intrinsics.* @@ -13,6 +14,8 @@ import kotlin.coroutines.intrinsics.* * This suspending function is cancellable. * If the [Job] of the current coroutine is cancelled or completed when this suspending function is invoked or while * this function is waiting for dispatch, it resumes with a [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. * * **Note**: This function always [checks for cancellation][ensureActive] even when it does not suspend. * diff --git a/kotlinx-coroutines-core/common/src/channels/AbstractChannel.kt b/kotlinx-coroutines-core/common/src/channels/AbstractChannel.kt index 28c7ceabe1..3cd6de5f51 100644 --- a/kotlinx-coroutines-core/common/src/channels/AbstractChannel.kt +++ b/kotlinx-coroutines-core/common/src/channels/AbstractChannel.kt @@ -16,7 +16,9 @@ import kotlin.native.concurrent.* /** * Abstract send channel. It is a base class for all send channel implementations. */ -internal abstract class AbstractSendChannel : SendChannel { +internal abstract class AbstractSendChannel( + @JvmField protected val onUndeliveredElement: OnUndeliveredElement? +) : SendChannel { /** @suppress **This is unstable API and it is subject to change.** */ protected val queue = LockFreeLinkedListHead() @@ -151,24 +153,34 @@ internal abstract class AbstractSendChannel : SendChannel { // We should check for closed token on offer as well, otherwise offer won't be linearizable // in the face of concurrent close() // See https://github.com/Kotlin/kotlinx.coroutines/issues/359 - throw recoverStackTrace(helpCloseAndGetSendException(closedForSend ?: return false)) + throw recoverStackTrace(helpCloseAndGetSendException(element, closedForSend ?: return false)) + } + result is Closed<*> -> { + throw recoverStackTrace(helpCloseAndGetSendException(element, result)) } - result is Closed<*> -> throw recoverStackTrace(helpCloseAndGetSendException(result)) else -> error("offerInternal returned $result") } } - private fun helpCloseAndGetSendException(closed: Closed<*>): Throwable { + private fun helpCloseAndGetSendException(element: E, closed: Closed<*>): Throwable { // To ensure linearizablity we must ALWAYS help close the channel when we observe that it was closed // See https://github.com/Kotlin/kotlinx.coroutines/issues/1419 helpClose(closed) + // Element was not delivered -> cals onUndeliveredElement + onUndeliveredElement?.callUndeliveredElementCatchingException(element)?.let { + // If it crashes, add send exception as suppressed for better diagnostics + it.addSuppressed(closed.sendException) + throw it + } return closed.sendException } - private suspend fun sendSuspend(element: E): Unit = suspendAtomicCancellableCoroutineReusable sc@ { cont -> + private suspend fun sendSuspend(element: E): Unit = suspendCancellableCoroutineReusable sc@ { cont -> loop@ while (true) { if (isFullImpl) { - val send = SendElement(element, cont) + val send = if (onUndeliveredElement == null) + SendElement(element, cont) else + SendElementWithUndeliveredHandler(element, cont, onUndeliveredElement) val enqueueResult = enqueueSend(send) when { enqueueResult == null -> { // enqueued successfully @@ -176,7 +188,7 @@ internal abstract class AbstractSendChannel : SendChannel { return@sc } enqueueResult is Closed<*> -> { - cont.helpCloseAndResumeWithSendException(enqueueResult) + cont.helpCloseAndResumeWithSendException(element, enqueueResult) return@sc } enqueueResult === ENQUEUE_FAILED -> {} // try to offer instead @@ -193,7 +205,7 @@ internal abstract class AbstractSendChannel : SendChannel { } offerResult === OFFER_FAILED -> continue@loop offerResult is Closed<*> -> { - cont.helpCloseAndResumeWithSendException(offerResult) + cont.helpCloseAndResumeWithSendException(element, offerResult) return@sc } else -> error("offerInternal returned $offerResult") @@ -201,9 +213,15 @@ internal abstract class AbstractSendChannel : SendChannel { } } - private fun Continuation<*>.helpCloseAndResumeWithSendException(closed: Closed<*>) { + private fun Continuation<*>.helpCloseAndResumeWithSendException(element: E, closed: Closed<*>) { helpClose(closed) - resumeWithException(closed.sendException) + val sendException = closed.sendException + onUndeliveredElement?.callUndeliveredElementCatchingException(element)?.let { + it.addSuppressed(sendException) + resumeWithException(it) + return + } + resumeWithException(sendException) } /** @@ -375,7 +393,7 @@ internal abstract class AbstractSendChannel : SendChannel { select.disposeOnSelect(node) return } - enqueueResult is Closed<*> -> throw recoverStackTrace(helpCloseAndGetSendException(enqueueResult)) + enqueueResult is Closed<*> -> throw recoverStackTrace(helpCloseAndGetSendException(element, enqueueResult)) enqueueResult === ENQUEUE_FAILED -> {} // try to offer enqueueResult is Receive<*> -> {} // try to offer else -> error("enqueueSend returned $enqueueResult ") @@ -391,7 +409,7 @@ internal abstract class AbstractSendChannel : SendChannel { block.startCoroutineUnintercepted(receiver = this, completion = select.completion) return } - offerResult is Closed<*> -> throw recoverStackTrace(helpCloseAndGetSendException(offerResult)) + offerResult is Closed<*> -> throw recoverStackTrace(helpCloseAndGetSendException(element, offerResult)) else -> error("offerSelectInternal returned $offerResult") } } @@ -431,7 +449,7 @@ internal abstract class AbstractSendChannel : SendChannel { // ------ private ------ private class SendSelect( - override val pollResult: Any?, + override val pollResult: E, // E | Closed - the result pollInternal returns when it rendezvous with this node @JvmField val channel: AbstractSendChannel, @JvmField val select: SelectInstance, @JvmField val block: suspend (SendChannel) -> R @@ -440,11 +458,13 @@ internal abstract class AbstractSendChannel : SendChannel { select.trySelectOther(otherOp) as Symbol? // must return symbol override fun completeResumeSend() { - block.startCoroutine(receiver = channel, completion = select.completion) + block.startCoroutineCancellable(receiver = channel, completion = select.completion) } override fun dispose() { // invoked on select completion - remove() + if (!remove()) return + // if the node was successfully removed (meaning it was added but was not received) then element not delivered + undeliveredElement() } override fun resumeSendClosed(closed: Closed<*>) { @@ -452,6 +472,10 @@ internal abstract class AbstractSendChannel : SendChannel { select.resumeSelectWithException(closed.sendException) } + override fun undeliveredElement() { + channel.onUndeliveredElement?.callUndeliveredElement(pollResult, select.completion.context) + } + override fun toString(): String = "SendSelect@$hexAddress($pollResult)[$channel, $select]" } @@ -469,7 +493,9 @@ internal abstract class AbstractSendChannel : SendChannel { /** * Abstract send/receive channel. It is a base class for all channel implementations. */ -internal abstract class AbstractChannel : AbstractSendChannel(), Channel { +internal abstract class AbstractChannel( + onUndeliveredElement: OnUndeliveredElement? +) : AbstractSendChannel(onUndeliveredElement), Channel { // ------ extension points for buffered channels ------ /** @@ -501,6 +527,8 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel : AbstractSendChannel(), Channel receiveSuspend(receiveMode: Int): R = suspendAtomicCancellableCoroutineReusable sc@ { cont -> - val receive = ReceiveElement(cont as CancellableContinuation, receiveMode) + private suspend fun receiveSuspend(receiveMode: Int): R = suspendCancellableCoroutineReusable sc@ { cont -> + val receive = if (onUndeliveredElement == null) + ReceiveElement(cont as CancellableContinuation, receiveMode) else + ReceiveElementWithUndeliveredHandler(cont as CancellableContinuation, receiveMode, onUndeliveredElement) while (true) { if (enqueueReceive(receive)) { removeReceiveOnCancel(cont, receive) @@ -561,7 +591,7 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel : AbstractSendChannel(), Channel @@ -785,7 +820,7 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel, receive: Receive<*>) = cont.invokeOnCancellation(handler = RemoveReceiveOnCancel(receive).asHandler) - private inner class RemoveReceiveOnCancel(private val receive: Receive<*>) : CancelHandler() { + private inner class RemoveReceiveOnCancel(private val receive: Receive<*>) : BeforeResumeCancelHandler() { override fun invoke(cause: Throwable?) { if (receive.remove()) onReceiveDequeued() @@ -793,7 +828,7 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel(val channel: AbstractChannel) : ChannelIterator { + private class Itr(@JvmField val channel: AbstractChannel) : ChannelIterator { var result: Any? = POLL_FAILED // E | POLL_FAILED | Closed override suspend fun hasNext(): Boolean { @@ -814,7 +849,7 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel + private suspend fun hasNextSuspend(): Boolean = suspendCancellableCoroutineReusable sc@ { cont -> val receive = ReceiveHasNext(this, cont) while (true) { if (channel.enqueueReceive(receive)) { @@ -832,7 +867,8 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel : AbstractSendChannel(), Channel( + private open class ReceiveElement( @JvmField val cont: CancellableContinuation, @JvmField val receiveMode: Int ) : Receive() { @@ -860,9 +896,8 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel value } - @Suppress("IMPLICIT_CAST_TO_ANY") override fun tryResumeReceive(value: E, otherOp: PrepareOp?): Symbol? { - val token = cont.tryResume(resumeValue(value), otherOp?.desc) ?: return null + val token = cont.tryResume(resumeValue(value), otherOp?.desc, resumeOnCancellationFun(value)) ?: return null assert { token === RESUME_TOKEN } // the only other possible result // We can call finishPrepare only after successful tryResume, so that only good affected node is saved otherOp?.finishPrepare() @@ -881,12 +916,22 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel( + private class ReceiveElementWithUndeliveredHandler( + cont: CancellableContinuation, + receiveMode: Int, + @JvmField val onUndeliveredElement: OnUndeliveredElement + ) : ReceiveElement(cont, receiveMode) { + override fun resumeOnCancellationFun(value: E): ((Throwable) -> Unit)? = + onUndeliveredElement.bindCancellationFun(value, cont.context) + } + + private open class ReceiveHasNext( @JvmField val iterator: Itr, @JvmField val cont: CancellableContinuation ) : Receive() { override fun tryResumeReceive(value: E, otherOp: PrepareOp?): Symbol? { - val token = cont.tryResume(true, otherOp?.desc) ?: return null + val token = cont.tryResume(true, otherOp?.desc, resumeOnCancellationFun(value)) + ?: return null assert { token === RESUME_TOKEN } // the only other possible result // We can call finishPrepare only after successful tryResume, so that only good affected node is saved otherOp?.finishPrepare() @@ -906,13 +951,17 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel Unit)? = + iterator.channel.onUndeliveredElement?.bindCancellationFun(value, cont.context) + override fun toString(): String = "ReceiveHasNext@$hexAddress" } @@ -927,16 +976,20 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel) { if (!select.trySelect()) return when (receiveMode) { RECEIVE_THROWS_ON_CLOSE -> select.resumeSelectWithException(closed.receiveException) - RECEIVE_RESULT -> block.startCoroutine(ValueOrClosed.closed(closed.closeCause), select.completion) + RECEIVE_RESULT -> block.startCoroutineCancellable(ValueOrClosed.closed(closed.closeCause), select.completion) RECEIVE_NULL_ON_CLOSE -> if (closed.closeCause == null) { - block.startCoroutine(null, select.completion) + block.startCoroutineCancellable(null, select.completion) } else { select.resumeSelectWithException(closed.receiveException) } @@ -948,6 +1001,9 @@ internal abstract class AbstractChannel : AbstractSendChannel(), Channel Unit)? = + channel.onUndeliveredElement?.bindCancellationFun(value, select.completion.context) + override fun toString(): String = "ReceiveSelect@$hexAddress[$select,receiveMode=$receiveMode]" } } @@ -957,6 +1013,10 @@ internal const val RECEIVE_THROWS_ON_CLOSE = 0 internal const val RECEIVE_NULL_ON_CLOSE = 1 internal const val RECEIVE_RESULT = 2 +@JvmField +@SharedImmutable +internal val EMPTY = Symbol("EMPTY") // marker for Conflated & Buffered channels + @JvmField @SharedImmutable internal val OFFER_SUCCESS: Any = Symbol("OFFER_SUCCESS") @@ -983,7 +1043,7 @@ internal typealias Handler = (Throwable?) -> Unit * Represents sending waiter in the queue. */ internal abstract class Send : LockFreeLinkedListNode() { - abstract val pollResult: Any? // E | Closed + abstract val pollResult: Any? // E | Closed - the result pollInternal returns when it rendezvous with this node // Returns: null - failure, // RETRY_ATOMIC for retry (only when otherOp != null), // RESUME_TOKEN on success (call completeResumeSend) @@ -991,6 +1051,7 @@ internal abstract class Send : LockFreeLinkedListNode() { abstract fun tryResumeSend(otherOp: PrepareOp?): Symbol? abstract fun completeResumeSend() abstract fun resumeSendClosed(closed: Closed<*>) + open fun undeliveredElement() {} } /** @@ -1009,9 +1070,8 @@ internal interface ReceiveOrClosed { /** * Represents sender for a specific element. */ -@Suppress("UNCHECKED_CAST") -internal class SendElement( - override val pollResult: Any?, +internal open class SendElement( + override val pollResult: E, @JvmField val cont: CancellableContinuation ) : Send() { override fun tryResumeSend(otherOp: PrepareOp?): Symbol? { @@ -1021,9 +1081,27 @@ internal class SendElement( otherOp?.finishPrepare() // finish preparations return RESUME_TOKEN } + override fun completeResumeSend() = cont.completeResume(RESUME_TOKEN) override fun resumeSendClosed(closed: Closed<*>) = cont.resumeWithException(closed.sendException) - override fun toString(): String = "SendElement@$hexAddress($pollResult)" + override fun toString(): String = "$classSimpleName@$hexAddress($pollResult)" +} + +internal class SendElementWithUndeliveredHandler( + pollResult: E, + cont: CancellableContinuation, + @JvmField val onUndeliveredElement: OnUndeliveredElement +) : SendElement(pollResult, cont) { + override fun remove(): Boolean { + if (!super.remove()) return false + // if the node was successfully removed (meaning it was added but was not received) then we have undelivered element + undeliveredElement() + return true + } + + override fun undeliveredElement() { + onUndeliveredElement.callUndeliveredElement(pollResult, cont.context) + } } /** @@ -1048,6 +1126,7 @@ internal class Closed( internal abstract class Receive : LockFreeLinkedListNode(), ReceiveOrClosed { override val offerResult get() = OFFER_SUCCESS abstract fun resumeReceiveClosed(closed: Closed<*>) + open fun resumeOnCancellationFun(value: E): ((Throwable) -> Unit)? = null } @Suppress("NOTHING_TO_INLINE", "UNCHECKED_CAST") diff --git a/kotlinx-coroutines-core/common/src/channels/ArrayBroadcastChannel.kt b/kotlinx-coroutines-core/common/src/channels/ArrayBroadcastChannel.kt index 155652fd6f..91b5473c41 100644 --- a/kotlinx-coroutines-core/common/src/channels/ArrayBroadcastChannel.kt +++ b/kotlinx-coroutines-core/common/src/channels/ArrayBroadcastChannel.kt @@ -28,7 +28,7 @@ internal class ArrayBroadcastChannel( * Buffer capacity. */ val capacity: Int -) : AbstractSendChannel(), BroadcastChannel { +) : AbstractSendChannel(null), BroadcastChannel { init { require(capacity >= 1) { "ArrayBroadcastChannel capacity must be at least 1, but $capacity was specified" } } @@ -180,6 +180,8 @@ internal class ArrayBroadcastChannel( this.tail = tail + 1 return@withLock // go out of lock to wakeup this sender } + // Too late, already cancelled, but we removed it from the queue and need to release resources. + // However, ArrayBroadcastChannel does not support onUndeliveredElement, so nothing to do } } } @@ -205,7 +207,7 @@ internal class ArrayBroadcastChannel( private class Subscriber( private val broadcastChannel: ArrayBroadcastChannel - ) : AbstractChannel(), ReceiveChannel { + ) : AbstractChannel(null), ReceiveChannel { private val subLock = ReentrantLock() private val _subHead = atomic(0L) diff --git a/kotlinx-coroutines-core/common/src/channels/ArrayChannel.kt b/kotlinx-coroutines-core/common/src/channels/ArrayChannel.kt index e26579eff7..8a08106516 100644 --- a/kotlinx-coroutines-core/common/src/channels/ArrayChannel.kt +++ b/kotlinx-coroutines-core/common/src/channels/ArrayChannel.kt @@ -23,8 +23,9 @@ internal open class ArrayChannel( /** * Buffer capacity. */ - val capacity: Int -) : AbstractChannel() { + val capacity: Int, + onUndeliveredElement: OnUndeliveredElement? +) : AbstractChannel(onUndeliveredElement) { init { require(capacity >= 1) { "ArrayChannel capacity must be at least 1, but $capacity was specified" } } @@ -34,7 +35,8 @@ internal open class ArrayChannel( * Guarded by lock. * Allocate minimum of capacity and 16 to avoid excess memory pressure for large channels when it's not necessary. */ - private var buffer: Array = arrayOfNulls(min(capacity, 8)) + private var buffer: Array = arrayOfNulls(min(capacity, 8)).apply { fill(EMPTY) } + private var head: Int = 0 private val size = atomic(0) // Invariant: size <= capacity @@ -143,6 +145,7 @@ internal open class ArrayChannel( for (i in 0 until currentSize) { newBuffer[i] = buffer[(head + i) % buffer.size] } + newBuffer.fill(EMPTY, currentSize, newSize) buffer = newBuffer head = 0 } @@ -172,6 +175,8 @@ internal open class ArrayChannel( replacement = send!!.pollResult break@loop } + // too late, already cancelled, but we removed it from the queue and need to notify on undelivered element + send!!.undeliveredElement() } } if (replacement !== POLL_FAILED && replacement !is Closed<*>) { @@ -254,17 +259,23 @@ internal open class ArrayChannel( // Note: this function is invoked when channel is already closed override fun onCancelIdempotent(wasClosed: Boolean) { // clear buffer first, but do not wait for it in helpers - if (wasClosed) { - lock.withLock { - repeat(size.value) { - buffer[head] = 0 - head = (head + 1) % buffer.size + val onUndeliveredElement = onUndeliveredElement + var undeliveredElementException: UndeliveredElementException? = null // first cancel exception, others suppressed + lock.withLock { + repeat(size.value) { + val value = buffer[head] + if (onUndeliveredElement != null && value !== EMPTY) { + @Suppress("UNCHECKED_CAST") + undeliveredElementException = onUndeliveredElement.callUndeliveredElementCatchingException(value as E, undeliveredElementException) } - size.value = 0 + buffer[head] = EMPTY + head = (head + 1) % buffer.size } + size.value = 0 } // then clean all queued senders super.onCancelIdempotent(wasClosed) + undeliveredElementException?.let { throw it } // throw cancel exception at the end if there was one } // ------ debug ------ diff --git a/kotlinx-coroutines-core/common/src/channels/Channel.kt b/kotlinx-coroutines-core/common/src/channels/Channel.kt index c4b4a9b25e..25085ed2d2 100644 --- a/kotlinx-coroutines-core/common/src/channels/Channel.kt +++ b/kotlinx-coroutines-core/common/src/channels/Channel.kt @@ -44,19 +44,17 @@ public interface SendChannel { * Sends the specified [element] to this channel, suspending the caller while the buffer of this channel is full * or if it does not exist, or throws an exception if the channel [is closed for `send`][isClosedForSend] (see [close] for details). * - * Note that closing a channel _after_ this function has suspended does not cause this suspended [send] invocation + * [Closing][close] a channel _after_ this function has suspended does not cause this suspended [send] invocation * to abort, because closing a channel is conceptually like sending a special "close token" over this channel. * All elements sent over the channel are delivered in first-in first-out order. The sent element * will be delivered to receivers before the close token. * * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this * function is suspended, this function immediately resumes with a [CancellationException]. - * - * *Cancellation of suspended `send` is atomic*: when this function - * throws a [CancellationException], it means that the [element] was not sent to this channel. - * As a side-effect of atomic cancellation, a thread-bound coroutine (to some UI thread, for example) may - * continue to execute even after it was cancelled from the same thread in the case when this `send` operation - * was already resumed and the continuation was posted for execution to the thread's queue. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. The `send` call can send the element to the channel, + * but then throw [CancellationException], thus an exception should not be treated as a failure to deliver the element. + * See "Undelivered elements" section in [Channel] documentation for details on handling undelivered elements. * * Note that this function does not check for cancellation when it is not suspended. * Use [yield] or [CoroutineScope.isActive] to periodically check for cancellation in tight loops if needed. @@ -81,6 +79,11 @@ public interface SendChannel { * in situations when `send` suspends. * * Throws an exception if the channel [is closed for `send`][isClosedForSend] (see [close] for details). + * + * When `offer` call returns `false` it guarantees that the element was not delivered to the consumer and it + * it does not call `onUndeliveredElement` that was installed for this channel. If the channel was closed, + * then it calls `onUndeliveredElement` before throwing an exception. + * See "Undelivered elements" section in [Channel] documentation for details on handling undelivered elements. */ public fun offer(element: E): Boolean @@ -170,12 +173,10 @@ public interface ReceiveChannel { * * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this * function is suspended, this function immediately resumes with a [CancellationException]. - * - * *Cancellation of suspended `receive` is atomic*: when this function - * throws a [CancellationException], it means that the element was not retrieved from this channel. - * As a side-effect of atomic cancellation, a thread-bound coroutine (to some UI thread, for example) may - * continue to execute even after it was cancelled from the same thread in the case when this `receive` operation - * was already resumed and the continuation was posted for execution to the thread's queue. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. The `receive` call can retrieve the element from the channel, + * but then throw [CancellationException], thus failing to deliver the element. + * See "Undelivered elements" section in [Channel] documentation for details on handling undelivered elements. * * Note that this function does not check for cancellation when it is not suspended. * Use [yield] or [CoroutineScope.isActive] to periodically check for cancellation in tight loops if needed. @@ -200,12 +201,10 @@ public interface ReceiveChannel { * * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this * function is suspended, this function immediately resumes with a [CancellationException]. - * - * *Cancellation of suspended `receive` is atomic*: when this function - * throws a [CancellationException], it means that the element was not retrieved from this channel. - * As a side-effect of atomic cancellation, a thread-bound coroutine (to some UI thread, for example) may - * continue to execute even after it was cancelled from the same thread in the case when this `receive` operation - * was already resumed and the continuation was posted for execution to the thread's queue. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. The `receiveOrNull` call can retrieve the element from the channel, + * but then throw [CancellationException], thus failing to deliver the element. + * See "Undelivered elements" section in [Channel] documentation for details on handling undelivered elements. * * Note that this function does not check for cancellation when it is not suspended. * Use [yield] or [CoroutineScope.isActive] to periodically check for cancellation in tight loops if needed. @@ -250,12 +249,10 @@ public interface ReceiveChannel { * * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this * function is suspended, this function immediately resumes with a [CancellationException]. - * - * *Cancellation of suspended `receive` is atomic*: when this function - * throws a [CancellationException], it means that the element was not retrieved from this channel. - * As a side-effect of atomic cancellation, a thread-bound coroutine (to some UI thread, for example) may - * continue to execute even after it was cancelled from the same thread in the case when this receive operation - * was already resumed and the continuation was posted for execution to the thread's queue. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. The `receiveOrClosed` call can retrieve the element from the channel, + * but then throw [CancellationException], thus failing to deliver the element. + * See "Undelivered elements" section in [Channel] documentation for details on handling undelivered elements. * * Note that this function does not check for cancellation when it is not suspended. * Use [yield] or [CoroutineScope.isActive] to periodically check for cancellation in tight loops if needed. @@ -332,7 +329,7 @@ public interface ReceiveChannel { * @suppress *This is an internal API, do not use*: Inline classes ABI is not stable yet and * [KT-27524](https://youtrack.jetbrains.com/issue/KT-27524) needs to be fixed. */ -@Suppress("NON_PUBLIC_PRIMARY_CONSTRUCTOR_OF_INLINE_CLASS") +@Suppress("NON_PUBLIC_PRIMARY_CONSTRUCTOR_OF_INLINE_CLASS", "EXPERIMENTAL_FEATURE_WARNING") @InternalCoroutinesApi // until https://youtrack.jetbrains.com/issue/KT-27524 is fixed public inline class ValueOrClosed internal constructor(private val holder: Any?) { @@ -439,12 +436,10 @@ public interface ChannelIterator { * * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this * function is suspended, this function immediately resumes with a [CancellationException]. - * - * *Cancellation of suspended `receive` is atomic*: when this function - * throws a [CancellationException], it means that the element was not retrieved from this channel. - * As a side-effect of atomic cancellation, a thread-bound coroutine (to some UI thread, for example) may - * continue to execute even after it was cancelled from the same thread in the case when this receive operation - * was already resumed and the continuation was posted for execution to the thread's queue. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. The `hasNext` call can retrieve the element from the channel, + * but then throw [CancellationException], thus failing to deliver the element. + * See "Undelivered elements" section in [Channel] documentation for details on handling undelivered elements. * * Note that this function does not check for cancellation when it is not suspended. * Use [yield] or [CoroutineScope.isActive] to periodically check for cancellation in tight loops if needed. @@ -508,6 +503,69 @@ public interface ChannelIterator { * * When `capacity` is positive but less than [UNLIMITED] — it creates an array-based channel with the specified capacity. * This channel has an array buffer of a fixed `capacity`. * [Sending][send] suspends only when the buffer is full, and [receiving][receive] suspends only when the buffer is empty. + * + * ### Prompt cancellation guarantee + * + * All suspending functions with channels provide **prompt cancellation guarantee**. + * If the job was cancelled while send or receive function was suspended, it will not resume successfully, + * but throws a [CancellationException]. + * With a single-threaded [dispatcher][CoroutineDispatcher] like [Dispatchers.Main] this gives a + * guarantee that if a piece code running in this thread cancels a [Job], then a coroutine running this job cannot + * resume successfully and continue to run, ensuring a prompt response to its cancellation. + * + * > **Prompt cancellation guarantee** for channel operations was added since `kotlinx.coroutines` version `1.4.0` + * > and had replaced a channel-specific atomic-cancellation that was not consistent with other suspending functions. + * > The low-level mechanics of prompt cancellation are explained in [suspendCancellableCoroutine] function. + * + * ### Undelivered elements + * + * As a result of a prompt cancellation guarantee, when a closeable resource + * (like open file or a handle to another native resource) is transferred via channel from one coroutine to another + * it can fail to be delivered and will be lost if either send or receive operations are cancelled in transit. + * + * A `Channel()` constructor function has an `onUndeliveredElement` optional parameter. + * When `onUndeliveredElement` parameter is set, the corresponding function is called once for each element + * that was sent to the channel with the call to the [send][SendChannel.send] function but failed to be delivered, + * which can happen in the following cases: + * + * * When [send][SendChannel.send] operation throws an exception because it was cancelled before it had a chance to actually + * send the element or because the channel was [closed][SendChannel.close] or [cancelled][ReceiveChannel.cancel]. + * * When [offer][SendChannel.offer] operation throws an exception when + * the channel was [closed][SendChannel.close] or [cancelled][ReceiveChannel.cancel]. + * * When [receive][ReceiveChannel.receive], [receiveOrNull][ReceiveChannel.receiveOrNull], or [hasNext][ChannelIterator.hasNext] + * operation throws an exception when it had retrieved the element from the + * channel but was cancelled before the code following the receive call resumed. + * * The channel was [cancelled][ReceiveChannel.cancel], in which case `onUndeliveredElement` is called on every + * remaining element in the channel's buffer. + * + * Note, that `onUndeliveredElement` function is called synchronously in an arbitrary context. It should be fast, non-blocking, + * and should not throw exceptions. Any exception thrown by `onUndeliveredElement` is wrapped into an internal runtime + * exception which is either rethrown from the caller method or handed off to the exception handler in the current context + * (see [CoroutineExceptionHandler]) when one is available. + * + * A typical usage for `onDeliveredElement` is to close a resource that is being transferred via the channel. The + * following code pattern guarantees that opened resources are closed even if producer, consumer, and/or channel + * are cancelled. Resources are never lost. + * + * ``` + * // Create the channel with onUndeliveredElement block that closes a resource + * val channel = Channel(capacity) { resource -> resource.close() } + * + * // Producer code + * val resourceToSend = openResource() + * channel.send(resourceToSend) + * + * // Consumer code + * val resourceReceived = channel.receive() + * try { + * // work with received resource + * } finally { + * resourceReceived.close() + * } + * ``` + * + * > Note, that if you do any kind of work in between `openResource()` and `channel.send(...)`, then you should + * > ensure that resource gets closed in case this additional code fails. */ public interface Channel : SendChannel, ReceiveChannel { /** @@ -557,17 +615,26 @@ public interface Channel : SendChannel, ReceiveChannel { * See [Channel] interface documentation for details. * * @param capacity either a positive channel capacity or one of the constants defined in [Channel.Factory]. + * @param onUndeliveredElement an optional function that is called when element was sent but was not delivered to the consumer. + * See "Undelivered elements" section in [Channel] documentation. * @throws IllegalArgumentException when [capacity] < -2 */ -public fun Channel(capacity: Int = RENDEZVOUS): Channel = +public fun Channel(capacity: Int = RENDEZVOUS, onUndeliveredElement: ((E) -> Unit)? = null): Channel = when (capacity) { - RENDEZVOUS -> RendezvousChannel() - UNLIMITED -> LinkedListChannel() - CONFLATED -> ConflatedChannel() - BUFFERED -> ArrayChannel(CHANNEL_DEFAULT_CAPACITY) - else -> ArrayChannel(capacity) + RENDEZVOUS -> RendezvousChannel(onUndeliveredElement) + UNLIMITED -> LinkedListChannel(onUndeliveredElement) + CONFLATED -> ConflatedChannel(onUndeliveredElement) + BUFFERED -> ArrayChannel(CHANNEL_DEFAULT_CAPACITY, onUndeliveredElement) + else -> ArrayChannel(capacity, onUndeliveredElement) } +/** + * @suppress Binary compatibility only, should not be documented + */ +// This declaration is hidden since version 1.4.0 +@Deprecated(level = DeprecationLevel.HIDDEN, message = "Binary compatibility") +public fun Channel(capacity: Int = RENDEZVOUS): Channel = Channel(capacity, onUndeliveredElement = null) + /** * Indicates an attempt to [send][SendChannel.send] to a [isClosedForSend][SendChannel.isClosedForSend] channel * that was closed without a cause. A _failed_ channel rethrows the original [close][SendChannel.close] cause diff --git a/kotlinx-coroutines-core/common/src/channels/Channels.common.kt b/kotlinx-coroutines-core/common/src/channels/Channels.common.kt index 8c61928aa4..d19028bf63 100644 --- a/kotlinx-coroutines-core/common/src/channels/Channels.common.kt +++ b/kotlinx-coroutines-core/common/src/channels/Channels.common.kt @@ -40,12 +40,9 @@ public inline fun BroadcastChannel.consume(block: ReceiveChannel.() * * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this * function is suspended, this function immediately resumes with [CancellationException]. - * - * *Cancellation of suspended receive is atomic* -- when this function - * throws [CancellationException] it means that the element was not retrieved from this channel. - * As a side-effect of atomic cancellation, a thread-bound coroutine (to some UI thread, for example) may - * continue to execute even after it was cancelled from the same thread in the case when this receive operation - * was already resumed and the continuation was posted for execution to the thread's queue. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. If the `receiveOrNull` call threw [CancellationException] there is no way + * to tell if some element was already received from the channel or not. See [Channel] documentation for details. * * Note, that this function does not check for cancellation when it is not suspended. * Use [yield] or [CoroutineScope.isActive] to periodically check for cancellation in tight loops if needed. diff --git a/kotlinx-coroutines-core/common/src/channels/ConflatedBroadcastChannel.kt b/kotlinx-coroutines-core/common/src/channels/ConflatedBroadcastChannel.kt index 2b9375ddec..ba2ccea92d 100644 --- a/kotlinx-coroutines-core/common/src/channels/ConflatedBroadcastChannel.kt +++ b/kotlinx-coroutines-core/common/src/channels/ConflatedBroadcastChannel.kt @@ -282,7 +282,7 @@ public class ConflatedBroadcastChannel() : BroadcastChannel { private class Subscriber( private val broadcastChannel: ConflatedBroadcastChannel - ) : ConflatedChannel(), ReceiveChannel { + ) : ConflatedChannel(null), ReceiveChannel { override fun onCancelIdempotent(wasClosed: Boolean) { if (wasClosed) { diff --git a/kotlinx-coroutines-core/common/src/channels/ConflatedChannel.kt b/kotlinx-coroutines-core/common/src/channels/ConflatedChannel.kt index 4734766914..75e421c6e7 100644 --- a/kotlinx-coroutines-core/common/src/channels/ConflatedChannel.kt +++ b/kotlinx-coroutines-core/common/src/channels/ConflatedChannel.kt @@ -7,7 +7,6 @@ package kotlinx.coroutines.channels import kotlinx.coroutines.* import kotlinx.coroutines.internal.* import kotlinx.coroutines.selects.* -import kotlin.native.concurrent.* /** * Channel that buffers at most one element and conflates all subsequent `send` and `offer` invocations, @@ -18,7 +17,7 @@ import kotlin.native.concurrent.* * * This channel is created by `Channel(Channel.CONFLATED)` factory function invocation. */ -internal open class ConflatedChannel : AbstractChannel() { +internal open class ConflatedChannel(onUndeliveredElement: OnUndeliveredElement?) : AbstractChannel(onUndeliveredElement) { protected final override val isBufferAlwaysEmpty: Boolean get() = false protected final override val isBufferEmpty: Boolean get() = value === EMPTY protected final override val isBufferAlwaysFull: Boolean get() = false @@ -30,10 +29,6 @@ internal open class ConflatedChannel : AbstractChannel() { private var value: Any? = EMPTY - private companion object { - private val EMPTY = Symbol("EMPTY") - } - // result is `OFFER_SUCCESS | Closed` protected override fun offerInternal(element: E): Any { var receive: ReceiveOrClosed? = null @@ -54,7 +49,7 @@ internal open class ConflatedChannel : AbstractChannel() { } } } - value = element + updateValueLocked(element)?.let { throw it } return OFFER_SUCCESS } // breaks here if offer meets receiver @@ -87,7 +82,7 @@ internal open class ConflatedChannel : AbstractChannel() { if (!select.trySelect()) { return ALREADY_SELECTED } - value = element + updateValueLocked(element)?.let { throw it } return OFFER_SUCCESS } // breaks here if offer meets receiver @@ -120,12 +115,20 @@ internal open class ConflatedChannel : AbstractChannel() { } protected override fun onCancelIdempotent(wasClosed: Boolean) { - if (wasClosed) { - lock.withLock { - value = EMPTY - } + var undeliveredElementException: UndeliveredElementException? = null // resource cancel exception + lock.withLock { + undeliveredElementException = updateValueLocked(EMPTY) } super.onCancelIdempotent(wasClosed) + undeliveredElementException?.let { throw it } // throw exception at the end if there was one + } + + private fun updateValueLocked(element: Any?): UndeliveredElementException? { + val old = value + val undeliveredElementException = if (old === EMPTY) null else + onUndeliveredElement?.callUndeliveredElementCatchingException(old as E) + value = element + return undeliveredElementException } override fun enqueueReceiveInternal(receive: Receive): Boolean = lock.withLock { diff --git a/kotlinx-coroutines-core/common/src/channels/LinkedListChannel.kt b/kotlinx-coroutines-core/common/src/channels/LinkedListChannel.kt index e66bbb2279..2f46421344 100644 --- a/kotlinx-coroutines-core/common/src/channels/LinkedListChannel.kt +++ b/kotlinx-coroutines-core/common/src/channels/LinkedListChannel.kt @@ -17,7 +17,7 @@ import kotlinx.coroutines.selects.* * * @suppress **This an internal API and should not be used from general code.** */ -internal open class LinkedListChannel : AbstractChannel() { +internal open class LinkedListChannel(onUndeliveredElement: OnUndeliveredElement?) : AbstractChannel(onUndeliveredElement) { protected final override val isBufferAlwaysEmpty: Boolean get() = true protected final override val isBufferEmpty: Boolean get() = true protected final override val isBufferAlwaysFull: Boolean get() = false diff --git a/kotlinx-coroutines-core/common/src/channels/Produce.kt b/kotlinx-coroutines-core/common/src/channels/Produce.kt index 1b1581a99e..6e454d4b27 100644 --- a/kotlinx-coroutines-core/common/src/channels/Produce.kt +++ b/kotlinx-coroutines-core/common/src/channels/Produce.kt @@ -27,7 +27,11 @@ public interface ProducerScope : CoroutineScope, SendChannel { /** * Suspends the current coroutine until the channel is either [closed][SendChannel.close] or [cancelled][ReceiveChannel.cancel] - * and invokes the given [block] before resuming the coroutine. This suspending function is cancellable. + * and invokes the given [block] before resuming the coroutine. + * + * This suspending function is cancellable. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. * * Note that when the producer channel is cancelled, this function resumes with a cancellation exception. * Therefore, in case of cancellation, no code after the call to this function will be executed. diff --git a/kotlinx-coroutines-core/common/src/channels/RendezvousChannel.kt b/kotlinx-coroutines-core/common/src/channels/RendezvousChannel.kt index 700f50908c..857a97938f 100644 --- a/kotlinx-coroutines-core/common/src/channels/RendezvousChannel.kt +++ b/kotlinx-coroutines-core/common/src/channels/RendezvousChannel.kt @@ -4,6 +4,8 @@ package kotlinx.coroutines.channels +import kotlinx.coroutines.internal.* + /** * Rendezvous channel. This channel does not have any buffer at all. An element is transferred from sender * to receiver only when [send] and [receive] invocations meet in time (rendezvous), so [send] suspends @@ -13,7 +15,7 @@ package kotlinx.coroutines.channels * * This implementation is fully lock-free. **/ -internal open class RendezvousChannel : AbstractChannel() { +internal open class RendezvousChannel(onUndeliveredElement: OnUndeliveredElement?) : AbstractChannel(onUndeliveredElement) { protected final override val isBufferAlwaysEmpty: Boolean get() = true protected final override val isBufferEmpty: Boolean get() = true protected final override val isBufferAlwaysFull: Boolean get() = true diff --git a/kotlinx-coroutines-core/common/src/flow/Builders.kt b/kotlinx-coroutines-core/common/src/flow/Builders.kt index 8fd9ae76a4..8076d2f878 100644 --- a/kotlinx-coroutines-core/common/src/flow/Builders.kt +++ b/kotlinx-coroutines-core/common/src/flow/Builders.kt @@ -283,11 +283,12 @@ public fun channelFlow(@BuilderInference block: suspend ProducerScope.() * Adjacent applications of [callbackFlow], [flowOn], [buffer], [produceIn], and [broadcastIn] are * always fused so that only one properly configured channel is used for execution. * - * Example of usage: + * Example of usage that converts a multi-short callback API to a flow. + * For single-shot callbacks use [suspendCancellableCoroutine]. * * ``` * fun flowFrom(api: CallbackBasedApi): Flow = callbackFlow { - * val callback = object : Callback { // implementation of some callback interface + * val callback = object : Callback { // Implementation of some callback interface * override fun onNextValue(value: T) { * // To avoid blocking you can configure channel capacity using * // either buffer(Channel.CONFLATED) or buffer(Channel.UNLIMITED) to avoid overfill @@ -311,6 +312,10 @@ public fun channelFlow(@BuilderInference block: suspend ProducerScope.() * awaitClose { api.unregister(callback) } * } * ``` + * + * > The callback `register`/`unregister` methods provided by an external API must be thread-safe, because + * > `awaitClose` block can be called at any time due to asynchronous nature of cancellation, even + * > concurrently with the call of the callback. */ @ExperimentalCoroutinesApi public fun callbackFlow(@BuilderInference block: suspend ProducerScope.() -> Unit): Flow = CallbackFlowBuilder(block) diff --git a/kotlinx-coroutines-core/common/src/flow/Channels.kt b/kotlinx-coroutines-core/common/src/flow/Channels.kt index 2d3ef95aa1..c7b9b71f5b 100644 --- a/kotlinx-coroutines-core/common/src/flow/Channels.kt +++ b/kotlinx-coroutines-core/common/src/flow/Channels.kt @@ -20,6 +20,9 @@ import kotlinx.coroutines.flow.internal.unsafeFlow as flow * the channel afterwards. If you need to iterate over the channel without consuming it, * a regular `for` loop should be used instead. * + * Note, that emitting values from a channel into a flow is not atomic. A value that was received from the + * channel many not reach the flow collector if it was cancelled and will be lost. + * * This function provides a more efficient shorthand for `channel.consumeEach { value -> emit(value) }`. * See [consumeEach][ReceiveChannel.consumeEach]. */ diff --git a/kotlinx-coroutines-core/common/src/flow/operators/Context.kt b/kotlinx-coroutines-core/common/src/flow/operators/Context.kt index 010d781c02..cf9575b078 100644 --- a/kotlinx-coroutines-core/common/src/flow/operators/Context.kt +++ b/kotlinx-coroutines-core/common/src/flow/operators/Context.kt @@ -172,13 +172,17 @@ public fun Flow.conflate(): Flow = buffer(CONFLATED) * * For more explanation of context preservation please refer to [Flow] documentation. * - * This operators retains a _sequential_ nature of flow if changing the context does not call for changing + * This operator retains a _sequential_ nature of flow if changing the context does not call for changing * the [dispatcher][CoroutineDispatcher]. Otherwise, if changing dispatcher is required, it collects * flow emissions in one coroutine that is run using a specified [context] and emits them from another coroutines * with the original collector's context using a channel with a [default][Channel.BUFFERED] buffer size * between two coroutines similarly to [buffer] operator, unless [buffer] operator is explicitly called * before or after `flowOn`, which requests buffering behavior and specifies channel size. * + * Note, that flows operating across different dispatchers might lose some in-flight elements when cancelled. + * In particular, this operator ensures that downstream flow does not resume on cancellation even if the element + * was already emitted by the upstream flow. + * * ### Operator fusion * * Adjacent applications of [channelFlow], [flowOn], [buffer], [produceIn], and [broadcastIn] are diff --git a/kotlinx-coroutines-core/common/src/internal/Atomic.kt b/kotlinx-coroutines-core/common/src/internal/Atomic.kt index 94f6ab9cf2..a27d5491d1 100644 --- a/kotlinx-coroutines-core/common/src/internal/Atomic.kt +++ b/kotlinx-coroutines-core/common/src/internal/Atomic.kt @@ -39,7 +39,8 @@ public abstract class OpDescriptor { } @SharedImmutable -private val NO_DECISION: Any = Symbol("NO_DECISION") +@JvmField +internal val NO_DECISION: Any = Symbol("NO_DECISION") /** * Descriptor for multi-word atomic operation. @@ -52,9 +53,13 @@ private val NO_DECISION: Any = Symbol("NO_DECISION") * * @suppress **This is unstable API and it is subject to change.** */ +@InternalCoroutinesApi public abstract class AtomicOp : OpDescriptor() { private val _consensus = atomic(NO_DECISION) + // Returns NO_DECISION when there is not decision yet + val consensus: Any? get() = _consensus.value + val isDecided: Boolean get() = _consensus.value !== NO_DECISION /** diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt index cf31fcf07d..b7b2954f6a 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt @@ -2,10 +2,10 @@ * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ -package kotlinx.coroutines +package kotlinx.coroutines.internal import kotlinx.atomicfu.* -import kotlinx.coroutines.internal.* +import kotlinx.coroutines.* import kotlin.coroutines.* import kotlin.jvm.* import kotlin.native.concurrent.* @@ -19,7 +19,7 @@ internal val REUSABLE_CLAIMED = Symbol("REUSABLE_CLAIMED") internal class DispatchedContinuation( @JvmField val dispatcher: CoroutineDispatcher, @JvmField val continuation: Continuation -) : DispatchedTask(MODE_ATOMIC_DEFAULT), CoroutineStackFrame, Continuation by continuation { +) : DispatchedTask(MODE_UNINITIALIZED), CoroutineStackFrame, Continuation by continuation { @JvmField @Suppress("PropertyName") internal var _state: Any? = UNDEFINED @@ -37,19 +37,19 @@ internal class DispatchedContinuation( * 3) [REUSABLE_CLAIMED]. CC is currently being reused and its owner executes `suspend` block: * ``` * // state == null | CC - * suspendAtomicCancellableCoroutineReusable { cont -> + * suspendCancellableCoroutineReusable { cont -> * // state == REUSABLE_CLAIMED * block(cont) * } * // state == CC * ``` - * 4) [Throwable] continuation was cancelled with this cause while being in [suspendAtomicCancellableCoroutineReusable], + * 4) [Throwable] continuation was cancelled with this cause while being in [suspendCancellableCoroutineReusable], * [CancellableContinuationImpl.getResult] will check for cancellation later. * * [REUSABLE_CLAIMED] state is required to prevent the lost resume in the channel. * AbstractChannel.receive method relies on the fact that the following pattern * ``` - * suspendAtomicCancellableCoroutineReusable { cont -> + * suspendCancellableCoroutineReusable { cont -> * val result = pollFastPath() * if (result != null) cont.resume(result) * } @@ -67,12 +67,12 @@ internal class DispatchedContinuation( /* * Reusability control: * `null` -> no reusability at all, false - * If current state is not CCI, then we are within `suspendAtomicCancellableCoroutineReusable`, true + * If current state is not CCI, then we are within `suspendCancellableCoroutineReusable`, true * Else, if result is CCI === requester. * Identity check my fail for the following pattern: * ``` * loop: - * suspendAtomicCancellableCoroutineReusable { } // Reusable, outer coroutine stores the child handle + * suspendCancellableCoroutineReusable { } // Reusable, outer coroutine stores the child handle * suspendCancellableCoroutine { } // **Not reusable**, handle should be disposed after {}, otherwise * it will leak because it won't be freed by `releaseInterceptedContinuation` * ``` @@ -83,7 +83,7 @@ internal class DispatchedContinuation( } /** - * Claims the continuation for [suspendAtomicCancellableCoroutineReusable] block, + * Claims the continuation for [suspendCancellableCoroutineReusable] block, * so all cancellations will be postponed. */ @Suppress("UNCHECKED_CAST") @@ -119,7 +119,7 @@ internal class DispatchedContinuation( * If continuation was cancelled, it becomes non-reusable. * * ``` - * suspendAtomicCancellableCoroutineReusable { // <- claimed + * suspendCancellableCoroutineReusable { // <- claimed * // Any asynchronous cancellation is "postponed" while this block * // is being executed * } // postponed cancellation is checked here in `getResult` @@ -180,10 +180,10 @@ internal class DispatchedContinuation( val state = result.toState() if (dispatcher.isDispatchNeeded(context)) { _state = state - resumeMode = MODE_ATOMIC_DEFAULT + resumeMode = MODE_ATOMIC dispatcher.dispatch(context, this) } else { - executeUnconfined(state, MODE_ATOMIC_DEFAULT) { + executeUnconfined(state, MODE_ATOMIC) { withCoroutineContext(this.context, countOrElement) { continuation.resumeWith(result) } @@ -194,29 +194,42 @@ internal class DispatchedContinuation( // We inline it to save an entry on the stack in cases where it shows (unconfined dispatcher) // It is used only in Continuation.resumeCancellableWith @Suppress("NOTHING_TO_INLINE") - inline fun resumeCancellableWith(result: Result) { - val state = result.toState() + inline fun resumeCancellableWith( + result: Result, + noinline onCancellation: ((cause: Throwable) -> Unit)? + ) { + val state = result.toState(onCancellation) if (dispatcher.isDispatchNeeded(context)) { _state = state resumeMode = MODE_CANCELLABLE dispatcher.dispatch(context, this) } else { executeUnconfined(state, MODE_CANCELLABLE) { - if (!resumeCancelled()) { + if (!resumeCancelled(state)) { resumeUndispatchedWith(result) } } } } + // takeState had already cleared the state so we cancel takenState here + override fun cancelCompletedResult(takenState: Any?, cause: Throwable) { + // It is Ok to call onCancellation here without try/catch around it, since this function only faces + // a "bound" cancellation handler that performs the safe call to the user-specified code. + if (takenState is CompletedWithCancellation) { + takenState.onCancellation(cause) + } + } + @Suppress("NOTHING_TO_INLINE") - inline fun resumeCancelled(): Boolean { + inline fun resumeCancelled(state: Any?): Boolean { val job = context[Job] if (job != null && !job.isActive) { - resumeWithException(job.getCancellationException()) + val cause = job.getCancellationException() + cancelCompletedResult(state, cause) + resumeWithException(cause) return true } - return false } @@ -245,8 +258,11 @@ internal class DispatchedContinuation( * @suppress **This an internal API and should not be used from general code.** */ @InternalCoroutinesApi -public fun Continuation.resumeCancellableWith(result: Result): Unit = when (this) { - is DispatchedContinuation -> resumeCancellableWith(result) +public fun Continuation.resumeCancellableWith( + result: Result, + onCancellation: ((cause: Throwable) -> Unit)? = null +): Unit = when (this) { + is DispatchedContinuation -> resumeCancellableWith(result, onCancellation) else -> resumeWith(result) } @@ -265,6 +281,7 @@ private inline fun DispatchedContinuation<*>.executeUnconfined( contState: Any?, mode: Int, doYield: Boolean = false, block: () -> Unit ): Boolean { + assert { mode != MODE_UNINITIALIZED } // invalid execution mode val eventLoop = ThreadLocalEventLoop.eventLoop // If we are yielding and unconfined queue is empty, we can bail out as part of fast path if (doYield && eventLoop.isUnconfinedQueueEmpty) return false diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt index 32258ba101..1f4942a358 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt @@ -8,12 +8,44 @@ import kotlinx.coroutines.internal.* import kotlin.coroutines.* import kotlin.jvm.* -@PublishedApi internal const val MODE_ATOMIC_DEFAULT = 0 // schedule non-cancellable dispatch for suspendCoroutine -@PublishedApi internal const val MODE_CANCELLABLE = 1 // schedule cancellable dispatch for suspendCancellableCoroutine -@PublishedApi internal const val MODE_UNDISPATCHED = 2 // when the thread is right, but need to mark it with current coroutine +/** + * Non-cancellable dispatch mode. + * + * **DO NOT CHANGE THE CONSTANT VALUE**. It might be inlined into legacy user code that was calling + * inline `suspendAtomicCancellableCoroutine` function and did not support reuse. + */ +internal const val MODE_ATOMIC = 0 + +/** + * Cancellable dispatch mode. It is used by user-facing [suspendCancellableCoroutine]. + * Note, that implementation of cancellability checks mode via [Int.isCancellableMode] extension. + * + * **DO NOT CHANGE THE CONSTANT VALUE**. It is being into the user code from [suspendCancellableCoroutine]. + */ +@PublishedApi +internal const val MODE_CANCELLABLE = 1 + +/** + * Cancellable dispatch mode for [suspendCancellableCoroutineReusable]. + * Note, that implementation of cancellability checks mode via [Int.isCancellableMode] extension; + * implementation of reuse checks mode via [Int.isReusableMode] extension. + */ +internal const val MODE_CANCELLABLE_REUSABLE = 2 + +/** + * Undispatched mode for [CancellableContinuation.resumeUndispatched]. + * It is used when the thread is right, but it needs to be marked with the current coroutine. + */ +internal const val MODE_UNDISPATCHED = 4 -internal val Int.isCancellableMode get() = this == MODE_CANCELLABLE -internal val Int.isDispatchedMode get() = this == MODE_ATOMIC_DEFAULT || this == MODE_CANCELLABLE +/** + * Initial mode for [DispatchedContinuation] implementation, should never be used for dispatch, because it is always + * overwritten when continuation is resumed with the actual resume mode. + */ +internal const val MODE_UNINITIALIZED = -1 + +internal val Int.isCancellableMode get() = this == MODE_CANCELLABLE || this == MODE_CANCELLABLE_REUSABLE +internal val Int.isReusableMode get() = this == MODE_CANCELLABLE_REUSABLE internal abstract class DispatchedTask( @JvmField public var resumeMode: Int @@ -22,16 +54,32 @@ internal abstract class DispatchedTask( internal abstract fun takeState(): Any? - internal open fun cancelResult(state: Any?, cause: Throwable) {} + /** + * Called when this task was cancelled while it was being dispatched. + */ + internal open fun cancelCompletedResult(takenState: Any?, cause: Throwable) {} + /** + * There are two implementations of `DispatchedTask`: + * * [DispatchedContinuation] keeps only simple values as successfully results. + * * [CancellableContinuationImpl] keeps additional data with values and overrides this method to unwrap it. + */ @Suppress("UNCHECKED_CAST") internal open fun getSuccessfulResult(state: Any?): T = state as T - internal fun getExceptionalResult(state: Any?): Throwable? = + /** + * There are two implementations of `DispatchedTask`: + * * [DispatchedContinuation] is just an intermediate storage that stores the exception that has its stack-trace + * properly recovered and is ready to pass to the [delegate] continuation directly. + * * [CancellableContinuationImpl] stores raw cause of the failure in its state; when it needs to be dispatched + * its stack-trace has to be recovered, so it overrides this method for that purpose. + */ + internal open fun getExceptionalResult(state: Any?): Throwable? = (state as? CompletedExceptionally)?.cause public final override fun run() { + assert { resumeMode != MODE_UNINITIALIZED } // should have been set before dispatching val taskContext = this.taskContext var fatalException: Throwable? = null try { @@ -41,19 +89,22 @@ internal abstract class DispatchedTask( val state = takeState() // NOTE: Must take state in any case, even if cancelled withCoroutineContext(context, delegate.countOrElement) { val exception = getExceptionalResult(state) - val job = if (resumeMode.isCancellableMode) context[Job] else null /* * Check whether continuation was originally resumed with an exception. * If so, it dominates cancellation, otherwise the original exception * will be silently lost. */ - if (exception == null && job != null && !job.isActive) { + val job = if (exception == null && resumeMode.isCancellableMode) context[Job] else null + if (job != null && !job.isActive) { val cause = job.getCancellationException() - cancelResult(state, cause) + cancelCompletedResult(state, cause) continuation.resumeWithStackTrace(cause) } else { - if (exception != null) continuation.resumeWithException(exception) - else continuation.resume(getSuccessfulResult(state)) + if (exception != null) { + continuation.resumeWithException(exception) + } else { + continuation.resume(getSuccessfulResult(state)) + } } } } catch (e: Throwable) { @@ -97,8 +148,10 @@ internal abstract class DispatchedTask( } internal fun DispatchedTask.dispatch(mode: Int) { + assert { mode != MODE_UNINITIALIZED } // invalid mode value for this method val delegate = this.delegate - if (mode.isDispatchedMode && delegate is DispatchedContinuation<*> && mode.isCancellableMode == resumeMode.isCancellableMode) { + val undispatched = mode == MODE_UNDISPATCHED + if (!undispatched && delegate is DispatchedContinuation<*> && mode.isCancellableMode == resumeMode.isCancellableMode) { // dispatch directly using this instance's Runnable implementation val dispatcher = delegate.dispatcher val context = delegate.context @@ -108,21 +161,21 @@ internal fun DispatchedTask.dispatch(mode: Int) { resumeUnconfined() } } else { - resume(delegate, mode) + // delegate is coming from 3rd-party interceptor implementation (and does not support cancellation) + // or undispatched mode was requested + resume(delegate, undispatched) } } @Suppress("UNCHECKED_CAST") -internal fun DispatchedTask.resume(delegate: Continuation, useMode: Int) { - // slow-path - use delegate +internal fun DispatchedTask.resume(delegate: Continuation, undispatched: Boolean) { + // This resume is never cancellable. The result is always delivered to delegate continuation. val state = takeState() - val exception = getExceptionalResult(state)?.let { recoverStackTrace(it, delegate) } + val exception = getExceptionalResult(state) val result = if (exception != null) Result.failure(exception) else Result.success(getSuccessfulResult(state)) - when (useMode) { - MODE_ATOMIC_DEFAULT -> delegate.resumeWith(result) - MODE_CANCELLABLE -> delegate.resumeCancellableWith(result) - MODE_UNDISPATCHED -> (delegate as DispatchedContinuation).resumeUndispatchedWith(result) - else -> error("Invalid mode $useMode") + when { + undispatched -> (delegate as DispatchedContinuation).resumeUndispatchedWith(result) + else -> delegate.resumeWith(result) } } @@ -134,7 +187,7 @@ private fun DispatchedTask<*>.resumeUnconfined() { } else { // Was not active -- run event loop until all unconfined tasks are executed runUnconfinedEventLoop(eventLoop) { - resume(delegate, MODE_UNDISPATCHED) + resume(delegate, undispatched = true) } } } diff --git a/kotlinx-coroutines-core/common/src/internal/LockFreeLinkedList.common.kt b/kotlinx-coroutines-core/common/src/internal/LockFreeLinkedList.common.kt index f1663c3ddc..8508e39239 100644 --- a/kotlinx-coroutines-core/common/src/internal/LockFreeLinkedList.common.kt +++ b/kotlinx-coroutines-core/common/src/internal/LockFreeLinkedList.common.kt @@ -73,6 +73,7 @@ public expect abstract class AbstractAtomicDesc : AtomicDesc { protected open fun retry(affected: LockFreeLinkedListNode, next: Any): Boolean public abstract fun finishPrepare(prepareOp: PrepareOp) // non-null on failure public open fun onPrepare(prepareOp: PrepareOp): Any? // non-null on failure + public open fun onRemoved(affected: LockFreeLinkedListNode) // non-null on failure protected abstract fun finishOnSuccess(affected: LockFreeLinkedListNode, next: LockFreeLinkedListNode) } diff --git a/kotlinx-coroutines-core/common/src/internal/OnUndeliveredElement.kt b/kotlinx-coroutines-core/common/src/internal/OnUndeliveredElement.kt new file mode 100644 index 0000000000..1744359e93 --- /dev/null +++ b/kotlinx-coroutines-core/common/src/internal/OnUndeliveredElement.kt @@ -0,0 +1,43 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.internal + +import kotlinx.coroutines.* +import kotlin.coroutines.* + +internal typealias OnUndeliveredElement = (E) -> Unit + +internal fun OnUndeliveredElement.callUndeliveredElementCatchingException( + element: E, + undeliveredElementException: UndeliveredElementException? = null +): UndeliveredElementException? { + try { + invoke(element) + } catch (ex: Throwable) { + // undeliveredElementException.cause !== ex is an optimization in case the same exception is thrown + // over and over again by on OnUndeliveredElement + if (undeliveredElementException != null && undeliveredElementException.cause !== ex) { + undeliveredElementException.addSuppressedThrowable(ex) + } else { + return UndeliveredElementException("Exception in undelivered element handler for $element", ex) + } + } + return undeliveredElementException +} + +internal fun OnUndeliveredElement.callUndeliveredElement(element: E, context: CoroutineContext) { + callUndeliveredElementCatchingException(element, null)?.let { ex -> + handleCoroutineException(context, ex) + } +} + +internal fun OnUndeliveredElement.bindCancellationFun(element: E, context: CoroutineContext): (Throwable) -> Unit = + { _: Throwable -> callUndeliveredElement(element, context) } + +/** + * Internal exception that is thrown when [OnUndeliveredElement] handler in + * a [kotlinx.coroutines.channels.Channel] throws an exception. + */ +internal class UndeliveredElementException(message: String, cause: Throwable) : RuntimeException(message, cause) diff --git a/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt b/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt index 1b1c389dc4..f814b152b2 100644 --- a/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt +++ b/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt @@ -5,6 +5,7 @@ package kotlinx.coroutines.intrinsics import kotlinx.coroutines.* +import kotlinx.coroutines.internal.* import kotlin.coroutines.* import kotlin.coroutines.intrinsics.* @@ -21,9 +22,12 @@ public fun (suspend () -> T).startCoroutineCancellable(completion: Continuat * Use this function to start coroutine in a cancellable way, so that it can be cancelled * while waiting to be dispatched. */ -internal fun (suspend (R) -> T).startCoroutineCancellable(receiver: R, completion: Continuation) = +internal fun (suspend (R) -> T).startCoroutineCancellable( + receiver: R, completion: Continuation, + onCancellation: ((cause: Throwable) -> Unit)? = null +) = runSafely(completion) { - createCoroutineUnintercepted(receiver, completion).intercepted().resumeCancellableWith(Result.success(Unit)) + createCoroutineUnintercepted(receiver, completion).intercepted().resumeCancellableWith(Result.success(Unit), onCancellation) } /** diff --git a/kotlinx-coroutines-core/common/src/selects/Select.kt b/kotlinx-coroutines-core/common/src/selects/Select.kt index 628d6f7aa5..99c54f8417 100644 --- a/kotlinx-coroutines-core/common/src/selects/Select.kt +++ b/kotlinx-coroutines-core/common/src/selects/Select.kt @@ -189,14 +189,8 @@ public interface SelectInstance { * * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this * function is suspended, this function immediately resumes with [CancellationException]. - * - * Atomicity of cancellation depends on the clause: [onSend][SendChannel.onSend], [onReceive][ReceiveChannel.onReceive], - * [onReceiveOrNull][ReceiveChannel.onReceiveOrNull], and [onLock][Mutex.onLock] clauses are - * *atomically cancellable*. When select throws [CancellationException] it means that those clauses had not performed - * their respective operations. - * As a side-effect of atomic cancellation, a thread-bound coroutine (to some UI thread, for example) may - * continue to execute even after it was cancelled from the same thread in the case when this select operation - * was already resumed on atomically cancellable clause and the continuation was posted for execution to the thread's queue. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. * * Note that this function does not check for cancellation when it is not suspended. * Use [yield] or [CoroutineScope.isActive] to periodically check for cancellation in tight loops if needed. diff --git a/kotlinx-coroutines-core/common/src/sync/Mutex.kt b/kotlinx-coroutines-core/common/src/sync/Mutex.kt index 61e046c77a..73aaab5fbf 100644 --- a/kotlinx-coroutines-core/common/src/sync/Mutex.kt +++ b/kotlinx-coroutines-core/common/src/sync/Mutex.kt @@ -45,12 +45,10 @@ public interface Mutex { * * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this * function is suspended, this function immediately resumes with [CancellationException]. - * - * *Cancellation of suspended lock invocation is atomic* -- when this function - * throws [CancellationException] it means that the mutex was not locked. - * As a side-effect of atomic cancellation, a thread-bound coroutine (to some UI thread, for example) may - * continue to execute even after it was cancelled from the same thread in the case when this lock operation - * was already resumed and the continuation was posted for execution to the thread's queue. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. + * This function releases the lock if it was already acquired by this function before the [CancellationException] + * was thrown. * * Note that this function does not check for cancellation when it is not suspended. * Use [yield] or [CoroutineScope.isActive] to periodically check for cancellation in tight loops if needed. @@ -124,8 +122,6 @@ public suspend inline fun Mutex.withLock(owner: Any? = null, action: () -> T @SharedImmutable private val LOCK_FAIL = Symbol("LOCK_FAIL") @SharedImmutable -private val ENQUEUE_FAIL = Symbol("ENQUEUE_FAIL") -@SharedImmutable private val UNLOCK_FAIL = Symbol("UNLOCK_FAIL") @SharedImmutable private val SELECT_SUCCESS = Symbol("SELECT_SUCCESS") @@ -194,7 +190,7 @@ internal class MutexImpl(locked: Boolean) : Mutex, SelectClause2 { return lockSuspend(owner) } - private suspend fun lockSuspend(owner: Any?) = suspendAtomicCancellableCoroutineReusable sc@ { cont -> + private suspend fun lockSuspend(owner: Any?) = suspendCancellableCoroutineReusable sc@ { cont -> val waiter = LockCont(owner, cont) _state.loop { state -> when (state) { @@ -254,7 +250,7 @@ internal class MutexImpl(locked: Boolean) : Mutex, SelectClause2 { } is LockedQueue -> { check(state.owner !== owner) { "Already locked by $owner" } - val node = LockSelect(owner, this, select, block) + val node = LockSelect(owner, select, block) if (state.addLastIf(node) { _state.value === state }) { // successfully enqueued select.disposeOnSelect(node) @@ -352,7 +348,7 @@ internal class MutexImpl(locked: Boolean) : Mutex, SelectClause2 { override fun toString(): String = "LockedQueue[$owner]" } - private abstract class LockWaiter( + private abstract inner class LockWaiter( @JvmField val owner: Any? ) : LockFreeLinkedListNode(), DisposableHandle { final override fun dispose() { remove() } @@ -360,27 +356,32 @@ internal class MutexImpl(locked: Boolean) : Mutex, SelectClause2 { abstract fun completeResumeLockWaiter(token: Any) } - private class LockCont( + private inner class LockCont( owner: Any?, @JvmField val cont: CancellableContinuation ) : LockWaiter(owner) { - override fun tryResumeLockWaiter() = cont.tryResume(Unit) + override fun tryResumeLockWaiter() = cont.tryResume(Unit, idempotent = null) { + // if this continuation gets cancelled during dispatch to the caller, then release the lock + unlock(owner) + } override fun completeResumeLockWaiter(token: Any) = cont.completeResume(token) - override fun toString(): String = "LockCont[$owner, $cont]" + override fun toString(): String = "LockCont[$owner, $cont] for ${this@MutexImpl}" } - private class LockSelect( + private inner class LockSelect( owner: Any?, - @JvmField val mutex: Mutex, @JvmField val select: SelectInstance, @JvmField val block: suspend (Mutex) -> R ) : LockWaiter(owner) { override fun tryResumeLockWaiter(): Any? = if (select.trySelect()) SELECT_SUCCESS else null override fun completeResumeLockWaiter(token: Any) { assert { token === SELECT_SUCCESS } - block.startCoroutine(receiver = mutex, completion = select.completion) + block.startCoroutineCancellable(receiver = this@MutexImpl, completion = select.completion) { + // if this continuation gets cancelled during dispatch to the caller, then release the lock + unlock(owner) + } } - override fun toString(): String = "LockSelect[$owner, $mutex, $select]" + override fun toString(): String = "LockSelect[$owner, $select] for ${this@MutexImpl}" } // atomic unlock operation that checks that waiters queue is empty diff --git a/kotlinx-coroutines-core/common/src/sync/Semaphore.kt b/kotlinx-coroutines-core/common/src/sync/Semaphore.kt index 27c976ce3f..84b7f4f8a2 100644 --- a/kotlinx-coroutines-core/common/src/sync/Semaphore.kt +++ b/kotlinx-coroutines-core/common/src/sync/Semaphore.kt @@ -33,9 +33,10 @@ public interface Semaphore { * * This suspending function is cancellable. If the [Job] of the current coroutine is cancelled or completed while this * function is suspended, this function immediately resumes with [CancellationException]. - * - * *Cancellation of suspended semaphore acquisition is atomic* -- when this function - * throws [CancellationException] it means that the semaphore was not acquired. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. + * This function releases the semaphore if it was already acquired by this function before the [CancellationException] + * was thrown. * * Note, that this function does not check for cancellation when it does not suspend. * Use [CoroutineScope.isActive] or [CoroutineScope.ensureActive] to periodically @@ -148,6 +149,8 @@ private class SemaphoreImpl(private val permits: Int, acquiredPermits: Int) : Se private val _availablePermits = atomic(permits - acquiredPermits) override val availablePermits: Int get() = max(_availablePermits.value, 0) + private val onCancellationRelease = { _: Throwable -> release() } + override fun tryAcquire(): Boolean { _availablePermits.loop { p -> if (p <= 0) return false @@ -164,7 +167,7 @@ private class SemaphoreImpl(private val permits: Int, acquiredPermits: Int) : Se acquireSlowPath() } - private suspend fun acquireSlowPath() = suspendAtomicCancellableCoroutineReusable sc@ { cont -> + private suspend fun acquireSlowPath() = suspendCancellableCoroutineReusable sc@ { cont -> while (true) { if (addAcquireToQueue(cont)) return@sc val p = _availablePermits.getAndDecrement() @@ -203,6 +206,8 @@ private class SemaphoreImpl(private val permits: Int, acquiredPermits: Int) : Se // On CAS failure -- the cell must be either PERMIT or BROKEN // If the cell already has PERMIT from tryResumeNextFromQueue, try to grab it if (segment.cas(i, PERMIT, TAKEN)) { // took permit thus eliminating acquire/release pair + // The following resume must always succeed, since continuation was not published yet and we don't have + // to pass onCancellationRelease handle, since the coroutine did not suspend yet and cannot be cancelled cont.resume(Unit) return true } @@ -232,15 +237,15 @@ private class SemaphoreImpl(private val permits: Int, acquiredPermits: Int) : Se return !segment.cas(i, PERMIT, BROKEN) } cellState === CANCELLED -> return false // the acquire was already cancelled - else -> return (cellState as CancellableContinuation).tryResume() + else -> return (cellState as CancellableContinuation).tryResumeAcquire() } } -} -private fun CancellableContinuation.tryResume(): Boolean { - val token = tryResume(Unit) ?: return false - completeResume(token) - return true + private fun CancellableContinuation.tryResumeAcquire(): Boolean { + val token = tryResume(Unit, null, onCancellationRelease) ?: return false + completeResume(token) + return true + } } private class CancelSemaphoreAcquisitionHandler( diff --git a/kotlinx-coroutines-core/common/test/AtomicCancellationCommonTest.kt b/kotlinx-coroutines-core/common/test/AtomicCancellationCommonTest.kt index a9f58dd6ee..c763faf225 100644 --- a/kotlinx-coroutines-core/common/test/AtomicCancellationCommonTest.kt +++ b/kotlinx-coroutines-core/common/test/AtomicCancellationCommonTest.kt @@ -87,23 +87,23 @@ class AtomicCancellationCommonTest : TestBase() { } @Test - fun testLockAtomicCancel() = runTest { + fun testLockCancellable() = runTest { expect(1) val mutex = Mutex(true) // locked mutex val job = launch(start = CoroutineStart.UNDISPATCHED) { expect(2) mutex.lock() // suspends - expect(4) // should execute despite cancellation + expectUnreached() // should NOT execute because of cancellation } expect(3) mutex.unlock() // unlock mutex first job.cancel() // cancel the job next yield() // now yield - finish(5) + finish(4) } @Test - fun testSelectLockAtomicCancel() = runTest { + fun testSelectLockCancellable() = runTest { expect(1) val mutex = Mutex(true) // locked mutex val job = launch(start = CoroutineStart.UNDISPATCHED) { @@ -114,13 +114,12 @@ class AtomicCancellationCommonTest : TestBase() { "OK" } } - assertEquals("OK", result) - expect(5) // should execute despite cancellation + expectUnreached() // should NOT execute because of cancellation } expect(3) mutex.unlock() // unlock mutex first job.cancel() // cancel the job next yield() // now yield - finish(6) + finish(4) } } \ No newline at end of file diff --git a/kotlinx-coroutines-core/common/test/CancellableContinuationHandlersTest.kt b/kotlinx-coroutines-core/common/test/CancellableContinuationHandlersTest.kt index 00f719e632..3c11182e00 100644 --- a/kotlinx-coroutines-core/common/test/CancellableContinuationHandlersTest.kt +++ b/kotlinx-coroutines-core/common/test/CancellableContinuationHandlersTest.kt @@ -23,10 +23,23 @@ class CancellableContinuationHandlersTest : TestBase() { fun testDoubleSubscriptionAfterCompletion() = runTest { suspendCancellableCoroutine { c -> c.resume(Unit) - // Nothing happened - c.invokeOnCancellation { expectUnreached() } - // Cannot validate after completion + // First invokeOnCancellation is Ok c.invokeOnCancellation { expectUnreached() } + // Second invokeOnCancellation is not allowed + assertFailsWith { c.invokeOnCancellation { expectUnreached() } } + } + } + + @Test + fun testDoubleSubscriptionAfterCompletionWithException() = runTest { + assertFailsWith { + suspendCancellableCoroutine { c -> + c.resumeWithException(TestException()) + // First invokeOnCancellation is Ok + c.invokeOnCancellation { expectUnreached() } + // Second invokeOnCancellation is not allowed + assertFailsWith { c.invokeOnCancellation { expectUnreached() } } + } } } @@ -46,6 +59,59 @@ class CancellableContinuationHandlersTest : TestBase() { } } + @Test + fun testSecondSubscriptionAfterCancellation() = runTest { + try { + suspendCancellableCoroutine { c -> + // Set IOC first + c.invokeOnCancellation { + assertNull(it) + expect(2) + } + expect(1) + // then cancel (it gets called) + c.cancel() + // then try to install another one + assertFailsWith { c.invokeOnCancellation { expectUnreached() } } + } + } catch (e: CancellationException) { + finish(3) + } + } + + @Test + fun testSecondSubscriptionAfterResumeCancelAndDispatch() = runTest { + var cont: CancellableContinuation? = null + val job = launch(start = CoroutineStart.UNDISPATCHED) { + // will be cancelled during dispatch + assertFailsWith { + suspendCancellableCoroutine { c -> + cont = c + // Set IOC first -- not called (completed) + c.invokeOnCancellation { + assertTrue(it is CancellationException) + expect(4) + } + expect(1) + } + } + expect(5) + } + expect(2) + // then resume it + cont!!.resume(Unit) // schedule cancelled continuation for dispatch + // then cancel the job during dispatch + job.cancel() + expect(3) + yield() // finish dispatching (will call IOC handler here!) + expect(6) + // then try to install another one after we've done dispatching it + assertFailsWith { + cont!!.invokeOnCancellation { expectUnreached() } + } + finish(7) + } + @Test fun testDoubleSubscriptionAfterCancellationWithCause() = runTest { try { diff --git a/kotlinx-coroutines-core/common/test/CancellableContinuationTest.kt b/kotlinx-coroutines-core/common/test/CancellableContinuationTest.kt index 38fc9ff281..f9f610ceb5 100644 --- a/kotlinx-coroutines-core/common/test/CancellableContinuationTest.kt +++ b/kotlinx-coroutines-core/common/test/CancellableContinuationTest.kt @@ -116,4 +116,26 @@ class CancellableContinuationTest : TestBase() { continuation!!.resume(Unit) // Should not fail finish(4) } + + @Test + fun testCompleteJobWhileSuspended() = runTest { + expect(1) + val completableJob = Job() + val coroutineBlock = suspend { + assertFailsWith { + suspendCancellableCoroutine { cont -> + expect(2) + assertSame(completableJob, cont.context[Job]) + completableJob.complete() + } + expectUnreached() + } + expect(3) + } + coroutineBlock.startCoroutine(Continuation(completableJob) { + assertEquals(Unit, it.getOrNull()) + expect(4) + }) + finish(5) + } } \ No newline at end of file diff --git a/kotlinx-coroutines-core/common/test/CancellableResumeTest.kt b/kotlinx-coroutines-core/common/test/CancellableResumeTest.kt index 39176a9a94..fbfa082555 100644 --- a/kotlinx-coroutines-core/common/test/CancellableResumeTest.kt +++ b/kotlinx-coroutines-core/common/test/CancellableResumeTest.kt @@ -1,5 +1,5 @@ /* - * Copyright 2016-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ @file:Suppress("NAMED_ARGUMENTS_NOT_ALLOWED") // KT-21913 @@ -44,6 +44,33 @@ class CancellableResumeTest : TestBase() { expectUnreached() } + @Test + fun testResumeImmediateAfterCancelWithHandlerFailure() = runTest( + expected = { it is TestException }, + unhandled = listOf( + { it is CompletionHandlerException && it.cause is TestException2 }, + { it is CompletionHandlerException && it.cause is TestException3 } + ) + ) { + expect(1) + suspendCancellableCoroutine { cont -> + expect(2) + cont.invokeOnCancellation { + expect(3) + throw TestException2("FAIL") // invokeOnCancellation handler fails with exception + } + cont.cancel(TestException("FAIL")) + expect(4) + cont.resume("OK") { cause -> + expect(5) + assertTrue(cause is TestException) + throw TestException3("FAIL") // onCancellation block fails with exception + } + finish(6) + } + expectUnreached() + } + @Test fun testResumeImmediateAfterIndirectCancel() = runTest( expected = { it is CancellationException } @@ -63,6 +90,33 @@ class CancellableResumeTest : TestBase() { expectUnreached() } + @Test + fun testResumeImmediateAfterIndirectCancelWithHandlerFailure() = runTest( + expected = { it is CancellationException }, + unhandled = listOf( + { it is CompletionHandlerException && it.cause is TestException2 }, + { it is CompletionHandlerException && it.cause is TestException3 } + ) + ) { + expect(1) + val ctx = coroutineContext + suspendCancellableCoroutine { cont -> + expect(2) + cont.invokeOnCancellation { + expect(3) + throw TestException2("FAIL") // invokeOnCancellation handler fails with exception + } + ctx.cancel() + expect(4) + cont.resume("OK") { cause -> + expect(5) + throw TestException3("FAIL") // onCancellation block fails with exception + } + finish(6) + } + expectUnreached() + } + @Test fun testResumeLaterNormally() = runTest { expect(1) @@ -110,7 +164,12 @@ class CancellableResumeTest : TestBase() { } @Test - fun testResumeCancelWhileDispatched() = runTest { + fun testResumeLaterAfterCancelWithHandlerFailure() = runTest( + unhandled = listOf( + { it is CompletionHandlerException && it.cause is TestException2 }, + { it is CompletionHandlerException && it.cause is TestException3 } + ) + ) { expect(1) lateinit var cc: CancellableContinuation val job = launch(start = CoroutineStart.UNDISPATCHED) { @@ -118,36 +177,117 @@ class CancellableResumeTest : TestBase() { try { suspendCancellableCoroutine { cont -> expect(3) - // resumed first, then cancelled, so no invokeOnCancellation call - cont.invokeOnCancellation { expectUnreached() } + cont.invokeOnCancellation { + expect(5) + throw TestException2("FAIL") // invokeOnCancellation handler fails with exception + } cc = cont } expectUnreached() } catch (e: CancellationException) { - expect(8) + finish(9) } } expect(4) + job.cancel(TestCancellationException()) + expect(6) cc.resume("OK") { cause -> expect(7) assertTrue(cause is TestCancellationException) + throw TestException3("FAIL") // onCancellation block fails with exception + } + expect(8) + } + + @Test + fun testResumeCancelWhileDispatched() = runTest { + expect(1) + lateinit var cc: CancellableContinuation + val job = launch(start = CoroutineStart.UNDISPATCHED) { + expect(2) + try { + suspendCancellableCoroutine { cont -> + expect(3) + // resumed first, dispatched, then cancelled, but still got invokeOnCancellation call + cont.invokeOnCancellation { cause -> + // Note: invokeOnCancellation is called before cc.resume(value) { ... } handler + expect(7) + assertTrue(cause is TestCancellationException) + } + cc = cont + } + expectUnreached() + } catch (e: CancellationException) { + expect(9) + } + } + expect(4) + cc.resume("OK") { cause -> + // Note: this handler is called after invokeOnCancellation handler + expect(8) + assertTrue(cause is TestCancellationException) } expect(5) job.cancel(TestCancellationException()) // cancel while execution is dispatched expect(6) yield() // to coroutine -- throws cancellation exception - finish(9) + finish(10) } + @Test + fun testResumeCancelWhileDispatchedWithHandlerFailure() = runTest( + unhandled = listOf( + { it is CompletionHandlerException && it.cause is TestException2 }, + { it is CompletionHandlerException && it.cause is TestException3 } + ) + ) { + expect(1) + lateinit var cc: CancellableContinuation + val job = launch(start = CoroutineStart.UNDISPATCHED) { + expect(2) + try { + suspendCancellableCoroutine { cont -> + expect(3) + // resumed first, dispatched, then cancelled, but still got invokeOnCancellation call + cont.invokeOnCancellation { cause -> + // Note: invokeOnCancellation is called before cc.resume(value) { ... } handler + expect(7) + assertTrue(cause is TestCancellationException) + throw TestException2("FAIL") // invokeOnCancellation handler fails with exception + } + cc = cont + } + expectUnreached() + } catch (e: CancellationException) { + expect(9) + } + } + expect(4) + cc.resume("OK") { cause -> + // Note: this handler is called after invokeOnCancellation handler + expect(8) + assertTrue(cause is TestCancellationException) + throw TestException3("FAIL") // onCancellation block fails with exception + } + expect(5) + job.cancel(TestCancellationException()) // cancel while execution is dispatched + expect(6) + yield() // to coroutine -- throws cancellation exception + finish(10) + } @Test fun testResumeUnconfined() = runTest { val outerScope = this withContext(Dispatchers.Unconfined) { val result = suspendCancellableCoroutine { - outerScope.launch { it.resume("OK", {}) } + outerScope.launch { + it.resume("OK") { + expectUnreached() + } + } } assertEquals("OK", result) } } -} \ No newline at end of file +} diff --git a/kotlinx-coroutines-core/common/test/DispatchedContinuationTest.kt b/kotlinx-coroutines-core/common/test/DispatchedContinuationTest.kt new file mode 100644 index 0000000000..b69eb22e17 --- /dev/null +++ b/kotlinx-coroutines-core/common/test/DispatchedContinuationTest.kt @@ -0,0 +1,78 @@ +/* + * Copyright 2016-2020 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.* + +/** + * When using [suspendCoroutine] from the standard library the continuation must be dispatched atomically, + * without checking for cancellation at any point in time. + */ +class DispatchedContinuationTest : TestBase() { + private lateinit var cont: Continuation + + @Test + fun testCancelThenResume() = runTest { + expect(1) + launch(start = CoroutineStart.UNDISPATCHED) { + expect(2) + coroutineContext[Job]!!.cancel() + // a regular suspendCoroutine will still suspend despite the fact that coroutine was cancelled + val value = suspendCoroutine { + expect(3) + cont = it + } + expect(6) + assertEquals("OK", value) + } + expect(4) + cont.resume("OK") + expect(5) + yield() // to the launched job + finish(7) + } + + @Test + fun testCancelThenResumeUnconfined() = runTest { + expect(1) + launch(Dispatchers.Unconfined) { + expect(2) + coroutineContext[Job]!!.cancel() + // a regular suspendCoroutine will still suspend despite the fact that coroutine was cancelled + val value = suspendCoroutine { + expect(3) + cont = it + } + expect(5) + assertEquals("OK", value) + } + expect(4) + cont.resume("OK") // immediately resumes -- because unconfined + finish(6) + } + + @Test + fun testResumeThenCancel() = runTest { + expect(1) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + expect(2) + val value = suspendCoroutine { + expect(3) + cont = it + } + expect(7) + assertEquals("OK", value) + } + expect(4) + cont.resume("OK") + expect(5) + // now cancel the job, which the coroutine is waiting to be dispatched + job.cancel() + expect(6) + yield() // to the launched job + finish(8) + } +} \ No newline at end of file diff --git a/kotlinx-coroutines-core/common/test/channels/BasicOperationsTest.kt b/kotlinx-coroutines-core/common/test/channels/BasicOperationsTest.kt index a6ddd81185..91d941b32c 100644 --- a/kotlinx-coroutines-core/common/test/channels/BasicOperationsTest.kt +++ b/kotlinx-coroutines-core/common/test/channels/BasicOperationsTest.kt @@ -42,7 +42,7 @@ class BasicOperationsTest : TestBase() { @Test fun testInvokeOnClose() = TestChannelKind.values().forEach { kind -> reset() - val channel = kind.create() + val channel = kind.create() channel.invokeOnClose { if (it is AssertionError) { expect(3) @@ -59,7 +59,7 @@ class BasicOperationsTest : TestBase() { fun testInvokeOnClosed() = TestChannelKind.values().forEach { kind -> reset() expect(1) - val channel = kind.create() + val channel = kind.create() channel.close() channel.invokeOnClose { expect(2) } assertFailsWith { channel.invokeOnClose { expect(3) } } @@ -69,7 +69,7 @@ class BasicOperationsTest : TestBase() { @Test fun testMultipleInvokeOnClose() = TestChannelKind.values().forEach { kind -> reset() - val channel = kind.create() + val channel = kind.create() channel.invokeOnClose { expect(3) } expect(1) assertFailsWith { channel.invokeOnClose { expect(4) } } @@ -81,7 +81,7 @@ class BasicOperationsTest : TestBase() { @Test fun testIterator() = runTest { TestChannelKind.values().forEach { kind -> - val channel = kind.create() + val channel = kind.create() val iterator = channel.iterator() assertFailsWith { iterator.next() } channel.close() @@ -91,7 +91,7 @@ class BasicOperationsTest : TestBase() { } private suspend fun testReceiveOrNull(kind: TestChannelKind) = coroutineScope { - val channel = kind.create() + val channel = kind.create() val d = async(NonCancellable) { channel.receive() } @@ -108,7 +108,7 @@ class BasicOperationsTest : TestBase() { } private suspend fun testReceiveOrNullException(kind: TestChannelKind) = coroutineScope { - val channel = kind.create() + val channel = kind.create() val d = async(NonCancellable) { channel.receive() } @@ -132,7 +132,7 @@ class BasicOperationsTest : TestBase() { @Suppress("ReplaceAssertBooleanWithAssertEquality") private suspend fun testReceiveOrClosed(kind: TestChannelKind) = coroutineScope { reset() - val channel = kind.create() + val channel = kind.create() launch { expect(2) channel.send(1) @@ -159,7 +159,7 @@ class BasicOperationsTest : TestBase() { } private suspend fun testOffer(kind: TestChannelKind) = coroutineScope { - val channel = kind.create() + val channel = kind.create() val d = async { channel.send(42) } yield() channel.close() @@ -184,7 +184,7 @@ class BasicOperationsTest : TestBase() { private suspend fun testSendAfterClose(kind: TestChannelKind) { assertFailsWith { coroutineScope { - val channel = kind.create() + val channel = kind.create() channel.close() launch { @@ -195,7 +195,7 @@ class BasicOperationsTest : TestBase() { } private suspend fun testSendReceive(kind: TestChannelKind, iterations: Int) = coroutineScope { - val channel = kind.create() + val channel = kind.create() launch { repeat(iterations) { channel.send(it) } channel.close() diff --git a/kotlinx-coroutines-core/common/test/channels/BroadcastTest.kt b/kotlinx-coroutines-core/common/test/channels/BroadcastTest.kt index bb3142e54c..ab1a85d697 100644 --- a/kotlinx-coroutines-core/common/test/channels/BroadcastTest.kt +++ b/kotlinx-coroutines-core/common/test/channels/BroadcastTest.kt @@ -63,7 +63,7 @@ class BroadcastTest : TestBase() { val a = produce { expect(3) send("MSG") - expect(5) + expectUnreached() // is not executed, because send is cancelled } expect(2) yield() // to produce @@ -72,7 +72,7 @@ class BroadcastTest : TestBase() { expect(4) yield() // to abort produce assertTrue(a.isClosedForReceive) // the source channel was consumed - finish(6) + finish(5) } @Test diff --git a/kotlinx-coroutines-core/common/test/channels/ChannelUndeliveredElementFailureTest.kt b/kotlinx-coroutines-core/common/test/channels/ChannelUndeliveredElementFailureTest.kt new file mode 100644 index 0000000000..d2ef3d2691 --- /dev/null +++ b/kotlinx-coroutines-core/common/test/channels/ChannelUndeliveredElementFailureTest.kt @@ -0,0 +1,143 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.channels + +import kotlinx.coroutines.* +import kotlinx.coroutines.internal.* +import kotlinx.coroutines.selects.* +import kotlin.test.* + +/** + * Tests for failures inside `onUndeliveredElement` handler in [Channel]. + */ +class ChannelUndeliveredElementFailureTest : TestBase() { + private val item = "LOST" + private val onCancelFail: (String) -> Unit = { throw TestException(it) } + private val shouldBeUnhandled: List<(Throwable) -> Boolean> = listOf({ it.isElementCancelException() }) + + private fun Throwable.isElementCancelException() = + this is UndeliveredElementException && cause is TestException && cause!!.message == item + + @Test + fun testSendCancelledFail() = runTest(unhandled = shouldBeUnhandled) { + val channel = Channel(onUndeliveredElement = onCancelFail) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + channel.send(item) + expectUnreached() + } + job.cancel() + } + + @Test + fun testSendSelectCancelledFail() = runTest(unhandled = shouldBeUnhandled) { + val channel = Channel(onUndeliveredElement = onCancelFail) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + select { + channel.onSend(item) { + expectUnreached() + } + } + } + job.cancel() + } + + @Test + fun testReceiveCancelledFail() = runTest(unhandled = shouldBeUnhandled) { + val channel = Channel(onUndeliveredElement = onCancelFail) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + channel.receive() + expectUnreached() // will be cancelled before it dispatches + } + channel.send(item) + job.cancel() + } + + @Test + fun testReceiveSelectCancelledFail() = runTest(unhandled = shouldBeUnhandled) { + val channel = Channel(onUndeliveredElement = onCancelFail) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + select { + channel.onReceive { + expectUnreached() + } + } + expectUnreached() // will be cancelled before it dispatches + } + channel.send(item) + job.cancel() + } + + @Test + fun testReceiveOrNullCancelledFail() = runTest(unhandled = shouldBeUnhandled) { + val channel = Channel(onUndeliveredElement = onCancelFail) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + channel.receiveOrNull() + expectUnreached() // will be cancelled before it dispatches + } + channel.send(item) + job.cancel() + } + + @Test + fun testReceiveOrNullSelectCancelledFail() = runTest(unhandled = shouldBeUnhandled) { + val channel = Channel(onUndeliveredElement = onCancelFail) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + select { + channel.onReceiveOrNull { + expectUnreached() + } + } + expectUnreached() // will be cancelled before it dispatches + } + channel.send(item) + job.cancel() + } + + @Test + fun testReceiveOrClosedCancelledFail() = runTest(unhandled = shouldBeUnhandled) { + val channel = Channel(onUndeliveredElement = onCancelFail) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + channel.receiveOrClosed() + expectUnreached() // will be cancelled before it dispatches + } + channel.send(item) + job.cancel() + } + + @Test + fun testReceiveOrClosedSelectCancelledFail() = runTest(unhandled = shouldBeUnhandled) { + val channel = Channel(onUndeliveredElement = onCancelFail) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + select { + channel.onReceiveOrClosed { + expectUnreached() + } + } + expectUnreached() // will be cancelled before it dispatches + } + channel.send(item) + job.cancel() + } + + @Test + fun testHasNextCancelledFail() = runTest(unhandled = shouldBeUnhandled) { + val channel = Channel(onUndeliveredElement = onCancelFail) + val job = launch(start = CoroutineStart.UNDISPATCHED) { + channel.iterator().hasNext() + expectUnreached() // will be cancelled before it dispatches + } + channel.send(item) + job.cancel() + } + + @Test + fun testChannelCancelledFail() = runTest(expected = { it.isElementCancelException()}) { + val channel = Channel(1, onUndeliveredElement = onCancelFail) + channel.send(item) + channel.cancel() + expectUnreached() + } + +} \ No newline at end of file diff --git a/kotlinx-coroutines-core/common/test/channels/ChannelUndeliveredElementTest.kt b/kotlinx-coroutines-core/common/test/channels/ChannelUndeliveredElementTest.kt new file mode 100644 index 0000000000..0391e00033 --- /dev/null +++ b/kotlinx-coroutines-core/common/test/channels/ChannelUndeliveredElementTest.kt @@ -0,0 +1,104 @@ +package kotlinx.coroutines.channels + +import kotlinx.atomicfu.* +import kotlinx.coroutines.* +import kotlin.test.* + +class ChannelUndeliveredElementTest : TestBase() { + @Test + fun testSendSuccessfully() = runAllKindsTest { kind -> + val channel = kind.create { it.cancel() } + val res = Resource("OK") + launch { + channel.send(res) + } + val ok = channel.receive() + assertEquals("OK", ok.value) + assertFalse(res.isCancelled) // was not cancelled + channel.close() + assertFalse(res.isCancelled) // still was not cancelled + } + + @Test + fun testRendezvousSendCancelled() = runTest { + val channel = Channel { it.cancel() } + val res = Resource("OK") + val sender = launch(start = CoroutineStart.UNDISPATCHED) { + assertFailsWith { + channel.send(res) // suspends & get cancelled + } + } + sender.cancelAndJoin() + assertTrue(res.isCancelled) + } + + @Test + fun testBufferedSendCancelled() = runTest { + val channel = Channel(1) { it.cancel() } + val resA = Resource("A") + val resB = Resource("B") + val sender = launch(start = CoroutineStart.UNDISPATCHED) { + channel.send(resA) // goes to buffer + assertFailsWith { + channel.send(resB) // suspends & get cancelled + } + } + sender.cancelAndJoin() + assertFalse(resA.isCancelled) // it is in buffer, not cancelled + assertTrue(resB.isCancelled) // send was cancelled + channel.cancel() // now cancel the channel + assertTrue(resA.isCancelled) // now cancelled in buffer + } + + @Test + fun testConflatedResourceCancelled() = runTest { + val channel = Channel(Channel.CONFLATED) { it.cancel() } + val resA = Resource("A") + val resB = Resource("B") + channel.send(resA) + assertFalse(resA.isCancelled) + assertFalse(resB.isCancelled) + channel.send(resB) + assertTrue(resA.isCancelled) // it was conflated (lost) and thus cancelled + assertFalse(resB.isCancelled) + channel.close() + assertFalse(resB.isCancelled) // not cancelled yet, can be still read by receiver + channel.cancel() + assertTrue(resB.isCancelled) // now it is cancelled + } + + @Test + fun testSendToClosedChannel() = runAllKindsTest { kind -> + val channel = kind.create { it.cancel() } + channel.close() // immediately close channel + val res = Resource("OK") + assertFailsWith { + channel.send(res) // send fails to closed channel, resource was not delivered + } + assertTrue(res.isCancelled) + } + + private fun runAllKindsTest(test: suspend CoroutineScope.(TestChannelKind) -> Unit) { + for (kind in TestChannelKind.values()) { + if (kind.viaBroadcast) continue // does not support onUndeliveredElement + try { + runTest { + test(kind) + } + } catch(e: Throwable) { + error("$kind: $e", e) + } + } + } + + private class Resource(val value: String) { + private val _cancelled = atomic(false) + + val isCancelled: Boolean + get() = _cancelled.value + + fun cancel() { + check(!_cancelled.getAndSet(true)) { "Already cancelled" } + } + } +} \ No newline at end of file diff --git a/kotlinx-coroutines-core/common/test/channels/ConflatedChannelTest.kt b/kotlinx-coroutines-core/common/test/channels/ConflatedChannelTest.kt index 4deb3858f0..eca19458be 100644 --- a/kotlinx-coroutines-core/common/test/channels/ConflatedChannelTest.kt +++ b/kotlinx-coroutines-core/common/test/channels/ConflatedChannelTest.kt @@ -21,7 +21,7 @@ class ConflatedChannelTest : TestBase() { @Test fun testConflatedSend() = runTest { - val q = ConflatedChannel() + val q = Channel(Channel.CONFLATED) q.send(1) q.send(2) // shall conflated previously sent assertEquals(2, q.receiveOrNull()) diff --git a/kotlinx-coroutines-core/common/test/channels/TestChannelKind.kt b/kotlinx-coroutines-core/common/test/channels/TestChannelKind.kt index 69d8fd03e3..42330f1cae 100644 --- a/kotlinx-coroutines-core/common/test/channels/TestChannelKind.kt +++ b/kotlinx-coroutines-core/common/test/channels/TestChannelKind.kt @@ -7,9 +7,10 @@ package kotlinx.coroutines.channels import kotlinx.coroutines.* import kotlinx.coroutines.selects.* -enum class TestChannelKind(val capacity: Int, - private val description: String, - private val viaBroadcast: Boolean = false +enum class TestChannelKind( + val capacity: Int, + private val description: String, + val viaBroadcast: Boolean = false ) { RENDEZVOUS(0, "RendezvousChannel"), ARRAY_1(1, "ArrayChannel(1)"), @@ -22,8 +23,11 @@ enum class TestChannelKind(val capacity: Int, CONFLATED_BROADCAST(Channel.CONFLATED, "ConflatedBroadcastChannel", viaBroadcast = true) ; - fun create(): Channel = if (viaBroadcast) ChannelViaBroadcast(BroadcastChannel(capacity)) - else Channel(capacity) + fun create(onUndeliveredElement: ((T) -> Unit)? = null): Channel = when { + viaBroadcast && onUndeliveredElement != null -> error("Broadcast channels to do not support onUndeliveredElement") + viaBroadcast -> ChannelViaBroadcast(BroadcastChannel(capacity)) + else -> Channel(capacity, onUndeliveredElement) + } val isConflated get() = capacity == Channel.CONFLATED override fun toString(): String = description diff --git a/kotlinx-coroutines-core/common/test/flow/operators/CatchTest.kt b/kotlinx-coroutines-core/common/test/flow/operators/CatchTest.kt index 802ba1ef2f..eedfac2ea3 100644 --- a/kotlinx-coroutines-core/common/test/flow/operators/CatchTest.kt +++ b/kotlinx-coroutines-core/common/test/flow/operators/CatchTest.kt @@ -134,15 +134,14 @@ class CatchTest : TestBase() { // flowOn with a different dispatcher introduces asynchrony so that all exceptions in the // upstream flows are handled before they go downstream .onEach { value -> - expect(8) - assertEquals("OK", value) + expectUnreached() // already cancelled } .catch { e -> - expect(9) + expect(8) assertTrue(e is TestException) assertSame(d0, kotlin.coroutines.coroutineContext[ContinuationInterceptor] as CoroutineContext) } .collect() - finish(10) + finish(9) } } diff --git a/kotlinx-coroutines-core/common/test/flow/operators/CombineTest.kt b/kotlinx-coroutines-core/common/test/flow/operators/CombineTest.kt index a619355b68..2893321998 100644 --- a/kotlinx-coroutines-core/common/test/flow/operators/CombineTest.kt +++ b/kotlinx-coroutines-core/common/test/flow/operators/CombineTest.kt @@ -1,5 +1,5 @@ /* - * Copyright 2016-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ package kotlinx.coroutines.flow @@ -238,6 +238,22 @@ abstract class CombineTestBase : TestBase() { assertFailsWith(flow) finish(7) } + + @Test + fun testCancelledCombine() = runTest( + expected = { it is CancellationException } + ) { + coroutineScope { + val flow = flow { + emit(Unit) // emit + } + cancel() // cancel the scope + flow.combineLatest(flow) { u, _ -> u }.collect { + // should not be reached, because cancelled before it runs + expectUnreached() + } + } + } } class CombineTest : CombineTestBase() { diff --git a/kotlinx-coroutines-core/common/test/flow/operators/FlowOnTest.kt b/kotlinx-coroutines-core/common/test/flow/operators/FlowOnTest.kt index f8350ff584..0eae1a3860 100644 --- a/kotlinx-coroutines-core/common/test/flow/operators/FlowOnTest.kt +++ b/kotlinx-coroutines-core/common/test/flow/operators/FlowOnTest.kt @@ -341,4 +341,19 @@ class FlowOnTest : TestBase() { assertEquals(expected, value) } } + + @Test + fun testCancelledFlowOn() = runTest { + assertFailsWith { + coroutineScope { + flow { + emit(Unit) // emit to buffer + cancel() // now cancel + }.flowOn(wrapperDispatcher()).collect { + // should not be reached, because cancelled before it runs + expectUnreached() + } + } + } + } } diff --git a/kotlinx-coroutines-core/common/test/flow/operators/ZipTest.kt b/kotlinx-coroutines-core/common/test/flow/operators/ZipTest.kt index b28320c391..5f2b5a74cd 100644 --- a/kotlinx-coroutines-core/common/test/flow/operators/ZipTest.kt +++ b/kotlinx-coroutines-core/common/test/flow/operators/ZipTest.kt @@ -67,14 +67,12 @@ class ZipTest : TestBase() { val f1 = flow { emit("1") emit("2") - hang { - expect(1) - } + expectUnreached() // the above emit will get cancelled because f2 ends } val f2 = flowOf("a", "b") assertEquals(listOf("1a", "2b"), f1.zip(f2) { s1, s2 -> s1 + s2 }.toList()) - finish(2) + finish(1) } @Test @@ -92,25 +90,6 @@ class ZipTest : TestBase() { finish(2) } - @Test - fun testCancelWhenFlowIsDone2() = runTest { - val f1 = flow { - emit("1") - emit("2") - try { - emit("3") - expectUnreached() - } finally { - expect(1) - } - - } - - val f2 = flowOf("a", "b") - assertEquals(listOf("1a", "2b"), f1.zip(f2) { s1, s2 -> s1 + s2 }.toList()) - finish(2) - } - @Test fun testContextIsIsolatedReversed() = runTest { val f1 = flow { diff --git a/kotlinx-coroutines-core/common/test/selects/SelectLoopTest.kt b/kotlinx-coroutines-core/common/test/selects/SelectLoopTest.kt index 5af68f6be5..e31ccfc16d 100644 --- a/kotlinx-coroutines-core/common/test/selects/SelectLoopTest.kt +++ b/kotlinx-coroutines-core/common/test/selects/SelectLoopTest.kt @@ -24,19 +24,20 @@ class SelectLoopTest : TestBase() { expect(3) throw TestException() } - var isDone = false - while (!isDone) { - select { - channel.onReceiveOrNull { - expect(4) - assertEquals(Unit, it) - } - job.onJoin { - expect(5) - isDone = true + try { + while (true) { + select { + channel.onReceiveOrNull { + expectUnreached() + } + job.onJoin { + expectUnreached() + } } } + } catch (e: CancellationException) { + // select will get cancelled because of the failure of job + finish(4) } - finish(6) } } \ No newline at end of file diff --git a/kotlinx-coroutines-core/js/src/Promise.kt b/kotlinx-coroutines-core/js/src/Promise.kt index 6c3de76426..ab2003236a 100644 --- a/kotlinx-coroutines-core/js/src/Promise.kt +++ b/kotlinx-coroutines-core/js/src/Promise.kt @@ -62,6 +62,8 @@ public fun Promise.asDeferred(): Deferred { * This suspending function is cancellable. * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, this function * stops waiting for the promise and immediately resumes with [CancellationException]. + * There is a **prompt cancellation guarantee**. If the job was cancelled while this function was + * suspended, it will not resume successfully. See [suspendCancellableCoroutine] documentation for low-level details. */ public suspend fun Promise.await(): T = suspendCancellableCoroutine { cont: CancellableContinuation -> this@await.then( diff --git a/kotlinx-coroutines-core/js/src/internal/LinkedList.kt b/kotlinx-coroutines-core/js/src/internal/LinkedList.kt index 342b11c69a..b69850576e 100644 --- a/kotlinx-coroutines-core/js/src/internal/LinkedList.kt +++ b/kotlinx-coroutines-core/js/src/internal/LinkedList.kt @@ -124,6 +124,8 @@ public actual abstract class AbstractAtomicDesc : AtomicDesc() { return null } + actual open fun onRemoved(affected: Node) {} + actual final override fun prepare(op: AtomicOp<*>): Any? { val affected = affectedNode val failure = failure(affected) diff --git a/kotlinx-coroutines-core/jvm/src/DebugStrings.kt b/kotlinx-coroutines-core/jvm/src/DebugStrings.kt index 184fb655e3..2ccfebc6d3 100644 --- a/kotlinx-coroutines-core/jvm/src/DebugStrings.kt +++ b/kotlinx-coroutines-core/jvm/src/DebugStrings.kt @@ -4,6 +4,7 @@ package kotlinx.coroutines +import kotlinx.coroutines.internal.* import kotlin.coroutines.* // internal debugging tools for string representation diff --git a/kotlinx-coroutines-core/jvm/src/internal/LockFreeLinkedList.kt b/kotlinx-coroutines-core/jvm/src/internal/LockFreeLinkedList.kt index 29f37dac28..97f9978139 100644 --- a/kotlinx-coroutines-core/jvm/src/internal/LockFreeLinkedList.kt +++ b/kotlinx-coroutines-core/jvm/src/internal/LockFreeLinkedList.kt @@ -416,20 +416,26 @@ public actual open class LockFreeLinkedListNode { val next = this.next val removed = next.removed() if (affected._next.compareAndSet(this, removed)) { + // The element was actually removed + desc.onRemoved(affected) // Complete removal operation here. It bails out if next node is also removed and it becomes // responsibility of the next's removes to call correctPrev which would help fix all the links. next.correctPrev(null) } return REMOVE_PREPARED } - val isDecided = if (decision != null) { + // We need to ensure progress even if it operation result consensus was already decided + val consensus = if (decision != null) { // some other logic failure, including RETRY_ATOMIC -- reach consensus on decision fail reason ASAP atomicOp.decide(decision) - true // atomicOp.isDecided will be true as a result } else { - atomicOp.isDecided // consult with current decision status like in Harris DCSS + atomicOp.consensus // consult with current decision status like in Harris DCSS + } + val update: Any = when { + consensus === NO_DECISION -> atomicOp // desc.onPrepare returned null -> start doing atomic op + consensus == null -> desc.updatedNext(affected, next) // move forward if consensus on success + else -> next // roll back if consensus if failure } - val update: Any = if (isDecided) next else atomicOp // restore if decision was already reached affected._next.compareAndSet(this, update) return null } @@ -445,9 +451,10 @@ public actual open class LockFreeLinkedListNode { protected open fun takeAffectedNode(op: OpDescriptor): Node? = affectedNode!! // null for RETRY_ATOMIC protected open fun failure(affected: Node): Any? = null // next: Node | Removed protected open fun retry(affected: Node, next: Any): Boolean = false // next: Node | Removed - protected abstract fun updatedNext(affected: Node, next: Node): Any protected abstract fun finishOnSuccess(affected: Node, next: Node) + public abstract fun updatedNext(affected: Node, next: Node): Any + public abstract fun finishPrepare(prepareOp: PrepareOp) // non-null on failure @@ -456,6 +463,8 @@ public actual open class LockFreeLinkedListNode { return null } + public open fun onRemoved(affected: Node) {} // called once when node was prepared & later removed + @Suppress("UNCHECKED_CAST") final override fun prepare(op: AtomicOp<*>): Any? { while (true) { // lock free loop on next diff --git a/kotlinx-coroutines-core/jvm/test/AtomicCancellationTest.kt b/kotlinx-coroutines-core/jvm/test/AtomicCancellationTest.kt index 8a7dce01ee..2612b84153 100644 --- a/kotlinx-coroutines-core/jvm/test/AtomicCancellationTest.kt +++ b/kotlinx-coroutines-core/jvm/test/AtomicCancellationTest.kt @@ -9,25 +9,24 @@ import kotlinx.coroutines.selects.* import kotlin.test.* class AtomicCancellationTest : TestBase() { - @Test - fun testSendAtomicCancel() = runBlocking { + fun testSendCancellable() = runBlocking { expect(1) val channel = Channel() val job = launch(start = CoroutineStart.UNDISPATCHED) { expect(2) channel.send(42) // suspends - expect(4) // should execute despite cancellation + expectUnreached() // should NOT execute because of cancellation } expect(3) assertEquals(42, channel.receive()) // will schedule sender for further execution job.cancel() // cancel the job next yield() // now yield - finish(5) + finish(4) } @Test - fun testSelectSendAtomicCancel() = runBlocking { + fun testSelectSendCancellable() = runBlocking { expect(1) val channel = Channel() val job = launch(start = CoroutineStart.UNDISPATCHED) { @@ -38,34 +37,33 @@ class AtomicCancellationTest : TestBase() { "OK" } } - assertEquals("OK", result) - expect(5) // should execute despite cancellation + expectUnreached() // should NOT execute because of cancellation } expect(3) assertEquals(42, channel.receive()) // will schedule sender for further execution job.cancel() // cancel the job next yield() // now yield - finish(6) + finish(4) } @Test - fun testReceiveAtomicCancel() = runBlocking { + fun testReceiveCancellable() = runBlocking { expect(1) val channel = Channel() val job = launch(start = CoroutineStart.UNDISPATCHED) { expect(2) assertEquals(42, channel.receive()) // suspends - expect(4) // should execute despite cancellation + expectUnreached() // should NOT execute because of cancellation } expect(3) channel.send(42) // will schedule receiver for further execution job.cancel() // cancel the job next yield() // now yield - finish(5) + finish(4) } @Test - fun testSelectReceiveAtomicCancel() = runBlocking { + fun testSelectReceiveCancellable() = runBlocking { expect(1) val channel = Channel() val job = launch(start = CoroutineStart.UNDISPATCHED) { @@ -77,14 +75,13 @@ class AtomicCancellationTest : TestBase() { "OK" } } - assertEquals("OK", result) - expect(5) // should execute despite cancellation + expectUnreached() // should NOT execute because of cancellation } expect(3) channel.send(42) // will schedule receiver for further execution job.cancel() // cancel the job next yield() // now yield - finish(6) + finish(4) } @Test diff --git a/kotlinx-coroutines-core/jvm/test/JobStructuredJoinStressTest.kt b/kotlinx-coroutines-core/jvm/test/JobStructuredJoinStressTest.kt index ec3635ca36..50d86f32be 100644 --- a/kotlinx-coroutines-core/jvm/test/JobStructuredJoinStressTest.kt +++ b/kotlinx-coroutines-core/jvm/test/JobStructuredJoinStressTest.kt @@ -5,6 +5,7 @@ package kotlinx.coroutines import org.junit.* +import kotlin.coroutines.* /** * Test a race between job failure and join. @@ -12,22 +13,52 @@ import org.junit.* * See [#1123](https://github.com/Kotlin/kotlinx.coroutines/issues/1123). */ class JobStructuredJoinStressTest : TestBase() { - private val nRepeats = 1_000 * stressTestMultiplier + private val nRepeats = 10_000 * stressTestMultiplier @Test - fun testStress() { - repeat(nRepeats) { + fun testStressRegularJoin() { + stress(Job::join) + } + + @Test + fun testStressSuspendCancellable() { + stress { job -> + suspendCancellableCoroutine { cont -> + job.invokeOnCompletion { cont.resume(Unit) } + } + } + } + + @Test + fun testStressSuspendCancellableReusable() { + stress { job -> + suspendCancellableCoroutineReusable { cont -> + job.invokeOnCompletion { cont.resume(Unit) } + } + } + } + + private fun stress(join: suspend (Job) -> Unit) { + expect(1) + repeat(nRepeats) { index -> assertFailsWith { runBlocking { // launch in background val job = launch(Dispatchers.Default) { throw TestException("OK") // crash } - assertFailsWith { - job.join() + try { + join(job) + error("Should not complete successfully") + } catch (e: CancellationException) { + // must always crash with cancellation exception + expect(2 + index) + } catch (e: Throwable) { + error("Unexpected exception", e) } } } } + finish(2 + nRepeats) } } \ No newline at end of file diff --git a/kotlinx-coroutines-core/jvm/test/ReusableCancellableContinuationTest.kt b/kotlinx-coroutines-core/jvm/test/ReusableCancellableContinuationTest.kt index 892a2a62d4..56f1e28313 100644 --- a/kotlinx-coroutines-core/jvm/test/ReusableCancellableContinuationTest.kt +++ b/kotlinx-coroutines-core/jvm/test/ReusableCancellableContinuationTest.kt @@ -11,15 +11,14 @@ import kotlin.coroutines.* import kotlin.test.* class ReusableCancellableContinuationTest : TestBase() { - @Test fun testReusable() = runTest { - testContinuationsCount(10, 1, ::suspendAtomicCancellableCoroutineReusable) + testContinuationsCount(10, 1, ::suspendCancellableCoroutineReusable) } @Test fun testRegular() = runTest { - testContinuationsCount(10, 10, ::suspendAtomicCancellableCoroutine) + testContinuationsCount(10, 10, ::suspendCancellableCoroutine) } private suspend inline fun CoroutineScope.testContinuationsCount( @@ -51,7 +50,7 @@ class ReusableCancellableContinuationTest : TestBase() { fun testCancelledOnClaimedCancel() = runTest { expect(1) try { - suspendAtomicCancellableCoroutineReusable { + suspendCancellableCoroutineReusable { it.cancel() } expectUnreached() @@ -65,7 +64,7 @@ class ReusableCancellableContinuationTest : TestBase() { expect(1) // Bind child at first var continuation: Continuation<*>? = null - suspendAtomicCancellableCoroutineReusable { + suspendCancellableCoroutineReusable { expect(2) continuation = it launch { // Attach to the parent, avoid fast path @@ -77,13 +76,16 @@ class ReusableCancellableContinuationTest : TestBase() { ensureActive() // Verify child was bound FieldWalker.assertReachableCount(1, coroutineContext[Job]) { it === continuation } - suspendAtomicCancellableCoroutineReusable { - expect(5) - coroutineContext[Job]!!.cancel() - it.resume(Unit) + try { + suspendCancellableCoroutineReusable { + expect(5) + coroutineContext[Job]!!.cancel() + it.resume(Unit) // will not dispatch, will get CancellationException + } + } catch (e: CancellationException) { + assertFalse(isActive) + finish(6) } - assertFalse(isActive) - finish(6) } @Test @@ -93,7 +95,7 @@ class ReusableCancellableContinuationTest : TestBase() { launch { cont!!.resumeWith(Result.success(Unit)) } - suspendAtomicCancellableCoroutineReusable { + suspendCancellableCoroutineReusable { cont = it } ensureActive() @@ -108,7 +110,7 @@ class ReusableCancellableContinuationTest : TestBase() { launch { // Attach to the parent, avoid fast path cont!!.resumeWith(Result.success(Unit)) } - suspendAtomicCancellableCoroutine { + suspendCancellableCoroutine { cont = it } ensureActive() @@ -121,7 +123,7 @@ class ReusableCancellableContinuationTest : TestBase() { expect(1) var cont: Continuation<*>? = null try { - suspendAtomicCancellableCoroutineReusable { + suspendCancellableCoroutineReusable { cont = it it.cancel() } @@ -137,7 +139,7 @@ class ReusableCancellableContinuationTest : TestBase() { val currentJob = coroutineContext[Job]!! expect(1) // Bind child at first - suspendAtomicCancellableCoroutineReusable { + suspendCancellableCoroutineReusable { expect(2) // Attach to the parent, avoid fast path launch { @@ -153,15 +155,23 @@ class ReusableCancellableContinuationTest : TestBase() { assertFalse(isActive) // Child detached FieldWalker.assertReachableCount(0, currentJob) { it is CancellableContinuation<*> } - suspendAtomicCancellableCoroutineReusable { it.resume(Unit) } - suspendAtomicCancellableCoroutineReusable { it.resume(Unit) } - FieldWalker.assertReachableCount(0, currentJob) { it is CancellableContinuation<*> } - + expect(5) + try { + // Resume is non-atomic, so it throws cancellation exception + suspendCancellableCoroutineReusable { + expect(6) // but the code inside the block is executed + it.resume(Unit) + } + } catch (e: CancellationException) { + FieldWalker.assertReachableCount(0, currentJob) { it is CancellableContinuation<*> } + expect(7) + } try { - suspendAtomicCancellableCoroutineReusable {} + // No resume -- still cancellation exception + suspendCancellableCoroutineReusable {} } catch (e: CancellationException) { FieldWalker.assertReachableCount(0, currentJob) { it is CancellableContinuation<*> } - finish(5) + finish(8) } } diff --git a/kotlinx-coroutines-core/jvm/test/TestBase.kt b/kotlinx-coroutines-core/jvm/test/TestBase.kt index bf462cc78f..26c176319e 100644 --- a/kotlinx-coroutines-core/jvm/test/TestBase.kt +++ b/kotlinx-coroutines-core/jvm/test/TestBase.kt @@ -69,6 +69,8 @@ public actual open class TestBase actual constructor() { throw makeError(message, cause) } + public fun hasError() = error.get() != null + private fun makeError(message: Any, cause: Throwable? = null): IllegalStateException = IllegalStateException(message.toString(), cause).also { setError(it) diff --git a/kotlinx-coroutines-core/jvm/test/channels/BroadcastChannelMultiReceiveStressTest.kt b/kotlinx-coroutines-core/jvm/test/channels/BroadcastChannelMultiReceiveStressTest.kt index 54ba7b639f..2e73b2432a 100644 --- a/kotlinx-coroutines-core/jvm/test/channels/BroadcastChannelMultiReceiveStressTest.kt +++ b/kotlinx-coroutines-core/jvm/test/channels/BroadcastChannelMultiReceiveStressTest.kt @@ -48,8 +48,9 @@ class BroadcastChannelMultiReceiveStressTest( launch(pool + CoroutineName("Sender")) { var i = 0L while (isActive) { - broadcast.send(++i) - sentTotal.set(i) // set sentTotal only if `send` was not cancelled + i++ + broadcast.send(i) // could be cancelled + sentTotal.set(i) // only was for it if it was not cancelled } } val receivers = mutableListOf() @@ -88,10 +89,8 @@ class BroadcastChannelMultiReceiveStressTest( try { withTimeout(5000) { receivers.forEachIndexed { index, receiver -> - if (lastReceived[index].get() == total) - receiver.cancel() - else - receiver.join() + if (lastReceived[index].get() >= total) receiver.cancel() + receiver.join() } } } catch (e: Exception) { @@ -112,7 +111,7 @@ class BroadcastChannelMultiReceiveStressTest( check(i == last + 1) { "Last was $last, got $i" } receivedTotal.incrementAndGet() lastReceived[receiverIndex].set(i) - return i == stopOnReceive.get() + return i >= stopOnReceive.get() } private suspend fun doReceive(channel: ReceiveChannel, receiverIndex: Int) { diff --git a/kotlinx-coroutines-core/jvm/test/channels/ChannelAtomicCancelStressTest.kt b/kotlinx-coroutines-core/jvm/test/channels/ChannelAtomicCancelStressTest.kt deleted file mode 100644 index 6556888a0f..0000000000 --- a/kotlinx-coroutines-core/jvm/test/channels/ChannelAtomicCancelStressTest.kt +++ /dev/null @@ -1,156 +0,0 @@ -/* - * Copyright 2016-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. - */ - -package kotlinx.coroutines.channels - -import kotlinx.coroutines.* -import kotlinx.coroutines.selects.* -import org.junit.After -import org.junit.Test -import org.junit.runner.* -import org.junit.runners.* -import kotlin.random.Random -import java.util.concurrent.atomic.* -import kotlin.test.* - -/** - * Tests cancel atomicity for channel send & receive operations, including their select versions. - */ -@RunWith(Parameterized::class) -class ChannelAtomicCancelStressTest(private val kind: TestChannelKind) : TestBase() { - companion object { - @Parameterized.Parameters(name = "{0}") - @JvmStatic - fun params(): Collection> = TestChannelKind.values().map { arrayOf(it) } - } - - private val TEST_DURATION = 1000L * stressTestMultiplier - - private val dispatcher = newFixedThreadPoolContext(2, "ChannelAtomicCancelStressTest") - private val scope = CoroutineScope(dispatcher) - - private val channel = kind.create() - private val senderDone = Channel(1) - private val receiverDone = Channel(1) - - private var lastSent = 0 - private var lastReceived = 0 - - private var stoppedSender = 0 - private var stoppedReceiver = 0 - - private var missedCnt = 0 - private var dupCnt = 0 - - val failed = AtomicReference() - - lateinit var sender: Job - lateinit var receiver: Job - - @After - fun tearDown() { - dispatcher.close() - } - - fun fail(e: Throwable) = failed.compareAndSet(null, e) - - private inline fun cancellable(done: Channel, block: () -> Unit) { - try { - block() - } finally { - if (!done.offer(true)) - fail(IllegalStateException("failed to offer to done channel")) - } - } - - @Test - fun testAtomicCancelStress() = runBlocking { - println("--- ChannelAtomicCancelStressTest $kind") - val deadline = System.currentTimeMillis() + TEST_DURATION - launchSender() - launchReceiver() - while (System.currentTimeMillis() < deadline && failed.get() == null) { - when (Random.nextInt(3)) { - 0 -> { // cancel & restart sender - stopSender() - launchSender() - } - 1 -> { // cancel & restart receiver - stopReceiver() - launchReceiver() - } - 2 -> yield() // just yield (burn a little time) - } - } - stopSender() - stopReceiver() - println(" Sent $lastSent ints to channel") - println(" Received $lastReceived ints from channel") - println(" Stopped sender $stoppedSender times") - println("Stopped receiver $stoppedReceiver times") - println(" Missed $missedCnt ints") - println(" Duplicated $dupCnt ints") - failed.get()?.let { throw it } - assertEquals(0, dupCnt) - if (!kind.isConflated) { - assertEquals(0, missedCnt) - assertEquals(lastSent, lastReceived) - } - } - - private fun launchSender() { - sender = scope.launch(start = CoroutineStart.ATOMIC) { - cancellable(senderDone) { - var counter = 0 - while (true) { - val trySend = lastSent + 1 - when (Random.nextInt(2)) { - 0 -> channel.send(trySend) - 1 -> select { channel.onSend(trySend) {} } - else -> error("cannot happen") - } - lastSent = trySend // update on success - when { - // must artificially slow down LINKED_LIST sender to avoid overwhelming receiver and going OOM - kind == TestChannelKind.LINKED_LIST -> while (lastSent > lastReceived + 100) yield() - // yield periodically to check cancellation on conflated channels - kind.isConflated -> if (counter++ % 100 == 0) yield() - } - } - } - } - } - - private suspend fun stopSender() { - stoppedSender++ - sender.cancel() - senderDone.receive() - } - - private fun launchReceiver() { - receiver = scope.launch(start = CoroutineStart.ATOMIC) { - cancellable(receiverDone) { - while (true) { - val received = when (Random.nextInt(2)) { - 0 -> channel.receive() - 1 -> select { channel.onReceive { it } } - else -> error("cannot happen") - } - val expected = lastReceived + 1 - if (received > expected) - missedCnt++ - if (received < expected) - dupCnt++ - lastReceived = received - } - } - } - } - - private suspend fun stopReceiver() { - stoppedReceiver++ - receiver.cancel() - receiverDone.receive() - } -} diff --git a/kotlinx-coroutines-core/jvm/test/channels/ChannelCancelUndeliveredElementStressTest.kt b/kotlinx-coroutines-core/jvm/test/channels/ChannelCancelUndeliveredElementStressTest.kt new file mode 100644 index 0000000000..76713aa173 --- /dev/null +++ b/kotlinx-coroutines-core/jvm/test/channels/ChannelCancelUndeliveredElementStressTest.kt @@ -0,0 +1,102 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.channels + +import kotlinx.coroutines.* +import kotlinx.coroutines.selects.* +import java.util.concurrent.atomic.* +import kotlin.random.* +import kotlin.test.* + +class ChannelCancelUndeliveredElementStressTest : TestBase() { + private val repeatTimes = 10_000 * stressTestMultiplier + + // total counters + private var sendCnt = 0 + private var offerFailedCnt = 0 + private var receivedCnt = 0 + private var undeliveredCnt = 0 + + // last operation + private var lastReceived = 0 + private var dSendCnt = 0 + private var dSendExceptionCnt = 0 + private var dOfferFailedCnt = 0 + private var dReceivedCnt = 0 + private val dUndeliveredCnt = AtomicInteger() + + @Test + fun testStress() = runTest { + repeat(repeatTimes) { + val channel = Channel(1) { dUndeliveredCnt.incrementAndGet() } + val j1 = launch(Dispatchers.Default) { + sendOne(channel) // send first + sendOne(channel) // send second + } + val j2 = launch(Dispatchers.Default) { + receiveOne(channel) // receive one element from the channel + channel.cancel() // cancel the channel + } + + joinAll(j1, j2) + + // All elements must be either received or undelivered (IN every run) + if (dSendCnt - dOfferFailedCnt != dReceivedCnt + dUndeliveredCnt.get()) { + println(" Send: $dSendCnt") + println("Send Exception: $dSendExceptionCnt") + println(" Offer failed: $dOfferFailedCnt") + println(" Received: $dReceivedCnt") + println(" Undelivered: ${dUndeliveredCnt.get()}") + error("Failed") + } + offerFailedCnt += dOfferFailedCnt + receivedCnt += dReceivedCnt + undeliveredCnt += dUndeliveredCnt.get() + // clear for next run + dSendCnt = 0 + dSendExceptionCnt = 0 + dOfferFailedCnt = 0 + dReceivedCnt = 0 + dUndeliveredCnt.set(0) + } + // Stats + println(" Send: $sendCnt") + println(" Offer failed: $offerFailedCnt") + println(" Received: $receivedCnt") + println(" Undelivered: $undeliveredCnt") + assertEquals(sendCnt - offerFailedCnt, receivedCnt + undeliveredCnt) + } + + private suspend fun sendOne(channel: Channel) { + dSendCnt++ + val i = ++sendCnt + try { + when (Random.nextInt(2)) { + 0 -> channel.send(i) + 1 -> if (!channel.offer(i)) { + dOfferFailedCnt++ + } + } + } catch(e: Throwable) { + assertTrue(e is CancellationException) // the only exception possible in this test + dSendExceptionCnt++ + throw e + } + } + + private suspend fun receiveOne(channel: Channel) { + val received = when (Random.nextInt(3)) { + 0 -> channel.receive() + 1 -> channel.receiveOrNull() ?: error("Cannot be closed yet") + 2 -> select { + channel.onReceive { it } + } + else -> error("Cannot happen") + } + assertTrue(received > lastReceived) + dReceivedCnt++ + lastReceived = received + } +} \ No newline at end of file diff --git a/kotlinx-coroutines-core/jvm/test/channels/ChannelSendReceiveStressTest.kt b/kotlinx-coroutines-core/jvm/test/channels/ChannelSendReceiveStressTest.kt index 00c5a6090f..f414c33338 100644 --- a/kotlinx-coroutines-core/jvm/test/channels/ChannelSendReceiveStressTest.kt +++ b/kotlinx-coroutines-core/jvm/test/channels/ChannelSendReceiveStressTest.kt @@ -35,7 +35,7 @@ class ChannelSendReceiveStressTest( private val maxBuffer = 10_000 // artificial limit for LinkedListChannel - val channel = kind.create() + val channel = kind.create() private val sendersCompleted = AtomicInteger() private val receiversCompleted = AtomicInteger() private val dupes = AtomicInteger() diff --git a/kotlinx-coroutines-core/jvm/test/channels/ChannelUndeliveredElementStressTest.kt b/kotlinx-coroutines-core/jvm/test/channels/ChannelUndeliveredElementStressTest.kt new file mode 100644 index 0000000000..1188329a4c --- /dev/null +++ b/kotlinx-coroutines-core/jvm/test/channels/ChannelUndeliveredElementStressTest.kt @@ -0,0 +1,255 @@ +/* + * Copyright 2016-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.channels + +import kotlinx.atomicfu.* +import kotlinx.coroutines.* +import kotlinx.coroutines.selects.* +import org.junit.After +import org.junit.Test +import org.junit.runner.* +import org.junit.runners.* +import kotlin.random.Random +import kotlin.test.* + +/** + * Tests resource transfer via channel send & receive operations, including their select versions, + * using `onUndeliveredElement` to detect lost resources and close them properly. + */ +@RunWith(Parameterized::class) +class ChannelUndeliveredElementStressTest(private val kind: TestChannelKind) : TestBase() { + companion object { + @Parameterized.Parameters(name = "{0}") + @JvmStatic + fun params(): Collection> = + TestChannelKind.values() + .filter { !it.viaBroadcast } + .map { arrayOf(it) } + } + + private val iterationDurationMs = 100L + private val testIterations = 20 * stressTestMultiplier // 2 sec + + private val dispatcher = newFixedThreadPoolContext(2, "ChannelAtomicCancelStressTest") + private val scope = CoroutineScope(dispatcher) + + private val channel = kind.create { it.failedToDeliver() } + private val senderDone = Channel(1) + private val receiverDone = Channel(1) + + @Volatile + private var lastReceived = -1L + + private var stoppedSender = 0L + private var stoppedReceiver = 0L + + private var sentCnt = 0L // total number of send attempts + private var receivedCnt = 0L // actually received successfully + private var dupCnt = 0L // duplicates (should never happen) + private val failedToDeliverCnt = atomic(0L) // out of sent + + private val modulo = 1 shl 25 + private val mask = (modulo - 1).toLong() + private val sentStatus = ItemStatus() // 1 - send norm, 2 - send select, +2 - did not throw exception + private val receivedStatus = ItemStatus() // 1-6 received + private val failedStatus = ItemStatus() // 1 - failed + + lateinit var sender: Job + lateinit var receiver: Job + + @After + fun tearDown() { + dispatcher.close() + } + + private inline fun cancellable(done: Channel, block: () -> Unit) { + try { + block() + } finally { + if (!done.offer(true)) + error(IllegalStateException("failed to offer to done channel")) + } + } + + @Test + fun testAtomicCancelStress() = runBlocking { + println("=== ChannelAtomicCancelStressTest $kind") + var nextIterationTime = System.currentTimeMillis() + iterationDurationMs + var iteration = 0 + launchSender() + launchReceiver() + while (!hasError()) { + if (System.currentTimeMillis() >= nextIterationTime) { + nextIterationTime += iterationDurationMs + iteration++ + verify(iteration) + if (iteration % 10 == 0) printProgressSummary(iteration) + if (iteration >= testIterations) break + launchSender() + launchReceiver() + } + when (Random.nextInt(3)) { + 0 -> { // cancel & restart sender + stopSender() + launchSender() + } + 1 -> { // cancel & restart receiver + stopReceiver() + launchReceiver() + } + 2 -> yield() // just yield (burn a little time) + } + } + } + + private suspend fun verify(iteration: Int) { + stopSender() + drainReceiver() + stopReceiver() + try { + assertEquals(0, dupCnt) + assertEquals(sentCnt - failedToDeliverCnt.value, receivedCnt) + } catch (e: Throwable) { + printProgressSummary(iteration) + printErrorDetails() + throw e + } + sentStatus.clear() + receivedStatus.clear() + failedStatus.clear() + } + + private fun printProgressSummary(iteration: Int) { + println("--- ChannelAtomicCancelStressTest $kind -- $iteration of $testIterations") + println(" Sent $sentCnt times to channel") + println(" Received $receivedCnt times from channel") + println(" Failed to deliver ${failedToDeliverCnt.value} times") + println(" Stopped sender $stoppedSender times") + println(" Stopped receiver $stoppedReceiver times") + println(" Duplicated $dupCnt deliveries") + } + + private fun printErrorDetails() { + val min = minOf(sentStatus.min, receivedStatus.min, failedStatus.min) + val max = maxOf(sentStatus.max, receivedStatus.max, failedStatus.max) + for (x in min..max) { + val sentCnt = if (sentStatus[x] != 0) 1 else 0 + val receivedCnt = if (receivedStatus[x] != 0) 1 else 0 + val failedToDeliverCnt = failedStatus[x] + if (sentCnt - failedToDeliverCnt != receivedCnt) { + println("!!! Error for value $x: " + + "sentStatus=${sentStatus[x]}, " + + "receivedStatus=${receivedStatus[x]}, " + + "failedStatus=${failedStatus[x]}" + ) + } + } + } + + + private fun launchSender() { + sender = scope.launch(start = CoroutineStart.ATOMIC) { + cancellable(senderDone) { + var counter = 0 + while (true) { + val trySendData = Data(sentCnt++) + val sendMode = Random.nextInt(2) + 1 + sentStatus[trySendData.x] = sendMode + when (sendMode) { + 1 -> channel.send(trySendData) + 2 -> select { channel.onSend(trySendData) {} } + else -> error("cannot happen") + } + sentStatus[trySendData.x] = sendMode + 2 + when { + // must artificially slow down LINKED_LIST sender to avoid overwhelming receiver and going OOM + kind == TestChannelKind.LINKED_LIST -> while (sentCnt > lastReceived + 100) yield() + // yield periodically to check cancellation on conflated channels + kind.isConflated -> if (counter++ % 100 == 0) yield() + } + } + } + } + } + + private suspend fun stopSender() { + stoppedSender++ + sender.cancel() + senderDone.receive() + } + + private fun launchReceiver() { + receiver = scope.launch(start = CoroutineStart.ATOMIC) { + cancellable(receiverDone) { + while (true) { + val receiveMode = Random.nextInt(6) + 1 + val receivedData = when (receiveMode) { + 1 -> channel.receive() + 2 -> select { channel.onReceive { it } } + 3 -> channel.receiveOrNull() ?: error("Should not be closed") + 4 -> select { channel.onReceiveOrNull { it ?: error("Should not be closed") } } + 5 -> channel.receiveOrClosed().value + 6 -> { + val iterator = channel.iterator() + check(iterator.hasNext()) { "Should not be closed" } + iterator.next() + } + else -> error("cannot happen") + } + receivedCnt++ + val received = receivedData.x + if (received <= lastReceived) + dupCnt++ + lastReceived = received + receivedStatus[received] = receiveMode + } + } + } + } + + private suspend fun drainReceiver() { + while (!channel.isEmpty) yield() // burn time until receiver gets it all + } + + private suspend fun stopReceiver() { + stoppedReceiver++ + receiver.cancel() + receiverDone.receive() + } + + private inner class Data(val x: Long) { + private val failedToDeliver = atomic(false) + + fun failedToDeliver() { + check(failedToDeliver.compareAndSet(false, true)) { "onUndeliveredElement notified twice" } + failedToDeliverCnt.incrementAndGet() + failedStatus[x] = 1 + } + } + + inner class ItemStatus { + private val a = ByteArray(modulo) + private val _min = atomic(Long.MAX_VALUE) + private val _max = atomic(-1L) + + val min: Long get() = _min.value + val max: Long get() = _max.value + + operator fun set(x: Long, value: Int) { + a[(x and mask).toInt()] = value.toByte() + _min.update { y -> minOf(x, y) } + _max.update { y -> maxOf(x, y) } + } + + operator fun get(x: Long): Int = a[(x and mask).toInt()].toInt() + + fun clear() { + if (_max.value < 0) return + for (x in _min.value.._max.value) a[(x and mask).toInt()] = 0 + _min.value = Long.MAX_VALUE + _max.value = -1L + } + } +} diff --git a/kotlinx-coroutines-core/jvm/test/channels/InvokeOnCloseStressTest.kt b/kotlinx-coroutines-core/jvm/test/channels/InvokeOnCloseStressTest.kt index 864a0b4c2e..888522c63c 100644 --- a/kotlinx-coroutines-core/jvm/test/channels/InvokeOnCloseStressTest.kt +++ b/kotlinx-coroutines-core/jvm/test/channels/InvokeOnCloseStressTest.kt @@ -39,7 +39,7 @@ class InvokeOnCloseStressTest : TestBase(), CoroutineScope { private suspend fun runStressTest(kind: TestChannelKind) { repeat(iterations) { val counter = AtomicInteger(0) - val channel = kind.create() + val channel = kind.create() val latch = CountDownLatch(1) val j1 = async { diff --git a/kotlinx-coroutines-core/jvm/test/channels/SimpleSendReceiveJvmTest.kt b/kotlinx-coroutines-core/jvm/test/channels/SimpleSendReceiveJvmTest.kt index 07c431bb4d..eeddfb5f49 100644 --- a/kotlinx-coroutines-core/jvm/test/channels/SimpleSendReceiveJvmTest.kt +++ b/kotlinx-coroutines-core/jvm/test/channels/SimpleSendReceiveJvmTest.kt @@ -28,7 +28,7 @@ class SimpleSendReceiveJvmTest( } } - val channel = kind.create() + val channel = kind.create() @Test fun testSimpleSendReceive() = runBlocking { diff --git a/kotlinx-coroutines-core/jvm/test/sync/MutexStressTest.kt b/kotlinx-coroutines-core/jvm/test/sync/MutexStressTest.kt index 8ecb8fd741..bb713b258d 100644 --- a/kotlinx-coroutines-core/jvm/test/sync/MutexStressTest.kt +++ b/kotlinx-coroutines-core/jvm/test/sync/MutexStressTest.kt @@ -5,6 +5,7 @@ package kotlinx.coroutines.sync import kotlinx.coroutines.* +import kotlinx.coroutines.selects.* import kotlin.test.* class MutexStressTest : TestBase() { @@ -26,4 +27,67 @@ class MutexStressTest : TestBase() { jobs.forEach { it.join() } assertEquals(n * k, shared) } + + @Test + fun stressUnlockCancelRace() = runTest { + val n = 10_000 * stressTestMultiplier + val mutex = Mutex(true) // create a locked mutex + newSingleThreadContext("SemaphoreStressTest").use { pool -> + repeat (n) { + // Initially, we hold the lock and no one else can `lock`, + // otherwise it's a bug. + assertTrue(mutex.isLocked) + var job1EnteredCriticalSection = false + val job1 = launch(start = CoroutineStart.UNDISPATCHED) { + mutex.lock() + job1EnteredCriticalSection = true + mutex.unlock() + } + // check that `job1` didn't finish the call to `acquire()` + assertEquals(false, job1EnteredCriticalSection) + val job2 = launch(pool) { + mutex.unlock() + } + // Because `job2` executes in a separate thread, this + // cancellation races with the call to `unlock()`. + job1.cancelAndJoin() + job2.join() + assertFalse(mutex.isLocked) + mutex.lock() + } + } + } + + @Test + fun stressUnlockCancelRaceWithSelect() = runTest { + val n = 10_000 * stressTestMultiplier + val mutex = Mutex(true) // create a locked mutex + newSingleThreadContext("SemaphoreStressTest").use { pool -> + repeat (n) { + // Initially, we hold the lock and no one else can `lock`, + // otherwise it's a bug. + assertTrue(mutex.isLocked) + var job1EnteredCriticalSection = false + val job1 = launch(start = CoroutineStart.UNDISPATCHED) { + select { + mutex.onLock { + job1EnteredCriticalSection = true + mutex.unlock() + } + } + } + // check that `job1` didn't finish the call to `acquire()` + assertEquals(false, job1EnteredCriticalSection) + val job2 = launch(pool) { + mutex.unlock() + } + // Because `job2` executes in a separate thread, this + // cancellation races with the call to `unlock()`. + job1.cancelAndJoin() + job2.join() + assertFalse(mutex.isLocked) + mutex.lock() + } + } + } } \ No newline at end of file diff --git a/kotlinx-coroutines-core/jvm/test/sync/SemaphoreStressTest.kt b/kotlinx-coroutines-core/jvm/test/sync/SemaphoreStressTest.kt index 9c77990862..374a1e3d7c 100644 --- a/kotlinx-coroutines-core/jvm/test/sync/SemaphoreStressTest.kt +++ b/kotlinx-coroutines-core/jvm/test/sync/SemaphoreStressTest.kt @@ -5,7 +5,6 @@ import org.junit.Test import kotlin.test.assertEquals class SemaphoreStressTest : TestBase() { - @Test fun stressTestAsMutex() = runBlocking(Dispatchers.Default) { val n = 10_000 * stressTestMultiplier @@ -71,14 +70,14 @@ class SemaphoreStressTest : TestBase() { // Initially, we hold the permit and no one else can `acquire`, // otherwise it's a bug. assertEquals(0, semaphore.availablePermits) - var job1_entered_critical_section = false + var job1EnteredCriticalSection = false val job1 = launch(start = CoroutineStart.UNDISPATCHED) { semaphore.acquire() - job1_entered_critical_section = true + job1EnteredCriticalSection = true semaphore.release() } // check that `job1` didn't finish the call to `acquire()` - assertEquals(false, job1_entered_critical_section) + assertEquals(false, job1EnteredCriticalSection) val job2 = launch(pool) { semaphore.release() } @@ -91,5 +90,4 @@ class SemaphoreStressTest : TestBase() { } } } - } diff --git a/kotlinx-coroutines-core/native/src/internal/LinkedList.kt b/kotlinx-coroutines-core/native/src/internal/LinkedList.kt index 9657830e35..99ab042f3c 100644 --- a/kotlinx-coroutines-core/native/src/internal/LinkedList.kt +++ b/kotlinx-coroutines-core/native/src/internal/LinkedList.kt @@ -124,6 +124,8 @@ public actual abstract class AbstractAtomicDesc : AtomicDesc() { return null } + actual open fun onRemoved(affected: Node) {} + actual final override fun prepare(op: AtomicOp<*>): Any? { val affected = affectedNode val failure = failure(affected) diff --git a/kotlinx-coroutines-debug/test/CoroutinesDumpTest.kt b/kotlinx-coroutines-debug/test/CoroutinesDumpTest.kt index 8507721e30..fd0279123f 100644 --- a/kotlinx-coroutines-debug/test/CoroutinesDumpTest.kt +++ b/kotlinx-coroutines-debug/test/CoroutinesDumpTest.kt @@ -115,16 +115,22 @@ class CoroutinesDumpTest : DebugTestBase() { coroutineThread!!.interrupt() val expected = - ("kotlin.coroutines.intrinsics.IntrinsicsKt__IntrinsicsJvmKt.createCoroutineUnintercepted(IntrinsicsJvm.kt:116)\n" + - "kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt:23)\n" + - "kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt:109)\n" + - "kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt:160)\n" + - "kotlinx.coroutines.BuildersKt__Builders_commonKt.async(Builders.common.kt:88)\n" + - "kotlinx.coroutines.BuildersKt.async(Unknown Source)\n" + - "kotlinx.coroutines.BuildersKt__Builders_commonKt.async\$default(Builders.common.kt:81)\n" + - "kotlinx.coroutines.BuildersKt.async\$default(Unknown Source)\n" + - "kotlinx.coroutines.debug.CoroutinesDumpTest\$testCreationStackTrace\$1.invokeSuspend(CoroutinesDumpTest.kt)").trimStackTrace() - assertTrue(result.startsWith(expected)) + "kotlin.coroutines.intrinsics.IntrinsicsKt__IntrinsicsJvmKt.createCoroutineUnintercepted(IntrinsicsJvm.kt)\n" + + "kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt)\n" + + "kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable\$default(Cancellable.kt)\n" + + "kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt)\n" + + "kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt)\n" + + "kotlinx.coroutines.BuildersKt__Builders_commonKt.async(Builders.common.kt)\n" + + "kotlinx.coroutines.BuildersKt.async(Unknown Source)\n" + + "kotlinx.coroutines.BuildersKt__Builders_commonKt.async\$default(Builders.common.kt)\n" + + "kotlinx.coroutines.BuildersKt.async\$default(Unknown Source)\n" + + "kotlinx.coroutines.debug.CoroutinesDumpTest\$testCreationStackTrace\$1.invokeSuspend(CoroutinesDumpTest.kt)" + if (!result.startsWith(expected)) { + println("=== Actual result") + println(result) + error("Does not start with expected lines") + } + } @Test diff --git a/kotlinx-coroutines-debug/test/DebugProbesTest.kt b/kotlinx-coroutines-debug/test/DebugProbesTest.kt index 24050e563c..3b32db3a5a 100644 --- a/kotlinx-coroutines-debug/test/DebugProbesTest.kt +++ b/kotlinx-coroutines-debug/test/DebugProbesTest.kt @@ -40,24 +40,25 @@ class DebugProbesTest : DebugTestBase() { val deferred = createDeferred() val traces = listOf( "java.util.concurrent.ExecutionException\n" + - "\tat kotlinx.coroutines.debug.DebugProbesTest\$createDeferred\$1.invokeSuspend(DebugProbesTest.kt:16)\n" + + "\tat kotlinx.coroutines.debug.DebugProbesTest\$createDeferred\$1.invokeSuspend(DebugProbesTest.kt)\n" + "\t(Coroutine boundary)\n" + "\tat kotlinx.coroutines.DeferredCoroutine.await\$suspendImpl(Builders.common.kt)\n" + - "\tat kotlinx.coroutines.debug.DebugProbesTest.oneMoreNestedMethod(DebugProbesTest.kt:71)\n" + - "\tat kotlinx.coroutines.debug.DebugProbesTest.nestedMethod(DebugProbesTest.kt:66)\n" + + "\tat kotlinx.coroutines.debug.DebugProbesTest.oneMoreNestedMethod(DebugProbesTest.kt)\n" + + "\tat kotlinx.coroutines.debug.DebugProbesTest.nestedMethod(DebugProbesTest.kt)\n" + "\t(Coroutine creation stacktrace)\n" + - "\tat kotlin.coroutines.intrinsics.IntrinsicsKt__IntrinsicsJvmKt.createCoroutineUnintercepted(IntrinsicsJvm.kt:116)\n" + - "\tat kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt:23)\n" + - "\tat kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt:99)\n" + - "\tat kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt:148)\n" + - "\tat kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:45)\n" + + "\tat kotlin.coroutines.intrinsics.IntrinsicsKt__IntrinsicsJvmKt.createCoroutineUnintercepted(IntrinsicsJvm.kt)\n" + + "\tat kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt)\n" + + "\tat kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable\$default(Cancellable.kt)\n" + + "\tat kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt)\n" + + "\tat kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt)\n" + + "\tat kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt)\n" + "\tat kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)\n" + - "\tat kotlinx.coroutines.TestBase.runTest(TestBase.kt:138)\n" + - "\tat kotlinx.coroutines.TestBase.runTest\$default(TestBase.kt:19)\n" + - "\tat kotlinx.coroutines.debug.DebugProbesTest.testAsyncWithProbes(DebugProbesTest.kt:38)", + "\tat kotlinx.coroutines.TestBase.runTest(TestBase.kt)\n" + + "\tat kotlinx.coroutines.TestBase.runTest\$default(TestBase.kt)\n" + + "\tat kotlinx.coroutines.debug.DebugProbesTest.testAsyncWithProbes(DebugProbesTest.kt)", "Caused by: java.util.concurrent.ExecutionException\n" + - "\tat kotlinx.coroutines.debug.DebugProbesTest\$createDeferred\$1.invokeSuspend(DebugProbesTest.kt:16)\n" + - "\tat kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:32)\n") + "\tat kotlinx.coroutines.debug.DebugProbesTest\$createDeferred\$1.invokeSuspend(DebugProbesTest.kt)\n" + + "\tat kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt)\n") nestedMethod(deferred, traces) deferred.join() } diff --git a/reactive/kotlinx-coroutines-reactive/src/Channel.kt b/reactive/kotlinx-coroutines-reactive/src/Channel.kt index 379fc4ed53..26f14ec63d 100644 --- a/reactive/kotlinx-coroutines-reactive/src/Channel.kt +++ b/reactive/kotlinx-coroutines-reactive/src/Channel.kt @@ -48,7 +48,7 @@ public suspend inline fun Publisher.collect(action: (T) -> Unit): Unit = @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER", "SubscriberImplementation") private class SubscriptionChannel( private val request: Int -) : LinkedListChannel(), Subscriber { +) : LinkedListChannel(null), Subscriber { init { require(request >= 0) { "Invalid request size: $request" } } diff --git a/reactive/kotlinx-coroutines-rx2/src/RxChannel.kt b/reactive/kotlinx-coroutines-rx2/src/RxChannel.kt index e253161db0..633693e756 100644 --- a/reactive/kotlinx-coroutines-rx2/src/RxChannel.kt +++ b/reactive/kotlinx-coroutines-rx2/src/RxChannel.kt @@ -64,7 +64,7 @@ public suspend inline fun ObservableSource.collect(action: (T) -> Unit): @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") private class SubscriptionChannel : - LinkedListChannel(), Observer, MaybeObserver + LinkedListChannel(null), Observer, MaybeObserver { private val _subscription = atomic(null) diff --git a/reactive/kotlinx-coroutines-rx3/src/RxChannel.kt b/reactive/kotlinx-coroutines-rx3/src/RxChannel.kt index acb907b765..737cf6710d 100644 --- a/reactive/kotlinx-coroutines-rx3/src/RxChannel.kt +++ b/reactive/kotlinx-coroutines-rx3/src/RxChannel.kt @@ -54,7 +54,7 @@ public suspend inline fun ObservableSource.collect(action: (T) -> Unit): @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") private class SubscriptionChannel : - LinkedListChannel(), Observer, MaybeObserver + LinkedListChannel(null), Observer, MaybeObserver { private val _subscription = atomic(null)