Skip to content

Commit 57bd941

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. * Job.cancelChild(parent) is introduced as a dedicated method to send cancellation signal from parent to child. * Job.getCancellationException() now works in completing state, too, and it is used to get exception when parent's completion cancels children. * Child never aggregates exception received from it parent. * JobSupport.handleJobException() for launch/actor is split into: - cancelParentWithException - can be invoked multiple times on race; - handleJobException which - 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.
1 parent bca98f4 commit 57bd941

29 files changed

+600
-546
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

+25-7
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
/**
@@ -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

+1-8
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ import kotlin.coroutines.experimental.*
1313
* **Note: This class cannot be used outside of internal coroutines framework**.
1414
* **Note: cannot be internal 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/Job.kt

+33-16
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,9 @@ public interface Job : CoroutineContext.Element {
127127
* returned. The [JobCancellationException.cause] of the resulting [JobCancellationException] references
128128
* the original cancellation cause that was passed to [cancel] function.
129129
*
130-
* This function throws [IllegalStateException] when invoked on a job that has not
131-
* [completed][isCompleted] nor [cancelled][isCancelled] yet.
130+
* This function throws [IllegalStateException] when invoked on a job that is still active.
131+
*
132+
* @suppress **This is unstable API and it is subject to change.**
132133
*/
133134
public fun getCancellationException(): CancellationException
134135

@@ -163,6 +164,25 @@ public interface Job : CoroutineContext.Element {
163164
public fun cancel(cause: Throwable? = null): Boolean
164165

165166
// ------------ parent-child ------------
167+
168+
/**
169+
* Child is reporting failure to the parent by invoking this method.
170+
* This method is invoked by the child twice. The first time child report its root cause as soon as possible,
171+
* so that all its siblings and the parent can start finishing their work asap on failure. The second time
172+
* child invokes this method when it had aggregated and determined its final termination cause.
173+
*
174+
* @suppress **This is unstable API and it is subject to change.**
175+
*/
176+
public fun childFailed(cause: Throwable): Boolean
177+
178+
/**
179+
* Cancels child job. This method is invoked by [parentJob] to cancel this child job.
180+
* Child finds the cancellation cause using [getCancellationException] of the [parentJob].
181+
* This method does nothing is the child is already being cancelled.
182+
*
183+
* @suppress **This is unstable API and it is subject to change.**
184+
*/
185+
public fun cancelChild(parentJob: Job)
166186

167187
/**
168188
* Returns a sequence of this job's children.
@@ -201,9 +221,9 @@ public interface Job : CoroutineContext.Element {
201221
* lookup a [Job] instance in the parent context and use this function to attach themselves as a child.
202222
* They also store a reference to the resulting [DisposableHandle] and dispose a handle when they complete.
203223
*
204-
* @suppress This is an internal API. This method is too error prone for public API.
224+
* @suppress **This is unstable API and it is subject to change.**
225+
* This is an internal API. This method is too error prone for public API.
205226
*/
206-
@Deprecated(message = "Start child coroutine with 'parent' parameter", level = DeprecationLevel.WARNING)
207227
public fun attachChild(child: Job): DisposableHandle
208228

209229
/**
@@ -285,23 +305,20 @@ public interface Job : CoroutineContext.Element {
285305
public fun invokeOnCompletion(handler: CompletionHandler): DisposableHandle
286306

287307
/**
288-
* Registers handler that is **synchronously** invoked once on cancellation or completion of this job.
289-
* When job is already cancelling or complete, then the handler is immediately invoked
308+
* Registers handler that is **synchronously** invoked once on failure or completion of this job.
309+
* When job is already failing or complete, then the handler is immediately invoked
290310
* with a job's cancellation cause or `null` unless [invokeImmediately] is set to false.
291-
* Otherwise, handler will be invoked once when this job is cancelled or complete.
311+
* Otherwise, handler will be invoked once when this job is failing or is complete.
292312
*
293313
* The meaning of `cause` that is passed to the handler:
294314
* * Cause is `null` when job has completed normally.
295315
* * Cause is an instance of [CancellationException] when job was cancelled _normally_.
296316
* **It should not be treated as an error**. In particular, it should not be reported to error logs.
297317
* * Otherwise, the job had _failed_.
298318
*
299-
* Invocation of this handler on a transition to a transient _cancelling_ state
300-
* is controlled by [onCancelling] boolean parameter.
301-
* The handler is invoked on invocation of [cancel] when
302-
* job becomes _cancelling_ if [onCancelling] parameter is set to `true`. However,
303-
* when this [Job] is not backed by a coroutine, like [CompletableDeferred] or [CancellableContinuation]
304-
* (both of which do not posses a _cancelling_ state), then the value of [onCancelling] parameter is ignored.
319+
* Invocation of this handler on a transition to a _failing_ state
320+
* is controlled by [onFailing] boolean parameter.
321+
* The handler is invoked when the job is failing when [onFailing] parameter is set to `true`.
305322
*
306323
* The resulting [DisposableHandle] can be used to [dispose][DisposableHandle.dispose] the
307324
* registration of this handler and release its memory if its invocation is no longer needed.
@@ -316,15 +333,15 @@ public interface Job : CoroutineContext.Element {
316333
* This function should not be used in general application code.
317334
* Implementations of `CompletionHandler` must be fast and _lock-free_.
318335
*
319-
* @param onCancelling when `true`, then the [handler] is invoked as soon as this job transitions to _cancelling_ state;
336+
* @param onFailing when `true`, then the [handler] is invoked as soon as this job transitions to _failing_ state;
320337
* when `false` then the [handler] is invoked only when it transitions to _completed_ state.
321-
* @param invokeImmediately when `true` and this job is already in the desired state (depending on [onCancelling]),
338+
* @param invokeImmediately when `true` and this job is already in the desired state (depending on [onFailing]),
322339
* then the [handler] is immediately and synchronously invoked and [NonDisposableHandle] is returned;
323340
* when `false` then [NonDisposableHandle] is returned, but the [handler] is not invoked.
324341
* @param handler the handler.
325342
*/
326343
public fun invokeOnCompletion(
327-
onCancelling: Boolean = false,
344+
onFailing: Boolean = false,
328345
invokeImmediately: Boolean = true,
329346
handler: CompletionHandler): DisposableHandle
330347

0 commit comments

Comments
 (0)