Skip to content

Commit 3b2e437

Browse files
qwwdfsadqwwdfsad
committed
Change stacktrace recovery contract
 * Stacktraces are no longer recovered in DispatchedTask.run * Callers of uCont.resumeWithException are responsible for recovering the stacktrace * CancellableContinuation.resumeWithException recovers stacktrace if necessary * Properly recover stacktrace in Channel.receive fast-path * Properly recover stactraces on Channel.send fast-path * Restructure stacktrace recovery tests
1 parent 5f2413a commit 3b2e437

32 files changed

+606
-245
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ internal open class CancellableContinuationImpl<in T>(
244244
}
245245

246246
override fun resumeWith(result: Result<T>) {
247-
resumeImpl(result.toState(), resumeMode)
247+
resumeImpl(result.toState(this), resumeMode)
248248
}
249249

250250
override fun resume(value: T, onCancellation: (cause: Throwable) -> Unit) {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import kotlinx.coroutines.internal.*
99
import kotlin.coroutines.*
1010
import kotlin.jvm.*
1111

12-
internal fun <T> Result<T>.toState(): Any? =
13-
if (isSuccess) getOrThrow() else CompletedExceptionally(exceptionOrNull()!!) // todo: need to do it better
12+
internal fun <T> Result<T>.toState(): Any? = fold({ it }, { CompletedExceptionally(it) })
13+
14+
internal fun <T> Result<T>.toState(caller: CancellableContinuation<*>): Any? = fold({ it },
15+
{ CompletedExceptionally(recoverStackTrace(it, caller)) })
1416

1517
@Suppress("RESULT_CLASS_IN_RETURN_TYPE", "UNCHECKED_CAST")
1618
internal fun <T> recoverResult(state: Any?, uCont: Continuation<T>): Result<T> =

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,13 +144,13 @@ internal abstract class AbstractSendChannel<E> : SendChannel<E> {
144144

145145
public final override suspend fun send(element: E) {
146146
// fast path -- try offer non-blocking
147-
if (offer(element)) return
148-
// slow-path does suspend
147+
if (offerInternal(element) === OFFER_SUCCESS) return
148+
// slow-path does suspend or throws exception
149149
return sendSuspend(element)
150150
}
151151

152152
internal suspend fun sendFair(element: E) {
153-
if (offer(element)) {
153+
if (offerInternal(element) === OFFER_SUCCESS) {
154154
yield() // Works only on fast path to properly work in sequential use-cases
155155
return
156156
}
@@ -547,7 +547,11 @@ internal abstract class AbstractChannel<E> : AbstractSendChannel<E>(), Channel<E
547547
public final override suspend fun receive(): E {
548548
// fast path -- try poll non-blocking
549549
val result = pollInternal()
550-
if (result !== POLL_FAILED) return receiveResult(result)
550+
/*
551+
* If result is Closed -- go to tail-call slow-path that will allow us to
552+
* properly recover stacktrace without paying a performance cost on fast path.
553+
*/
554+
if (result !== POLL_FAILED && result !is Closed<*>) return receiveResult(result)
551555
// slow-path does suspend
552556
return receiveSuspend(RECEIVE_THROWS_ON_CLOSE)
553557
}

‎kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ internal abstract class DispatchedTask<in T>(
5252
cancelResult(state, cause)
5353
continuation.resumeWithStackTrace(cause)
5454
} else {
55-
if (exception != null) continuation.resumeWithStackTrace(exception)
55+
if (exception != null) continuation.resumeWithException(exception)
5656
else continuation.resume(getSuccessfulResult(state))
5757
}
5858
}
@@ -116,19 +116,8 @@ internal fun <T> DispatchedTask<T>.dispatch(mode: Int) {
116116
internal fun <T> DispatchedTask<T>.resume(delegate: Continuation<T>, useMode: Int) {
117117
// slow-path - use delegate
118118
val state = takeState()
119-
val exception = getExceptionalResult(state)?.let {
120-
/*
121-
* Recover stacktrace for non-dispatched tasks.
122-
* We usually do not recover stacktrace in a `resume` as all resumes go through `DispatchedTask.run`
123-
* and we recover stacktraces there, but this is not the case for a `suspend fun main()` that knows nothing about
124-
* kotlinx.coroutines and DispatchedTask
125-
*/
126-
if (delegate is DispatchedTask<*>) it else recoverStackTrace(it, delegate)
127-
}
128-
val result = if (exception != null)
129-
Result.failure(exception)
130-
else
131-
Result.success(state as T)
119+
val exception = getExceptionalResult(state)?.let { recoverStackTrace(it, delegate) }
120+
val result = if (exception != null) Result.failure(exception) else Result.success(state as T)
132121
when (useMode) {
133122
MODE_ATOMIC_DEFAULT -> delegate.resumeWith(result)
134123
MODE_CANCELLABLE -> delegate.resumeCancellableWith(result)

‎kotlinx-coroutines-core/common/src/internal/Scopes.kt

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ internal open class ScopeCoroutine<in T>(
3333
}
3434
}
3535

36-
internal fun AbstractCoroutine<*>.tryRecover(exception: Throwable): Throwable {
37-
val cont = (this as? ScopeCoroutine<*>)?.uCont ?: return exception
38-
return recoverStackTrace(exception, cont)
39-
}
40-
4136
internal class ContextScope(context: CoroutineContext) : CoroutineScope {
4237
override val coroutineContext: CoroutineContext = context
4338
}

‎kotlinx-coroutines-core/common/src/intrinsics/Undispatched.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ private inline fun <T> startDirect(completion: Continuation<T>, block: (Continua
8585
* First, this function initializes the parent job from the `parentContext` of this coroutine that was passed to it
8686
* during construction. Second, it starts the coroutine using [startCoroutineUninterceptedOrReturn].
8787
*/
88-
internal fun <T, R> AbstractCoroutine<T>.startUndispatchedOrReturn(receiver: R, block: suspend R.() -> T): Any? {
88+
internal fun <T, R> ScopeCoroutine<T>.startUndispatchedOrReturn(receiver: R, block: suspend R.() -> T): Any? {
8989
initParentJob()
9090
return undispatchedResult({ true }) {
9191
block.startCoroutineUninterceptedOrReturn(receiver, this)
@@ -95,15 +95,15 @@ internal fun <T, R> AbstractCoroutine<T>.startUndispatchedOrReturn(receiver: R,
9595
/**
9696
* Same as [startUndispatchedOrReturn], but ignores [TimeoutCancellationException] on fast-path.
9797
*/
98-
internal fun <T, R> AbstractCoroutine<T>.startUndispatchedOrReturnIgnoreTimeout(
98+
internal fun <T, R> ScopeCoroutine<T>.startUndispatchedOrReturnIgnoreTimeout(
9999
receiver: R, block: suspend R.() -> T): Any? {
100100
initParentJob()
101101
return undispatchedResult({ e -> !(e is TimeoutCancellationException && e.coroutine === this) }) {
102102
block.startCoroutineUninterceptedOrReturn(receiver, this)
103103
}
104104
}
105105

106-
private inline fun <T> AbstractCoroutine<T>.undispatchedResult(
106+
private inline fun <T> ScopeCoroutine<T>.undispatchedResult(
107107
shouldThrow: (Throwable) -> Boolean,
108108
startBlock: () -> Any?
109109
): Any? {
@@ -129,8 +129,8 @@ private inline fun <T> AbstractCoroutine<T>.undispatchedResult(
129129
if (state === COMPLETING_WAITING_CHILDREN) return COROUTINE_SUSPENDED // (2)
130130
return if (state is CompletedExceptionally) { // (3)
131131
when {
132-
shouldThrow(state.cause) -> throw tryRecover(state.cause)
133-
result is CompletedExceptionally -> throw tryRecover(result.cause)
132+
shouldThrow(state.cause) -> throw recoverStackTrace(state.cause, uCont)
133+
result is CompletedExceptionally -> throw recoverStackTrace(result.cause, uCont)
134134
else -> result
135135
}
136136
} else {

‎kotlinx-coroutines-core/jvm/test-resources/stacktraces/channels/testCancelledOffer.txt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
kotlinx.coroutines.JobCancellationException: Job was cancelled; job=JobImpl{Cancelling}@2a06d350
2+
(Coroutine boundary)
3+
at kotlinx.coroutines.channels.AbstractSendChannel.offer(AbstractChannel.kt:170)
4+
at kotlinx.coroutines.channels.ChannelCoroutine.offer(ChannelCoroutine.kt)
5+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testCancelledOffer$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:153)
6+
Caused by: kotlinx.coroutines.JobCancellationException: Job was cancelled; job=JobImpl{Cancelling}@2a06d350
7+
at kotlinx.coroutines.JobSupport.createJobCancellationException(JobSupport.kt:680)
8+
at kotlinx.coroutines.JobSupport.createCauseException(JobSupport.kt:696)
9+
at kotlinx.coroutines.JobSupport.cancelMakeCompleting(JobSupport.kt:673)
10+
at kotlinx.coroutines.JobSupport.cancelImpl$kotlinx_coroutines_core(JobSupport.kt:645)
11+
at kotlinx.coroutines.JobSupport.cancelInternal(JobSupport.kt:611)
12+
at kotlinx.coroutines.JobSupport.cancel(JobSupport.kt:599)
13+
at kotlinx.coroutines.Job$DefaultImpls.cancel$default(Job.kt:164)
14+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testCancelledOffer$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:151)
15+
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)

‎kotlinx-coroutines-core/jvm/test-resources/stacktraces/channels/testOfferFromScope.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
kotlinx.coroutines.RecoverableTestException
2+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testOfferFromScope$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:109)
3+
(Coroutine boundary)
4+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest.sendInChannel(StackTraceRecoveryChannelsTest.kt:167)
5+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$sendWithContext$2.invokeSuspend(StackTraceRecoveryChannelsTest.kt:162)
6+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$sendFromScope$2.invokeSuspend(StackTraceRecoveryChannelsTest.kt:172)
7+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testOfferFromScope$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:112)
8+
Caused by: kotlinx.coroutines.RecoverableTestException
9+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testOfferFromScope$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:109)
10+
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)

‎kotlinx-coroutines-core/jvm/test-resources/stacktraces/channels/testOfferWithContextWrapped.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
kotlinx.coroutines.RecoverableTestException
2+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testOfferWithContextWrapped$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:98)
3+
(Coroutine boundary)
4+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest.sendInChannel(StackTraceRecoveryChannelsTest.kt:199)
5+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$sendWithContext$2.invokeSuspend(StackTraceRecoveryChannelsTest.kt:194)
6+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testOfferWithContextWrapped$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:100)
7+
Caused by: kotlinx.coroutines.RecoverableTestException
8+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testOfferWithContextWrapped$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:98)
9+
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)

‎kotlinx-coroutines-core/jvm/test-resources/stacktraces/channels/testOfferWithCurrentContext.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
kotlinx.coroutines.RecoverableTestException
2+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testOfferWithCurrentContext$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:86)
3+
(Coroutine boundary)
4+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest.sendInChannel(StackTraceRecoveryChannelsTest.kt:210)
5+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$sendWithContext$2.invokeSuspend(StackTraceRecoveryChannelsTest.kt:205)
6+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testOfferWithCurrentContext$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:89)
7+
Caused by: kotlinx.coroutines.RecoverableTestException
8+
at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testOfferWithCurrentContext$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt:86)
9+
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)

0 commit comments

Comments
 (0)