Skip to content

Commit ae57774

Browse files
committed
Eagerly create cancellation exception during Job.cancel call to make stacktrace even shorter
Also, fix stacktrace recovery in select clause
1 parent 3b2e437 commit ae57774

File tree

19 files changed

+155
-108
lines changed

19 files changed

+155
-108
lines changed

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ public abstract class kotlinx/coroutines/AbstractCoroutine : kotlinx/coroutines/
33
public fun <init> (Lkotlin/coroutines/CoroutineContext;Z)V
44
public synthetic fun <init> (Lkotlin/coroutines/CoroutineContext;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
55
protected fun afterResume (Ljava/lang/Object;)V
6+
protected fun cancellationExceptionMessage ()Ljava/lang/String;
67
public final fun getContext ()Lkotlin/coroutines/CoroutineContext;
78
public fun getCoroutineContext ()Lkotlin/coroutines/CoroutineContext;
89
public fun isActive ()Z
@@ -264,6 +265,10 @@ public final class kotlinx/coroutines/DelayKt {
264265
public static final fun delay (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
265266
}
266267

268+
public final class kotlinx/coroutines/DispatchedContinuationKt {
269+
public static final fun resumeCancellableWith (Lkotlin/coroutines/Continuation;Ljava/lang/Object;)V
270+
}
271+
267272
public final class kotlinx/coroutines/Dispatchers {
268273
public static final field INSTANCE Lkotlinx/coroutines/Dispatchers;
269274
public static final fun getDefault ()Lkotlinx/coroutines/CoroutineDispatcher;
@@ -387,7 +392,8 @@ public class kotlinx/coroutines/JobSupport : kotlinx/coroutines/ChildJob, kotlin
387392
public synthetic fun cancel (Ljava/lang/Throwable;)Z
388393
public fun cancel (Ljava/util/concurrent/CancellationException;)V
389394
public final fun cancelCoroutine (Ljava/lang/Throwable;)Z
390-
public fun cancelInternal (Ljava/lang/Throwable;)Z
395+
public fun cancelInternal (Ljava/lang/Throwable;)V
396+
protected fun cancellationExceptionMessage ()Ljava/lang/String;
391397
public fun childCancelled (Ljava/lang/Throwable;)Z
392398
public fun fold (Ljava/lang/Object;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object;
393399
public fun get (Lkotlin/coroutines/CoroutineContext$Key;)Lkotlin/coroutines/CoroutineContext$Element;
@@ -1049,7 +1055,7 @@ public final class kotlinx/coroutines/selects/SelectBuilderImpl : kotlinx/corout
10491055
public fun isSelected ()Z
10501056
public fun onTimeout (JLkotlin/jvm/functions/Function1;)V
10511057
public fun performAtomicTrySelect (Lkotlinx/coroutines/internal/AtomicDesc;)Ljava/lang/Object;
1052-
public fun resumeSelectCancellableWithException (Ljava/lang/Throwable;)V
1058+
public fun resumeSelectWithException (Ljava/lang/Throwable;)V
10531059
public fun resumeWith (Ljava/lang/Object;)V
10541060
public fun toString ()Ljava/lang/String;
10551061
public fun trySelect ()Z
@@ -1073,7 +1079,7 @@ public abstract interface class kotlinx/coroutines/selects/SelectInstance {
10731079
public abstract fun getCompletion ()Lkotlin/coroutines/Continuation;
10741080
public abstract fun isSelected ()Z
10751081
public abstract fun performAtomicTrySelect (Lkotlinx/coroutines/internal/AtomicDesc;)Ljava/lang/Object;
1076-
public abstract fun resumeSelectCancellableWithException (Ljava/lang/Throwable;)V
1082+
public abstract fun resumeSelectWithException (Ljava/lang/Throwable;)V
10771083
public abstract fun trySelect ()Z
10781084
public abstract fun trySelectOther (Lkotlinx/coroutines/internal/LockFreeLinkedListNode$PrepareOp;)Ljava/lang/Object;
10791085
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ public abstract class AbstractCoroutine<in T>(
9494
*/
9595
protected open fun onCancelled(cause: Throwable, handled: Boolean) {}
9696

97+
override fun cancellationExceptionMessage(): String = "$classSimpleName was cancelled"
98+
9799
@Suppress("UNCHECKED_CAST")
98100
protected final override fun onCompletionInternal(state: Any?) {
99101
if (state is CompletedExceptionally)

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,6 @@ internal open class CancellableContinuationImpl<in T>(
257257
}
258258
}
259259

260-
internal fun resumeWithExceptionMode(exception: Throwable, mode: Int) =
261-
resumeImpl(CompletedExceptionally(exception), mode)
262-
263260
public override fun invokeOnCancellation(handler: CompletionHandler) {
264261
var handleCache: CancelHandler? = null
265262
_state.loop { state ->

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ public fun Job.cancelChildren() = cancelChildren(null)
503503
*/
504504
@Deprecated(level = DeprecationLevel.HIDDEN, message = "Since 1.2.0, binary compatibility with versions <= 1.1.x")
505505
public fun Job.cancelChildren(cause: Throwable? = null) {
506-
children.forEach { (it as? JobSupport)?.cancelInternal(cause) }
506+
children.forEach { (it as? JobSupport)?.cancelInternal(cause.orCancellation(this)) }
507507
}
508508

509509
// -------------------- CoroutineContext extensions --------------------
@@ -586,9 +586,11 @@ public fun Job.cancel(message: String, cause: Throwable? = null): Unit = cancel(
586586
* @suppress This method has bad semantics when cause is not a [CancellationException]. Use [CoroutineContext.cancel].
587587
*/
588588
@Deprecated(level = DeprecationLevel.HIDDEN, message = "Since 1.2.0, binary compatibility with versions <= 1.1.x")
589-
public fun CoroutineContext.cancel(cause: Throwable? = null): Boolean =
590-
@Suppress("DEPRECATION")
591-
(this[Job] as? JobSupport)?.cancelInternal(cause) ?: false
589+
public fun CoroutineContext.cancel(cause: Throwable? = null): Boolean {
590+
val job = this[Job] as? JobSupport ?: return false
591+
job.cancelInternal(cause.orCancellation(job))
592+
return true
593+
}
592594

593595
/**
594596
* Cancels all children of the [Job] in this context, without touching the state of this job itself
@@ -610,9 +612,12 @@ public fun CoroutineContext.cancelChildren() = cancelChildren(null)
610612
*/
611613
@Deprecated(level = DeprecationLevel.HIDDEN, message = "Since 1.2.0, binary compatibility with versions <= 1.1.x")
612614
public fun CoroutineContext.cancelChildren(cause: Throwable? = null) {
613-
this[Job]?.children?.forEach { (it as? JobSupport)?.cancelInternal(cause) }
615+
val job = this[Job] ?: return
616+
job.children.forEach { (it as? JobSupport)?.cancelInternal(cause.orCancellation(job)) }
614617
}
615618

619+
private fun Throwable?.orCancellation(job: Job): Throwable = this ?: JobCancellationException("Job was cancelled", null, job)
620+
616621
/**
617622
* No-op implementation of [DisposableHandle].
618623
* @suppress **This an internal API and should not be used from general code.**

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

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
244244
// A case of no exceptions
245245
if (exceptions.isEmpty()) {
246246
// materialize cancellation exception if it was not materialized yet
247-
if (state.isCancelling) return createJobCancellationException()
247+
if (state.isCancelling) return defaultCancellationException()
248248
return null
249249
}
250250
// Take either the first real exception (not a cancellation) or just the first exception
@@ -406,8 +406,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
406406
}
407407

408408
protected fun Throwable.toCancellationException(message: String? = null): CancellationException =
409-
this as? CancellationException ?:
410-
JobCancellationException(message ?: "$classSimpleName was cancelled", this, this@JobSupport)
409+
this as? CancellationException ?: defaultCancellationException(message, this)
411410

412411
/**
413412
* Returns the cause that signals the completion of this job -- it returns the original
@@ -597,19 +596,23 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
597596

598597
// external cancel with cause, never invoked implicitly from internal machinery
599598
public override fun cancel(cause: CancellationException?) {
600-
cancelInternal(cause) // must delegate here, because some classes override cancelInternal(x)
599+
cancelInternal(cause ?: defaultCancellationException())
601600
}
602601

602+
protected open fun cancellationExceptionMessage(): String = "Job was cancelled"
603+
603604
// HIDDEN in Job interface. Invoked only by legacy compiled code.
604605
// external cancel with (optional) cause, never invoked implicitly from internal machinery
605606
@Deprecated(level = DeprecationLevel.HIDDEN, message = "Added since 1.2.0 for binary compatibility with versions <= 1.1.x")
606-
public override fun cancel(cause: Throwable?): Boolean =
607-
cancelInternal(cause)
607+
public override fun cancel(cause: Throwable?): Boolean {
608+
cancelInternal(cause?.toCancellationException() ?: defaultCancellationException())
609+
return true
610+
}
608611

609612
// It is overridden in channel-linked implementation
610-
// Note: Boolean result is used only in HIDDEN DEPRECATED functions that were public in versions <= 1.1.x
611-
public open fun cancelInternal(cause: Throwable?): Boolean =
612-
cancelImpl(cause) && handlesException
613+
public open fun cancelInternal(cause: Throwable) {
614+
cancelImpl(cause)
615+
}
613616

614617
// Parent is cancelling child
615618
public final override fun parentCancelled(parentJob: ParentJob) {
@@ -677,8 +680,9 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
677680
}
678681
}
679682

680-
private fun createJobCancellationException() =
681-
JobCancellationException("Job was cancelled", null, this)
683+
@Suppress("NOTHING_TO_INLINE") // Save a stack frame
684+
internal inline fun defaultCancellationException(message: String? = null, cause: Throwable? = null) =
685+
JobCancellationException(message ?: cancellationExceptionMessage(), cause, this)
682686

683687
override fun getChildJobCancellationCause(): CancellationException {
684688
// determine root cancellation cause of this job (why is it cancelling its children?)
@@ -694,7 +698,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
694698

695699
// cause is Throwable or ParentJob when cancelChild was invoked
696700
private fun createCauseException(cause: Any?): Throwable = when (cause) {
697-
is Throwable? -> cause ?: createJobCancellationException()
701+
is Throwable? -> cause ?: defaultCancellationException()
698702
else -> (cause as ParentJob).getChildJobCancellationCause()
699703
}
700704

@@ -1225,7 +1229,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
12251229
// already complete -- select result
12261230
if (select.trySelect()) {
12271231
if (state is CompletedExceptionally) {
1228-
select.resumeSelectCancellableWithException(state.cause)
1232+
select.resumeSelectWithException(state.cause)
12291233
}
12301234
else {
12311235
block.startCoroutineUnintercepted(state.unboxState() as T, select.completion)
@@ -1249,7 +1253,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
12491253
val state = this.state
12501254
// Note: await is non-atomic (can be cancelled while dispatched)
12511255
if (state is CompletedExceptionally)
1252-
select.resumeSelectCancellableWithException(state.cause)
1256+
select.resumeSelectWithException(state.cause)
12531257
else
12541258
block.startCoroutineCancellable(state.unboxState() as T, select.completion)
12551259
}
@@ -1384,8 +1388,8 @@ private class ResumeAwaitOnCompletion<T>(
13841388
val state = job.state
13851389
assert { state !is Incomplete }
13861390
if (state is CompletedExceptionally) {
1387-
// Resume with exception in atomic way to preserve exception
1388-
continuation.resumeWithExceptionMode(state.cause, MODE_ATOMIC_DEFAULT)
1391+
// Resume with with the corresponding exception to preserve it
1392+
continuation.resumeWithException(state.cause)
13891393
} else {
13901394
// Resuming with value in a cancellable way (AwaitContinuation is configured for this mode).
13911395
@Suppress("UNCHECKED_CAST")

kotlinx-coroutines-core/common/src/channels/AbstractChannel.kt

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ internal abstract class AbstractSendChannel<E> : SendChannel<E> {
463463

464464
override fun resumeSendClosed(closed: Closed<*>) {
465465
if (select.trySelect())
466-
select.resumeSelectCancellableWithException(closed.sendException)
466+
select.resumeSelectWithException(closed.sendException)
467467
}
468468

469469
override fun toString(): String = "SendSelect@$hexAddress($pollResult)[$channel, $select]"
@@ -550,18 +550,14 @@ internal abstract class AbstractChannel<E> : AbstractSendChannel<E>(), Channel<E
550550
/*
551551
* If result is Closed -- go to tail-call slow-path that will allow us to
552552
* properly recover stacktrace without paying a performance cost on fast path.
553+
* We prefer to recover stacktrace using suspending path to have a more precise stacktrace.
553554
*/
554-
if (result !== POLL_FAILED && result !is Closed<*>) return receiveResult(result)
555+
@Suppress("UNCHECKED_CAST")
556+
if (result !== POLL_FAILED && result !is Closed<*>) return result as E
555557
// slow-path does suspend
556558
return receiveSuspend(RECEIVE_THROWS_ON_CLOSE)
557559
}
558560

559-
@Suppress("UNCHECKED_CAST")
560-
private fun receiveResult(result: Any?): E {
561-
if (result is Closed<*>) throw recoverStackTrace(result.receiveException)
562-
return result as E
563-
}
564-
565561
@Suppress("UNCHECKED_CAST")
566562
private suspend fun <R> receiveSuspend(receiveMode: Int): R = suspendAtomicCancellableCoroutineReusable sc@ { cont ->
567563
val receive = ReceiveElement<E>(cont as CancellableContinuation<Any?>, receiveMode)
@@ -975,12 +971,12 @@ internal abstract class AbstractChannel<E> : AbstractSendChannel<E>(), Channel<E
975971
override fun resumeReceiveClosed(closed: Closed<*>) {
976972
if (!select.trySelect()) return
977973
when (receiveMode) {
978-
RECEIVE_THROWS_ON_CLOSE -> select.resumeSelectCancellableWithException(closed.receiveException)
974+
RECEIVE_THROWS_ON_CLOSE -> select.resumeSelectWithException(closed.receiveException)
979975
RECEIVE_RESULT -> block.startCoroutine(ValueOrClosed.closed<R>(closed.closeCause), select.completion)
980976
RECEIVE_NULL_ON_CLOSE -> if (closed.closeCause == null) {
981977
block.startCoroutine(null, select.completion)
982978
} else {
983-
select.resumeSelectCancellableWithException(closed.receiveException)
979+
select.resumeSelectWithException(closed.receiveException)
984980
}
985981
}
986982
}

kotlinx-coroutines-core/common/src/channels/Broadcast.kt

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,17 +97,18 @@ private open class BroadcastCoroutine<E>(
9797
get() = this
9898

9999
@Deprecated(level = DeprecationLevel.HIDDEN, message = "Since 1.2.0, binary compatibility with versions <= 1.1.x")
100-
final override fun cancel(cause: Throwable?): Boolean =
101-
cancelInternal(cause)
100+
final override fun cancel(cause: Throwable?): Boolean {
101+
cancelInternal(cause ?: defaultCancellationException())
102+
return true
103+
}
102104

103105
final override fun cancel(cause: CancellationException?) {
104-
cancelInternal(cause)
106+
cancelInternal(cause ?: defaultCancellationException())
105107
}
106108

107-
override fun cancelInternal(cause: Throwable?): Boolean {
108-
_channel.cancel(cause?.toCancellationException()) // cancel the channel
109+
override fun cancelInternal(cause: Throwable) {
110+
_channel.cancel(cause.toCancellationException()) // cancel the channel
109111
cancelCoroutine(cause) // cancel the job
110-
return true // does not matter - result is used in DEPRECATED functions only
111112
}
112113

113114
override fun onCompleted(value: Unit) {

kotlinx-coroutines-core/common/src/channels/ChannelCoroutine.kt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,23 @@ internal open class ChannelCoroutine<E>(
1616
val channel: Channel<E> get() = this
1717

1818
override fun cancel() {
19-
cancelInternal(null)
19+
cancelInternal(defaultCancellationException())
2020
}
2121

2222
@Deprecated(level = DeprecationLevel.HIDDEN, message = "Since 1.2.0, binary compatibility with versions <= 1.1.x")
23-
final override fun cancel(cause: Throwable?): Boolean =
24-
cancelInternal(cause)
23+
final override fun cancel(cause: Throwable?): Boolean {
24+
cancelInternal(defaultCancellationException())
25+
return true
26+
}
2527

2628
final override fun cancel(cause: CancellationException?) {
27-
cancelInternal(cause)
29+
cancelInternal(cause ?: defaultCancellationException())
2830
}
2931

30-
override fun cancelInternal(cause: Throwable?): Boolean {
31-
val exception = cause?.toCancellationException()
32-
?: JobCancellationException("$classSimpleName was cancelled", null, this)
32+
override fun cancelInternal(cause: Throwable) {
33+
val exception = cause.toCancellationException()
3334
_channel.cancel(exception) // cancel the channel
3435
cancelCoroutine(exception) // cancel the job
35-
return true // does not matter - result is used in DEPRECATED functions only
3636
}
3737

3838
@Suppress("UNCHECKED_CAST")

kotlinx-coroutines-core/common/src/channels/ConflatedBroadcastChannel.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ public class ConflatedBroadcastChannel<E>() : BroadcastChannel<E> {
275275
private fun <R> registerSelectSend(select: SelectInstance<R>, element: E, block: suspend (SendChannel<E>) -> R) {
276276
if (!select.trySelect()) return
277277
offerInternal(element)?.let {
278-
select.resumeSelectCancellableWithException(it.sendException)
278+
select.resumeSelectWithException(it.sendException)
279279
return
280280
}
281281
block.startCoroutineUnintercepted(receiver = this, completion = select.completion)

kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,9 @@ internal class DispatchedContinuation<in T>(
174174
}
175175
}
176176

177-
@Suppress("NOTHING_TO_INLINE") // we need it inline to save us an entry on the stack
177+
// We inline it to save an entry on the stack in cases where it shows (unconfined dispatcher)
178+
// It is used only in Continuation<T>.resumeCancellableWith
179+
@Suppress("NOTHING_TO_INLINE")
178180
inline fun resumeCancellableWith(result: Result<T>) {
179181
val state = result.toState()
180182
if (dispatcher.isDispatchNeeded(context)) {
@@ -220,8 +222,14 @@ internal class DispatchedContinuation<in T>(
220222
"DispatchedContinuation[$dispatcher, ${continuation.toDebugString()}]"
221223
}
222224

223-
@Suppress("NOTHING_TO_INLINE") // we need it inline to save us an entry on the stack
224-
internal inline fun <T> Continuation<T>.resumeCancellableWith(result: Result<T>) = when (this) {
225+
/**
226+
* It is not inline to save bytecode (it is pretty big and used in many places)
227+
* and we leave it public so that its name is not mangled in use stack traces if it shows there.
228+
* It may appear in stack traces when coroutines are started/resumed with unconfined dispatcher.
229+
* @suppress **This an internal API and should not be used from general code.**
230+
*/
231+
@InternalCoroutinesApi
232+
public fun <T> Continuation<T>.resumeCancellableWith(result: Result<T>) = when (this) {
225233
is DispatchedContinuation -> resumeCancellableWith(result)
226234
else -> resumeWith(result)
227235
}

kotlinx-coroutines-core/common/src/selects/Select.kt

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,10 @@ public interface SelectInstance<in R> {
133133
public val completion: Continuation<R>
134134

135135
/**
136-
* Resumes this instance in a cancellable way ([MODE_CANCELLABLE]).
136+
* Resumes this instance in a dispatched way with exception.
137+
* This method can be called from any context.
137138
*/
138-
public fun resumeSelectCancellableWithException(exception: Throwable)
139+
public fun resumeSelectWithException(exception: Throwable)
139140

140141
/**
141142
* Disposes the specified handle when this instance is selected.
@@ -282,10 +283,10 @@ internal class SelectBuilderImpl<in R>(
282283
}
283284
}
284285

285-
// Resumes in MODE_CANCELLABLE, can be called from an arbitrary context
286-
override fun resumeSelectCancellableWithException(exception: Throwable) {
287-
doResume({ CompletedExceptionally(exception) }) {
288-
uCont.intercepted().resumeCancellableWith(Result.failure(exception))
286+
// Resumes in dispatched way so that it can be called from an arbitrary context
287+
override fun resumeSelectWithException(exception: Throwable) {
288+
doResume({ CompletedExceptionally(recoverStackTrace(exception, uCont)) }) {
289+
uCont.intercepted().resumeWith(Result.failure(exception))
289290
}
290291
}
291292

@@ -317,7 +318,7 @@ internal class SelectBuilderImpl<in R>(
317318
// Note: may be invoked multiple times, but only the first trySelect succeeds anyway
318319
override fun invoke(cause: Throwable?) {
319320
if (trySelect())
320-
resumeSelectCancellableWithException(job.getCancellationException())
321+
resumeSelectWithException(job.getCancellationException())
321322
}
322323
override fun toString(): String = "SelectOnCancelling[${this@SelectBuilderImpl}]"
323324
}

0 commit comments

Comments
 (0)