Skip to content

Commit 3777779

Browse files
committed
Fix exception aggregation to ensure atomic handling of exceptions
* Removed legacy onFinishing handler support from JobSupport. - It is no longer needed, because handleJobException covers #208 * Fixed bugs that were masked by cancelling parent from onFinishing. * Consistent "Finishing" state was introduced in internal machinery: - Job enters finishing state when it is failing or it is completing and has children - Finishing state cleanly aggregates all failures, tracks cancellation and completion status * Job.isFailed is introduced as a consistent way to query the "failing" state of the job that was previously only implicitly available via invokeOnCompletion handler (cause != null means a failed job) and the documentation for both Job & Deferred is updated to reflect that. * Source-incompatible change: Job.invokeOnCompletion boolean parameter is change to onFailing. Such handlers are now invoked as soon as the job starts failing and the root cause exception of the failure is consistently passed to all the handlers. * The following internal methods were introduced to facilitate this: - Job.failParent(exception) is used by child to signal failure to parent - Job.cancelChild(parentJob) is used by parent to cancel child. * Child never aggregates exception received from it parent, but uses it as it root cause if there is no other exception. * JobSupport.handleJobException() logic for launch/actor is split into: - failParent - can be invoked multiple times on race; - handleJobException which is invoked exactly once. * Exception materiazization is much lazier now, which should significantly improve performance when cancelling large hierarchies. * Other minor perf improvements in JobSupport code. Fixes #585
1 parent 19f3032 commit 3777779

36 files changed

+1024
-675
lines changed

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

+7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
public abstract class kotlinx/coroutines/AbstractCoroutine : kotlin/coroutines/Continuation, kotlinx/coroutines/CoroutineScope, kotlinx/coroutines/Job {
2+
protected final field parentContext Lkotlin/coroutines/experimental/CoroutineContext;
23
public fun <init> (Lkotlin/coroutines/CoroutineContext;Z)V
34
public synthetic fun <init> (Lkotlin/coroutines/CoroutineContext;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
45
public final fun getContext ()Lkotlin/coroutines/CoroutineContext;
@@ -376,7 +377,9 @@ public abstract interface class kotlinx/coroutines/Job : kotlin/coroutines/Corou
376377
public static final field Key Lkotlinx/coroutines/Job$Key;
377378
public abstract fun attachChild (Lkotlinx/coroutines/Job;)Lkotlinx/coroutines/DisposableHandle;
378379
public abstract fun cancel (Ljava/lang/Throwable;)Z
380+
public abstract fun cancelChild (Lkotlinx/coroutines/experimental/Job;)V
379381
public abstract synthetic fun cancelChildren (Ljava/lang/Throwable;)V
382+
public abstract fun childFailed (Ljava/lang/Throwable;)Z
380383
public abstract fun getCancellationException ()Ljava/util/concurrent/CancellationException;
381384
public abstract fun getChildren ()Lkotlin/sequences/Sequence;
382385
public abstract fun getCompletionException ()Ljava/lang/Throwable;
@@ -388,6 +391,7 @@ public abstract interface class kotlinx/coroutines/Job : kotlin/coroutines/Corou
388391
public abstract fun isActive ()Z
389392
public abstract fun isCancelled ()Z
390393
public abstract fun isCompleted ()Z
394+
public abstract fun isFailed ()Z
391395
public abstract fun join (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
392396
public abstract fun plus (Lkotlinx/coroutines/Job;)Lkotlinx/coroutines/Job;
393397
public abstract fun start ()Z
@@ -447,7 +451,9 @@ public final class kotlinx/coroutines/NonCancellable : kotlin/coroutines/Abstrac
447451
public static final field INSTANCE Lkotlinx/coroutines/NonCancellable;
448452
public fun attachChild (Lkotlinx/coroutines/Job;)Lkotlinx/coroutines/DisposableHandle;
449453
public fun cancel (Ljava/lang/Throwable;)Z
454+
public fun cancelChild (Lkotlinx/coroutines/experimental/Job;)V
450455
public synthetic fun cancelChildren (Ljava/lang/Throwable;)V
456+
public fun childFailed (Ljava/lang/Throwable;)Z
451457
public fun getCancellationException ()Ljava/util/concurrent/CancellationException;
452458
public fun getChildren ()Lkotlin/sequences/Sequence;
453459
public fun getCompletionException ()Ljava/lang/Throwable;
@@ -459,6 +465,7 @@ public final class kotlinx/coroutines/NonCancellable : kotlin/coroutines/Abstrac
459465
public fun isActive ()Z
460466
public fun isCancelled ()Z
461467
public fun isCompleted ()Z
468+
public fun isFailed ()Z
462469
public fun join (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
463470
public fun plus (Lkotlinx/coroutines/Job;)Lkotlinx/coroutines/Job;
464471
public fun start ()Z

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ internal abstract class AbstractContinuation<in T>(
8989
return
9090
}
9191
parent.start() // make sure the parent is started
92-
val handle = parent.invokeOnCompletion(onCancelling = true,
92+
val handle = parent.invokeOnCompletion(onFailing = true,
9393
handler = ChildContinuation(parent, this).asHandler)
9494

9595
parentHandle = handle

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

+26-8
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package kotlinx.coroutines
66

77
import kotlinx.coroutines.CoroutineStart.*
8+
import kotlinx.coroutines.internal.*
89
import kotlinx.coroutines.intrinsics.*
910
import kotlin.coroutines.*
1011

@@ -30,7 +31,11 @@ import kotlin.coroutines.*
3031
*/
3132
@Suppress("EXPOSED_SUPER_CLASS")
3233
public abstract class AbstractCoroutine<in T>(
33-
private val parentContext: CoroutineContext,
34+
/**
35+
* Context of the parent coroutine.
36+
*/
37+
@JvmField
38+
protected val parentContext: CoroutineContext,
3439
active: Boolean = true
3540
) : JobSupport(active), Job, Continuation<T>, CoroutineScope {
3641
@Suppress("LeakingThis")
@@ -63,19 +68,25 @@ public abstract class AbstractCoroutine<in T>(
6368
}
6469

6570
/**
66-
* This function is invoked once when this coroutine is cancelled or is completed,
67-
* similarly to [invokeOnCompletion] with `onCancelling` set to `true`.
71+
* @suppress **Deprecated**: Override [onFailing].
72+
*/
73+
@Deprecated("Override onFailing")
74+
protected open fun onCancellation(cause: Throwable?) {}
75+
76+
/**
77+
* This function is invoked once when this coroutine is failing or is completed,
78+
* similarly to [invokeOnCompletion] with `onFailing` set to `true`.
6879
*
6980
* The meaning of [cause] parameter:
7081
* * Cause is `null` when job has completed normally.
7182
* * Cause is an instance of [CancellationException] when job was cancelled _normally_.
7283
* **It should not be treated as an error**. In particular, it should not be reported to error logs.
7384
* * Otherwise, the job had _failed_.
85+
*
86+
* @suppress **Deprecated**: Override [onFailing].
7487
*/
75-
protected open fun onCancellation(cause: Throwable?) {}
76-
77-
internal override fun onCancellationInternal(exceptionally: CompletedExceptionally?) {
78-
onCancellation(exceptionally?.cause)
88+
override fun onFailing(cause: Throwable?) {
89+
onCancellation(cause)
7990
}
8091

8192
/**
@@ -89,7 +100,7 @@ public abstract class AbstractCoroutine<in T>(
89100
protected open fun onCompletedExceptionally(exception: Throwable) {}
90101

91102
@Suppress("UNCHECKED_CAST")
92-
internal override fun onCompletionInternal(state: Any?, mode: Int) {
103+
internal override fun onCompletionInternal(state: Any?, mode: Int, suppressed: Boolean) {
93104
if (state is CompletedExceptionally)
94105
onCompletedExceptionally(state.cause)
95106
else
@@ -105,6 +116,13 @@ public abstract class AbstractCoroutine<in T>(
105116
makeCompletingOnce(result.toState(), defaultResumeMode)
106117
}
107118

119+
// todo: make it for all kinds of coroutines, now only launch & actor override and handleExceptionViaJob
120+
internal fun failParentImpl(exception: Throwable): Boolean {
121+
if (exception is CancellationException) return true
122+
val parentJob = parentContext[Job]
123+
return parentJob !== null && parentJob.childFailed(exception)
124+
}
125+
108126
internal final override fun handleOnCompletionException(exception: Throwable) {
109127
handleCoroutineException(parentContext, exception, this)
110128
}

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

+3-12
Original file line numberDiff line numberDiff line change
@@ -210,20 +210,11 @@ public suspend fun <T> run(context: CoroutineContext, block: suspend () -> T): T
210210
// --------------- implementation ---------------
211211

212212
private open class StandaloneCoroutine(
213-
private val parentContext: CoroutineContext,
213+
parentContext: CoroutineContext,
214214
active: Boolean
215215
) : AbstractCoroutine<Unit>(parentContext, active) {
216-
override fun hasOnFinishingHandler(update: Any?) = update is CompletedExceptionally
217-
218-
override fun handleJobException(exception: Throwable) {
219-
handleCoroutineException(parentContext, exception, this)
220-
}
221-
222-
override fun onFinishingInternal(update: Any?) {
223-
if (update is CompletedExceptionally && update.cause !is CancellationException) {
224-
parentContext[Job]?.cancel(update.cause)
225-
}
226-
}
216+
override fun failParent(exception: Throwable) = failParentImpl(exception)
217+
override fun handleJobException(exception: Throwable) = handleExceptionViaHandler(parentContext, exception)
227218
}
228219

229220
private class LazyStandaloneCoroutine(

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ private class CompletableDeferredImpl<T>(
6161
parent: Job?
6262
) : JobSupport(true), CompletableDeferred<T>, SelectClause1<T> {
6363
init { initParentJobInternal(parent) }
64-
override val onCancelMode: Int get() = ON_CANCEL_MAKE_COMPLETING
64+
override val onFailComplete get() = true
6565
override fun getCompleted(): T = getCompletedInternal() as T
6666
override suspend fun await(): T = awaitInternal() as T
6767
override val onAwait: SelectClause1<T> get() = this

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

+2-9
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,8 @@ public fun <T> Result<T>.toState(): Any? =
1717
* Class for an internal state of a job that had completed exceptionally, including cancellation.
1818
*
1919
* **Note: This class cannot be used outside of internal coroutines framework**.
20-
* **Note: cannot be internal until we get rid of MutableDelegateContinuation in IO**
20+
* **Note: cannot be internal and renamed until we get rid of MutableDelegateContinuation in IO**
2121
*
22-
* @param cause the exceptional completion cause. It's either original exceptional cause
23-
* or artificial JobCancellationException if no cause was provided
2422
* @suppress **This is unstable API and it is subject to change.**
2523
*/
2624
open class CompletedExceptionally(
@@ -34,14 +32,9 @@ open class CompletedExceptionally(
3432
*
3533
* **Note: This class cannot be used outside of internal coroutines framework**.
3634
*
37-
* @param job the job that was cancelled.
38-
* @param cause the exceptional completion cause. If `cause` is null, then a [JobCancellationException] is created.
3935
* @suppress **This is unstable API and it is subject to change.**
4036
*/
41-
internal class Cancelled(
42-
job: Job,
43-
cause: Throwable?
44-
) : CompletedExceptionally(cause ?: JobCancellationException("Job was cancelled normally", null, job))
37+
internal class Cancelled(cause: Throwable) : CompletedExceptionally(cause)
4538

4639
/**
4740
* A specific subclass of [CompletedExceptionally] for cancelled [AbstractContinuation].

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

+2
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,5 @@ internal expect val CancelHandlerBase.asHandler: CompletionHandler
4343
// :KLUDGE: We have to invoke a handler in platform-specific way via `invokeIt` extension,
4444
// because we play type tricks on Kotlin/JS and handler is not necessarily a function there
4545
internal expect fun CompletionHandler.invokeIt(cause: Throwable?)
46+
47+
internal inline fun <reified T> CompletionHandler.isHandlerOf(): Boolean = this is T

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

+17-12
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,27 @@ internal expect fun handleCoroutineExceptionImpl(context: CoroutineContext, exce
2020
* If invocation returned `true`, method terminates: now [Job] is responsible for handling an exception.
2121
* Otherwise, If there is [CoroutineExceptionHandler] in the context, it is used.
2222
* Otherwise all instances of [CoroutineExceptionHandler] found via [ServiceLoader] and [Thread.uncaughtExceptionHandler] are invoked
23+
*
24+
* todo: Deprecate/hide this function.
2325
*/
2426
@JvmOverloads // binary compatibility
2527
public fun handleCoroutineException(context: CoroutineContext, exception: Throwable, caller: Job? = null) {
26-
// if exception handling fails, make sure the original exception is not lost
28+
if (!handleExceptionViaJob(context, exception, caller)) {
29+
handleExceptionViaHandler(context, exception)
30+
}
31+
}
32+
33+
private fun handleExceptionViaJob(context: CoroutineContext, exception: Throwable, caller: Job?): Boolean {
34+
// Ignore CancellationException (they are normal ways to terminate a coroutine)
35+
if (exception is CancellationException) return true
36+
// If job is successfully cancelled, we're done
37+
val job = context[Job]
38+
return job !== null && job !== caller && job.cancel(exception)
39+
}
40+
41+
internal fun handleExceptionViaHandler(context: CoroutineContext, exception: Throwable) {
2742
try {
28-
// Ignore CancellationException (they are normal ways to terminate a coroutine)
29-
if (exception is CancellationException) {
30-
return
31-
}
32-
// If parent is successfully cancelled, we're done, it is now its responsibility to handle the exception
33-
val parent = context[Job]
34-
// E.g. actor registers itself in the context, in that case we should invoke handler
35-
if (parent !== null && parent !== caller && parent.cancel(exception)) {
36-
return
37-
}
38-
// If not, invoke exception handler from the context
43+
// Invoke exception handler from the context if present
3944
context[CoroutineExceptionHandler]?.let {
4045
it.handleException(context, exception)
4146
return

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ object GlobalScope : CoroutineScope {
180180
*/
181181
public suspend fun <R> coroutineScope(block: suspend CoroutineScope.() -> R): R {
182182
// todo: optimize implementation to a single allocated object
183-
val owner = ScopeOwnerCoroutine<R>(coroutineContext)
183+
val owner = ScopeCoroutine<R>(coroutineContext)
184184
owner.start(CoroutineStart.UNDISPATCHED, owner, block)
185185
owner.join()
186186
if (owner.isCancelled) {

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

+10-44
Original file line numberDiff line numberDiff line change
@@ -12,69 +12,35 @@ import kotlin.coroutines.*
1212
import kotlin.experimental.*
1313

1414
/**
15-
* Deferred value is a non-blocking cancellable future.
15+
* Deferred value is a non-blocking cancellable future &mdash; it is a [Job] that has a result.
1616
*
1717
* It is created with [async][CoroutineScope.async] coroutine builder or via constructor of [CompletableDeferred] class.
1818
* It is in [active][isActive] state while the value is being computed.
1919
*
20-
* Deferred value has the following states:
21-
*
22-
* | **State** | [isActive] | [isCompleted] | [isCompletedExceptionally] | [isCancelled] |
23-
* | --------------------------------------- | ---------- | ------------- | -------------------------- | ------------- |
24-
* | _New_ (optional initial state) | `false` | `false` | `false` | `false` |
25-
* | _Active_ (default initial state) | `true` | `false` | `false` | `false` |
26-
* | _Completing_ (optional transient state) | `true` | `false` | `false` | `false` |
27-
* | _Cancelling_ (optional transient state) | `false` | `false` | `false` | `true` |
28-
* | _Cancelled_ (final state) | `false` | `true` | `true` | `true` |
29-
* | _Resolved_ (final state) | `false` | `true` | `false` | `false` |
30-
* | _Failed_ (final state) | `false` | `true` | `true` | `false` |
20+
* Deferred value has the same state machine as the [Job] with additional convenience methods to retrieve
21+
* successful or failed result of the computation that was carried out. The result of the deferred is
22+
* available when it is [completed][isCompleted] and can be retrieved by [await] method, which throws
23+
* exception if the deferred had failed.
24+
* A _failed_ deferred is considered to be [completed exceptionally][isCompletedExceptionally].
25+
* The corresponding exception can be retrieved via [getCompletionExceptionOrNull] from a completed instance of deferred.
3126
*
3227
* Usually, a deferred value is created in _active_ state (it is created and started).
3328
* However, [async][CoroutineScope.async] coroutine builder has an optional `start` parameter that creates a deferred value in _new_ state
3429
* when this parameter is set to [CoroutineStart.LAZY].
3530
* Such a deferred can be be made _active_ by invoking [start], [join], or [await].
3631
*
37-
* A deferred can be _cancelled_ at any time with [cancel] function that forces it to transition to
38-
* _cancelling_ state immediately. Deferred that is not backed by a coroutine (see [CompletableDeferred]) and does not have
39-
* [children] becomes _cancelled_ on [cancel] immediately.
40-
* Otherwise, deferred becomes _cancelled_ when it finishes executing its code and
41-
* when all its children [complete][isCompleted].
42-
*
43-
* ```
44-
* wait children
45-
* +-----+ start +--------+ complete +-------------+ finish +-----------+
46-
* | New | ---------------> | Active | ----------> | Completing | ---+-> | Resolved |
47-
* +-----+ +--------+ +-------------+ | |(completed)|
48-
* | | | | +-----------+
49-
* | cancel | cancel | cancel |
50-
* V V | | +-----------+
51-
* +-----------+ finish +------------+ | +-> | Failed |
52-
* | Cancelled | <--------- | Cancelling | <---------------+ |(completed)|
53-
* |(completed)| +------------+ +-----------+
54-
* +-----------+
55-
* ```
56-
*
5732
* A deferred value is a [Job]. A job in the
5833
* [coroutineContext](https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.coroutines/coroutine-context.html)
5934
* of [async][CoroutineScope.async] builder represents the coroutine itself.
60-
* A deferred value is active while the coroutine is working and cancellation aborts the coroutine when
61-
* the coroutine is suspended on a _cancellable_ suspension point by throwing [CancellationException]
62-
* or the cancellation cause inside the coroutine.
63-
*
64-
* A deferred value can have a _parent_ job. A deferred value with a parent is cancelled when its parent is
65-
* cancelled or completes. Parent waits for all its [children] to complete in _completing_ or
66-
* _cancelling_ state. _Completing_ state is purely internal. For an outside observer a _completing_
67-
* deferred is still active, while internally it is waiting for its children.
6835
*
6936
* All functions on this interface and on all interfaces derived from it are **thread-safe** and can
7037
* be safely invoked from concurrent coroutines without external synchronization.
7138
*/
7239
public interface Deferred<out T> : Job {
7340
/**
74-
* Returns `true` if computation of this deferred value has _completed exceptionally_ -- it had
75-
* either _failed_ with exception during computation or was [cancelled][cancel].
76-
*
77-
* It implies that [isActive] is `false` and [isCompleted] is `true`.
41+
* Returns `true` if computation of this deferred value has _completed exceptionally_.
42+
* It is `true` when both [isCompleted] and [isFailed] are true.
43+
* It implies that [isActive] is `false`.
7844
*/
7945
public val isCompletedExceptionally: Boolean
8046

0 commit comments

Comments
 (0)