Skip to content

Get rid of NonRecoverableThrowable, mention way to opt-out stacktrace… #1420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/debugging.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ It is easy to demonstrate with actual stacktraces of the same program that await

The only downside of this approach is losing referential transparency of the exception.

### Stacktrace recovery machinery
### Stacktrace recovery machinery

This section explains the inner mechanism of stacktrace recovery and can be skipped.

Expand All @@ -56,6 +56,7 @@ and then throws the resulting exception instead of the original one.

Exception copy logic is straightforward:
1) If the exception class implements [CopyableThrowable], [CopyableThrowable.createCopy] is used.
`null` can be returned from `createCopy` to opt-out specific exception from being recovered.
2) If the exception class has class-specific fields not inherited from Throwable, the exception is not copied.
3) Otherwise, one of the public exception's constructor is invoked reflectively with an optional `initCause` call.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,3 @@ internal expect interface CoroutineStackFrame {
public val callerFrame: CoroutineStackFrame?
public fun getStackTraceElement(): StackTraceElement?
}

/**
* Marker that indicates that stacktrace of the exception should not be recovered.
* Currently internal, but may become public in the future
*/
internal interface NonRecoverableThrowable
17 changes: 10 additions & 7 deletions kotlinx-coroutines-core/common/test/TestBase.common.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
* Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/

@file:Suppress("unused")

package kotlinx.coroutines

import kotlinx.coroutines.flow.*
import kotlin.coroutines.*
import kotlinx.coroutines.internal.*
import kotlin.test.*

public expect open class TestBase constructor() {
Expand Down Expand Up @@ -56,12 +57,14 @@ public suspend inline fun <reified T : Throwable> assertFailsWith(flow: Flow<*>)
public suspend fun Flow<Int>.sum() = fold(0) { acc, value -> acc + value }
public suspend fun Flow<Long>.longSum() = fold(0L) { acc, value -> acc + value }

public class TestException(message: String? = null) : Throwable(message), NonRecoverableThrowable
public class TestException1(message: String? = null) : Throwable(message), NonRecoverableThrowable
public class TestException2(message: String? = null) : Throwable(message), NonRecoverableThrowable
public class TestException3(message: String? = null) : Throwable(message), NonRecoverableThrowable
public class TestCancellationException(message: String? = null) : CancellationException(message), NonRecoverableThrowable
public class TestRuntimeException(message: String? = null) : RuntimeException(message), NonRecoverableThrowable

// data is added to avoid stacktrace recovery because CopyableThrowable is not accessible from common modules
public class TestException(message: String? = null, private val data: Any? = null) : Throwable(message)
public class TestException1(message: String? = null, private val data: Any? = null) : Throwable(message)
public class TestException2(message: String? = null, private val data: Any? = null) : Throwable(message)
public class TestException3(message: String? = null, private val data: Any? = null) : Throwable(message)
public class TestCancellationException(message: String? = null, private val data: Any? = null) : CancellationException(message)
public class TestRuntimeException(message: String? = null, private val data: Any? = null) : RuntimeException(message)
public class RecoverableTestException(message: String? = null) : RuntimeException(message)
public class RecoverableTestCancellationException(message: String? = null) : CancellationException(message)

Expand Down
1 change: 0 additions & 1 deletion kotlinx-coroutines-core/jvm/src/Debug.kt
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public const val DEBUG_PROPERTY_NAME = "kotlinx.coroutines.debug"
* Stacktrace recovery mode wraps every exception into the exception of the same type with original exception
* as cause, but with stacktrace of the current coroutine.
* Exception is instantiated using reflection by using no-arg, cause or cause and message constructor.
* Stacktrace is not recovered if exception is an instance of [CancellationException] or [NonRecoverableThrowable].
*
* This mechanism is currently supported for channels, [async], [launch], [coroutineScope], [supervisorScope]
* and [withContext] builders.
Expand Down
11 changes: 4 additions & 7 deletions kotlinx-coroutines-core/jvm/src/internal/StackTraceRecovery.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import kotlin.coroutines.*
import kotlin.coroutines.intrinsics.*

internal actual fun <E : Throwable> recoverStackTrace(exception: E): E {
if (recoveryDisabled(exception)) return exception
if (!RECOVER_STACK_TRACES) return exception
// No unwrapping on continuation-less path: exception is not reported multiple times via slow paths
val copy = tryCopyException(exception) ?: return exception
return copy.sanitizeStackTrace()
Expand All @@ -39,7 +39,7 @@ private fun <E : Throwable> E.sanitizeStackTrace(): E {
}

internal actual fun <E : Throwable> recoverStackTrace(exception: E, continuation: Continuation<*>): E {
if (recoveryDisabled(exception) || continuation !is CoroutineStackFrame) return exception
if (!RECOVER_STACK_TRACES || continuation !is CoroutineStackFrame) return exception
return recoverFromStackFrame(exception, continuation)
}

Expand Down Expand Up @@ -133,15 +133,15 @@ private fun mergeRecoveredTraces(recoveredStacktrace: Array<StackTraceElement>,

@Suppress("NOTHING_TO_INLINE")
internal actual suspend inline fun recoverAndThrow(exception: Throwable): Nothing {
if (recoveryDisabled(exception)) throw exception
if (!RECOVER_STACK_TRACES) throw exception
suspendCoroutineUninterceptedOrReturn<Nothing> {
if (it !is CoroutineStackFrame) throw exception
throw recoverFromStackFrame(exception, it)
}
}

internal actual fun <E : Throwable> unwrap(exception: E): E {
if (recoveryDisabled(exception)) return exception
if (!RECOVER_STACK_TRACES) return exception
val cause = exception.cause
// Fast-path to avoid array cloning
if (cause == null || cause.javaClass != exception.javaClass) {
Expand All @@ -156,9 +156,6 @@ internal actual fun <E : Throwable> unwrap(exception: E): E {
}
}

private fun <E : Throwable> recoveryDisabled(exception: E) =
!RECOVER_STACK_TRACES || exception is NonRecoverableThrowable

private fun createStackTrace(continuation: CoroutineStackFrame): ArrayDeque<StackTraceElement> {
val stack = ArrayDeque<StackTraceElement>()
continuation.getStackTraceElement()?.let { stack.add(it) }
Expand Down