Skip to content

Commit 1c9afc9

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 82547f6 commit 1c9afc9

35 files changed

+976
-671
lines changed

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

+5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
public abstract class kotlinx/coroutines/experimental/AbstractCoroutine : kotlin/coroutines/experimental/Continuation, kotlinx/coroutines/experimental/CoroutineScope, kotlinx/coroutines/experimental/Job {
2+
protected final field parentContext Lkotlin/coroutines/experimental/CoroutineContext;
23
public fun <init> (Lkotlin/coroutines/experimental/CoroutineContext;Z)V
34
public synthetic fun <init> (Lkotlin/coroutines/experimental/CoroutineContext;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
45
public final fun getContext ()Lkotlin/coroutines/experimental/CoroutineContext;
@@ -369,7 +370,9 @@ public abstract interface class kotlinx/coroutines/experimental/Job : kotlin/cor
369370
public static final field Key Lkotlinx/coroutines/experimental/Job$Key;
370371
public abstract fun attachChild (Lkotlinx/coroutines/experimental/Job;)Lkotlinx/coroutines/experimental/DisposableHandle;
371372
public abstract fun cancel (Ljava/lang/Throwable;)Z
373+
public abstract fun cancelChild (Lkotlinx/coroutines/experimental/Job;)V
372374
public abstract synthetic fun cancelChildren (Ljava/lang/Throwable;)V
375+
public abstract fun childFailed (Ljava/lang/Throwable;)Z
373376
public abstract fun getCancellationException ()Ljava/util/concurrent/CancellationException;
374377
public abstract fun getChildren ()Lkotlin/sequences/Sequence;
375378
public abstract fun getCompletionException ()Ljava/lang/Throwable;
@@ -440,7 +443,9 @@ public final class kotlinx/coroutines/experimental/NonCancellable : kotlin/corou
440443
public static final field INSTANCE Lkotlinx/coroutines/experimental/NonCancellable;
441444
public fun attachChild (Lkotlinx/coroutines/experimental/Job;)Lkotlinx/coroutines/experimental/DisposableHandle;
442445
public fun cancel (Ljava/lang/Throwable;)Z
446+
public fun cancelChild (Lkotlinx/coroutines/experimental/Job;)V
443447
public synthetic fun cancelChildren (Ljava/lang/Throwable;)V
448+
public fun childFailed (Ljava/lang/Throwable;)Z
444449
public fun getCancellationException ()Ljava/util/concurrent/CancellationException;
445450
public fun getChildren ()Lkotlin/sequences/Sequence;
446451
public fun getCompletionException ()Ljava/lang/Throwable;

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.experimental
66

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

@@ -30,7 +31,11 @@ import kotlin.coroutines.experimental.*
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
@@ -112,6 +123,13 @@ public abstract class AbstractCoroutine<in T>(
112123
makeCompletingOnce(CompletedExceptionally(exception), defaultResumeMode)
113124
}
114125

126+
// todo: make it for all kinds of coroutines, now only launch & actor override and handleExceptionViaJob
127+
internal fun failParentImpl(exception: Throwable): Boolean {
128+
if (exception is CancellationException) return true
129+
val parentJob = parentContext[Job]
130+
return parentJob !== null && parentJob.childFailed(exception)
131+
}
132+
115133
internal final override fun handleOnCompletionException(exception: Throwable) {
116134
handleCoroutineException(parentContext, exception, this)
117135
}

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

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

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

226217
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
@@ -11,10 +11,8 @@ import kotlin.coroutines.experimental.*
1111
* Class for an internal state of a job that had completed exceptionally, including cancellation.
1212
*
1313
* **Note: This class cannot be used outside of internal coroutines framework**.
14-
* **Note: cannot be internal until we get rid of MutableDelegateContinuation in IO**
14+
* **Note: cannot be internal and renamed until we get rid of MutableDelegateContinuation in IO**
1515
*
16-
* @param cause the exceptional completion cause. It's either original exceptional cause
17-
* or artificial JobCancellationException if no cause was provided
1816
* @suppress **This is unstable API and it is subject to change.**
1917
*/
2018
open class CompletedExceptionally(
@@ -28,14 +26,9 @@ open class CompletedExceptionally(
2826
*
2927
* **Note: This class cannot be used outside of internal coroutines framework**.
3028
*
31-
* @param job the job that was cancelled.
32-
* @param cause the exceptional completion cause. If `cause` is null, then a [JobCancellationException] is created.
3329
* @suppress **This is unstable API and it is subject to change.**
3430
*/
35-
internal class Cancelled(
36-
job: Job,
37-
cause: Throwable?
38-
) : CompletedExceptionally(cause ?: JobCancellationException("Job was cancelled normally", null, job))
31+
internal class Cancelled(cause: Throwable) : CompletedExceptionally(cause)
3932

4033
/**
4134
* 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
@@ -175,7 +175,7 @@ object GlobalScope : CoroutineScope {
175175
*/
176176
public suspend fun <R> coroutineScope(block: suspend CoroutineScope.() -> R): R {
177177
// todo: optimize implementation to a single allocated object
178-
val owner = ScopeOwnerCoroutine<R>(coroutineContext)
178+
val owner = ScopeCoroutine<R>(coroutineContext)
179179
owner.start(CoroutineStart.UNDISPATCHED, owner, block)
180180
owner.join()
181181
if (owner.isCancelled) {

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

+10-44
Original file line numberDiff line numberDiff line change
@@ -9,69 +9,35 @@ import kotlinx.coroutines.experimental.selects.*
99
import kotlin.coroutines.experimental.*
1010

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

0 commit comments

Comments
 (0)