Skip to content

Commit 4ad776f

Browse files
committed
Ignore exception on cancel/fail race in CancellableContinuation
There seems to be no other way to satisfactory fix the problem of a race between cancellation on CancellationContinuation and concurrent failure of the corresponding background process in a way the distinguishes "failure from cancellation attempt" from a "true failure" without placing undue burden on anyone who is implementing `await()` extension function for various future types. - Added testTimeoutCancellationFailRace that works as a perfect litmus test; being allowed to run for a while it reliably detects various problems in alternative approaches. - Optimized CancellableContinuationImpl; merged its code with AbstractContinuation class that is not needed as a separate class and removed. - Deprecated and hidden CancellableContinuation.initCancellability(); it is now always invoked after the end of the block that was passed to suspendCancellableCoroutine. - Deprecated `holdCancellability` parameter to an internal suspendAtomicCancellableCoroutine function. Fixes #892 Fixes #830
1 parent 9ec881e commit 4ad776f

File tree

8 files changed

+208
-193
lines changed

8 files changed

+208
-193
lines changed

binary-compatibility-validator/reference-public-api/kotlinx-coroutines-core.txt

+14-3
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public final class kotlinx/coroutines/BuildersKt {
3434
public abstract interface class kotlinx/coroutines/CancellableContinuation : kotlin/coroutines/Continuation {
3535
public abstract fun cancel (Ljava/lang/Throwable;)Z
3636
public abstract fun completeResume (Ljava/lang/Object;)V
37-
public abstract fun initCancellability ()V
37+
public abstract synthetic fun initCancellability ()V
3838
public abstract fun invokeOnCancellation (Lkotlin/jvm/functions/Function1;)V
3939
public abstract fun isActive ()Z
4040
public abstract fun isCancelled ()Z
@@ -50,17 +50,28 @@ public final class kotlinx/coroutines/CancellableContinuation$DefaultImpls {
5050
public static synthetic fun tryResume$default (Lkotlinx/coroutines/CancellableContinuation;Ljava/lang/Object;Ljava/lang/Object;ILjava/lang/Object;)Ljava/lang/Object;
5151
}
5252

53-
public class kotlinx/coroutines/CancellableContinuationImpl : java/lang/Runnable, kotlin/coroutines/jvm/internal/CoroutineStackFrame, kotlinx/coroutines/CancellableContinuation {
53+
public class kotlinx/coroutines/CancellableContinuationImpl : kotlin/coroutines/jvm/internal/CoroutineStackFrame, kotlinx/coroutines/CancellableContinuation {
5454
public fun <init> (Lkotlin/coroutines/Continuation;I)V
55+
public fun cancel (Ljava/lang/Throwable;)Z
5556
public fun completeResume (Ljava/lang/Object;)V
5657
public fun getCallerFrame ()Lkotlin/coroutines/jvm/internal/CoroutineStackFrame;
5758
public fun getContext ()Lkotlin/coroutines/CoroutineContext;
59+
public fun getContinuationCancellationCause (Lkotlinx/coroutines/Job;)Ljava/lang/Throwable;
60+
public final fun getDelegate ()Lkotlin/coroutines/Continuation;
61+
public final fun getResult ()Ljava/lang/Object;
5862
public fun getStackTraceElement ()Ljava/lang/StackTraceElement;
5963
public fun getSuccessfulResult (Ljava/lang/Object;)Ljava/lang/Object;
60-
public fun initCancellability ()V
64+
public synthetic fun initCancellability ()V
65+
public fun invokeOnCancellation (Lkotlin/jvm/functions/Function1;)V
66+
public fun isActive ()Z
67+
public fun isCancelled ()Z
68+
public fun isCompleted ()Z
6169
protected fun nameString ()Ljava/lang/String;
6270
public fun resumeUndispatched (Lkotlinx/coroutines/CoroutineDispatcher;Ljava/lang/Object;)V
6371
public fun resumeUndispatchedWithException (Lkotlinx/coroutines/CoroutineDispatcher;Ljava/lang/Throwable;)V
72+
public fun resumeWith (Ljava/lang/Object;)V
73+
public fun takeState ()Ljava/lang/Object;
74+
public fun toString ()Ljava/lang/String;
6475
public fun tryResume (Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
6576
public fun tryResumeWithException (Ljava/lang/Throwable;)Ljava/lang/Object;
6677
}

common/kotlinx-coroutines-core-common/src/CancellableContinuation.kt

+27-82
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package kotlinx.coroutines
77
import kotlinx.coroutines.internal.*
88
import kotlin.coroutines.*
99
import kotlin.coroutines.intrinsics.*
10-
import kotlin.jvm.*
1110

1211
// --------------- cancellable continuations ---------------
1312

@@ -98,9 +97,17 @@ public interface CancellableContinuation<in T> : Continuation<T> {
9897
public fun completeResume(token: Any)
9998

10099
/**
101-
* Makes this continuation cancellable. Use it with `holdCancellability` optional parameter to
102-
* [suspendCancellableCoroutine] function. It throws [IllegalStateException] if invoked more than once.
100+
* Legacy function that turned on cancellation behavior in [suspendCancellableCoroutine] before kotlinx.coroutines 1.1.0.
101+
* This function does nothing and is left only for binary compatibility with old compiled code.
102+
*
103+
* @suppress **Deprecated**: This function is no longer used.
104+
* It is left for binary compatibility with code compiled before kotlinx.coroutines 1.1.0.
103105
*/
106+
@Deprecated(
107+
level = DeprecationLevel.HIDDEN,
108+
message = "This function is no longer used. " +
109+
"It is left for binary compatibility with code compiled before kotlinx.coroutines 1.1.0. "
110+
)
104111
@InternalCoroutinesApi
105112
public fun initCancellability()
106113

@@ -155,7 +162,9 @@ public suspend inline fun <T> suspendCancellableCoroutine(
155162
): T =
156163
suspendCoroutineUninterceptedOrReturn { uCont ->
157164
val cancellable = CancellableContinuationImpl(uCont.intercepted(), resumeMode = MODE_CANCELLABLE)
158-
cancellable.initCancellability()
165+
// NOTE: Before version 1.1.0 the following invocation was inlined here, so invocation of this
166+
// method indicates that the code was compiled by kotlinx.coroutines < 1.1.0
167+
// cancellable.initCancellability()
159168
block(cancellable)
160169
cancellable.getResult()
161170
}
@@ -172,16 +181,28 @@ public suspend inline fun <T> suspendCancellableCoroutine(
172181
*/
173182
@InternalCoroutinesApi
174183
public suspend inline fun <T> suspendAtomicCancellableCoroutine(
175-
holdCancellability: Boolean = false,
176184
crossinline block: (CancellableContinuation<T>) -> Unit
177185
): T =
178186
suspendCoroutineUninterceptedOrReturn { uCont ->
179187
val cancellable = CancellableContinuationImpl(uCont.intercepted(), resumeMode = MODE_ATOMIC_DEFAULT)
180-
if (!holdCancellability) cancellable.initCancellability()
181188
block(cancellable)
182189
cancellable.getResult()
183190
}
184191

192+
/**
193+
* @suppress **Deprecated**
194+
*/
195+
@Deprecated(
196+
message = "holdCancellability parameter is deprecated and is no longer used",
197+
replaceWith = ReplaceWith("suspendAtomicCancellableCoroutine(block)")
198+
)
199+
@InternalCoroutinesApi
200+
public suspend inline fun <T> suspendAtomicCancellableCoroutine(
201+
holdCancellability: Boolean = false,
202+
crossinline block: (CancellableContinuation<T>) -> Unit
203+
): T =
204+
suspendAtomicCancellableCoroutine(block)
205+
185206
/**
186207
* Removes a given node on cancellation.
187208
*/
@@ -213,79 +234,3 @@ private class DisposeOnCancel(private val handle: DisposableHandle) : CancelHand
213234
override fun invoke(cause: Throwable?) = handle.dispose()
214235
override fun toString(): String = "DisposeOnCancel[$handle]"
215236
}
216-
217-
@PublishedApi
218-
internal open class CancellableContinuationImpl<in T>(
219-
delegate: Continuation<T>,
220-
resumeMode: Int
221-
) : AbstractContinuation<T>(delegate, resumeMode), CancellableContinuation<T>, Runnable, CoroutineStackFrame {
222-
223-
public override val context: CoroutineContext = delegate.context
224-
225-
override val callerFrame: CoroutineStackFrame?
226-
get() = delegate as? CoroutineStackFrame
227-
228-
override fun getStackTraceElement(): StackTraceElement? = null
229-
230-
override fun initCancellability() {
231-
initParentJobInternal(delegate.context[Job])
232-
}
233-
234-
override fun tryResume(value: T, idempotent: Any?): Any? {
235-
loopOnState { state ->
236-
when (state) {
237-
is NotCompleted -> {
238-
val update: Any? = if (idempotent == null) value else
239-
CompletedIdempotentResult(idempotent, value, state)
240-
if (tryUpdateStateToFinal(state, update)) return state
241-
}
242-
is CompletedIdempotentResult -> {
243-
if (state.idempotentResume === idempotent) {
244-
check(state.result === value) { "Non-idempotent resume" }
245-
return state.token
246-
} else
247-
return null
248-
}
249-
else -> return null // cannot resume -- not active anymore
250-
}
251-
}
252-
}
253-
254-
override fun tryResumeWithException(exception: Throwable): Any? {
255-
loopOnState { state ->
256-
when (state) {
257-
is NotCompleted -> {
258-
if (tryUpdateStateToFinal(state, CompletedExceptionally(exception))) return state
259-
}
260-
else -> return null // cannot resume -- not active anymore
261-
}
262-
}
263-
}
264-
265-
override fun completeResume(token: Any) = completeStateUpdate(token as NotCompleted, state, resumeMode)
266-
267-
override fun CoroutineDispatcher.resumeUndispatched(value: T) {
268-
val dc = delegate as? DispatchedContinuation
269-
resumeImpl(value, if (dc?.dispatcher === this) MODE_UNDISPATCHED else resumeMode)
270-
}
271-
272-
override fun CoroutineDispatcher.resumeUndispatchedWithException(exception: Throwable) {
273-
val dc = delegate as? DispatchedContinuation
274-
resumeImpl(CompletedExceptionally(exception), if (dc?.dispatcher === this) MODE_UNDISPATCHED else resumeMode)
275-
}
276-
277-
@Suppress("UNCHECKED_CAST")
278-
override fun <T> getSuccessfulResult(state: Any?): T =
279-
if (state is CompletedIdempotentResult) state.result as T else state as T
280-
281-
protected override fun nameString(): String =
282-
"CancellableContinuation(${delegate.toDebugString()})"
283-
}
284-
285-
private class CompletedIdempotentResult(
286-
@JvmField val idempotentResume: Any?,
287-
@JvmField val result: Any?,
288-
@JvmField val token: NotCompleted
289-
) {
290-
override fun toString(): String = "CompletedIdempotentResult[$result]"
291-
}

0 commit comments

Comments
 (0)