Skip to content

Commit a910d75

Browse files
committed
More comments about CancellationException and its consistent use;
JobCancellationException.job is made internal.
1 parent d7e6822 commit a910d75

File tree

11 files changed

+37
-17
lines changed

11 files changed

+37
-17
lines changed

common/kotlinx-coroutines-core-common/src/main/kotlin/kotlinx/coroutines/experimental/AbstractCoroutine.kt

+5-2
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,11 @@ public abstract class AbstractCoroutine<in T>(
7676
* This function is invoked once when this coroutine is cancelled or is completed,
7777
* similarly to [invokeOnCompletion] with `onCancelling` set to `true`.
7878
*
79-
* @param cause the cause that was passed to [Job.cancel] function, [JobCancellationException] if it was completed without cause
80-
* or `null` if coroutine is completing normally.
79+
* The meaning of [cause] parameter:
80+
* * Cause is `null` when job has completed normally.
81+
* * Cause is an instance of [CancellationException] when job was cancelled _normally_.
82+
* **It should not be treated as an error**. In particular, it should not be reported to error logs.
83+
* * Otherwise, the job had _failed_.
8184
*/
8285
protected open fun onCancellation(cause: Throwable?) {}
8386

common/kotlinx-coroutines-core-common/src/main/kotlin/kotlinx/coroutines/experimental/CompletedExceptionally.kt

+5-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package kotlinx.coroutines.experimental
1818

19+
import kotlinx.coroutines.experimental.internalAnnotations.*
20+
1921
/**
2022
* Class for an internal state of a job that had completed exceptionally, including cancellation.
2123
*
@@ -26,7 +28,9 @@ package kotlinx.coroutines.experimental
2628
* or artificial JobCancellationException if no cause was provided
2729
* @suppress **This is unstable API and it is subject to change.**
2830
*/
29-
open class CompletedExceptionally(public val cause: Throwable) {
31+
open class CompletedExceptionally(
32+
@JvmField public val cause: Throwable
33+
) {
3034
override fun toString(): String = "$classSimpleName[$cause]"
3135
}
3236

common/kotlinx-coroutines-core-common/src/main/kotlin/kotlinx/coroutines/experimental/CompletionHandler.common.kt

+6
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ import kotlinx.coroutines.experimental.internal.*
2424
* Installed handler should not throw any exceptions. If it does, they will get caught,
2525
* wrapped into [CompletionHandlerException], and rethrown, potentially causing crash of unrelated code.
2626
*
27+
* The meaning of `cause` that is passed to the handler:
28+
* * Cause is `null` when job has completed normally.
29+
* * Cause is an instance of [CancellationException] when job was cancelled _normally_.
30+
* **It should not be treated as an error**. In particular, it should not be reported to error logs.
31+
* * Otherwise, the job had _failed_.
32+
*
2733
* **Note**: This type is a part of internal machinery that supports parent-child hierarchies
2834
* and allows for implementation of suspending functions that wait on the Job's state.
2935
* This type should not be used in general application code.

common/kotlinx-coroutines-core-common/src/main/kotlin/kotlinx/coroutines/experimental/Exceptions.common.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public expect class JobCancellationException(
2525
cause: Throwable?,
2626
job: Job
2727
) : CancellationException {
28-
val job: Job
28+
internal val job: Job
2929
}
3030

3131
internal expect class DispatchException(message: String, cause: Throwable) : RuntimeException

common/kotlinx-coroutines-core-common/src/main/kotlin/kotlinx/coroutines/experimental/Job.kt

+14-1
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,12 @@ public interface Job : CoroutineContext.Element {
279279
* with a job's exception or cancellation cause or `null`. Otherwise, handler will be invoked once when this
280280
* job is complete.
281281
*
282+
* The meaning of `cause` that is passed to the handler:
283+
* * Cause is `null` when job has completed normally.
284+
* * Cause is an instance of [CancellationException] when job was cancelled _normally_.
285+
* **It should not be treated as an error**. In particular, it should not be reported to error logs.
286+
* * Otherwise, the job had _failed_.
287+
*
282288
* The resulting [DisposableHandle] can be used to [dispose][DisposableHandle.dispose] the
283289
* registration of this handler and release its memory if its invocation is no longer needed.
284290
* There is no need to dispose the handler after completion of this job. The references to
@@ -297,6 +303,12 @@ public interface Job : CoroutineContext.Element {
297303
* with a job's cancellation cause or `null` unless [invokeImmediately] is set to false.
298304
* Otherwise, handler will be invoked once when this job is cancelled or complete.
299305
*
306+
* The meaning of `cause` that is passed to the handler:
307+
* * Cause is `null` when job has completed normally.
308+
* * Cause is an instance of [CancellationException] when job was cancelled _normally_.
309+
* **It should not be treated as an error**. In particular, it should not be reported to error logs.
310+
* * Otherwise, the job had _failed_.
311+
*
300312
* Invocation of this handler on a transition to a transient _cancelling_ state
301313
* is controlled by [onCancelling] boolean parameter.
302314
* The handler is invoked on invocation of [cancel] when
@@ -648,7 +660,8 @@ internal open class JobSupport constructor(active: Boolean) : Job, SelectClause0
648660
if (proposedUpdate !is CompletedExceptionally) return cancelled // not exception -- just use original cancelled
649661
val exception = proposedUpdate.cause
650662
if (cancelled.cause == exception) return cancelled // that is the cancelled we need already!
651-
663+
// todo: We need to rework this logic to keep original cancellation cause in the state and suppress other exceptions
664+
// that could have occurred while coroutine is being cancelled.
652665
// Do not spam with JCE in suppressed exceptions
653666
if (cancelled.cause !is JobCancellationException) {
654667
exception.addSuppressedThrowable(cancelled.cause)

core/kotlinx-coroutines-core/src/main/kotlin/kotlinx/coroutines/experimental/Exceptions.kt

+1-4
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,7 @@ public actual typealias CancellationException = java.util.concurrent.Cancellatio
4040
public actual class JobCancellationException public actual constructor(
4141
message: String,
4242
cause: Throwable?,
43-
/**
44-
* The job that was cancelled.
45-
*/
46-
public actual val job: Job
43+
@JvmField internal actual val job: Job
4744
) : CancellationException(message) {
4845

4946
init {

core/kotlinx-coroutines-core/src/main/kotlin/kotlinx/coroutines/experimental/channels/Produce.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ private class ProducerCoroutine<E>(
124124
val cause = exceptionally?.cause
125125
val processed = when (exceptionally) {
126126
// producer coroutine was cancelled -- cancel channel, but without cause if it was closed without cause
127-
is Cancelled -> _channel.cancel(if (cause is JobCancellationException) null else cause)
127+
is Cancelled -> _channel.cancel(if (cause is CancellationException) null else cause)
128128
else -> _channel.close(cause) // producer coroutine has completed -- close channel
129129
}
130130
if (!processed && cause != null)

js/kotlinx-coroutines-core-js/src/main/kotlin/kotlinx/coroutines/experimental/Exceptions.kt

+1-4
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,7 @@ public actual open class CancellationException actual constructor(message: Strin
4040
public actual class JobCancellationException public actual constructor(
4141
message: String,
4242
public override val cause: Throwable?,
43-
/**
44-
* The job that was cancelled.
45-
*/
46-
public actual val job: Job
43+
internal actual val job: Job
4744
) : CancellationException(message.withCause(cause)) {
4845
override fun toString(): String = "${super.toString()}; job=$job"
4946
override fun equals(other: Any?): Boolean =

reactive/kotlinx-coroutines-reactive/src/main/kotlin/kotlinx/coroutines/experimental/reactive/Publish.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ private class PublisherCoroutine<in T>(
167167
_nRequested.value = SIGNALLED // we'll signal onError/onCompleted (that the final state -- no CAS needed)
168168
val cause = getCompletionCause()
169169
try {
170-
if (cause != null && cause !is JobCancellationException)
170+
if (cause != null && cause !is CancellationException)
171171
subscriber.onError(cause)
172172
else
173173
subscriber.onComplete()

reactive/kotlinx-coroutines-rx1/src/main/kotlin/kotlinx/coroutines/experimental/rx1/RxObservable.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ private class RxObservableCoroutine<in T>(
168168
_nRequested.value = SIGNALLED // we'll signal onError/onCompleted (that the final state -- no CAS needed)
169169
val cause = getCompletionCause()
170170
try {
171-
if (cause != null && cause !is JobCancellationException)
171+
if (cause != null && cause !is CancellationException)
172172
subscriber.onError(cause)
173173
else
174174
subscriber.onCompleted()

reactive/kotlinx-coroutines-rx2/src/main/kotlin/kotlinx/coroutines/experimental/rx2/RxObservable.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ private class RxObservableCoroutine<T>(
158158
_signal.value = SIGNALLED // we'll signal onError/onCompleted (that the final state -- no CAS needed)
159159
val cause = getCompletionCause()
160160
try {
161-
if (cause != null && cause !is JobCancellationException)
161+
if (cause != null && cause !is CancellationException)
162162
subscriber.onError(cause)
163163
else
164164
subscriber.onComplete()

0 commit comments

Comments
 (0)