diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml index 62fd5c7dfd..461a31ed76 100644 --- a/.idea/codeStyles/Project.xml +++ b/.idea/codeStyles/Project.xml @@ -12,4 +12,4 @@ - \ No newline at end of file + diff --git a/kotlinx-coroutines-core/jvm/src/internal/ExceptionsConstructor.kt b/kotlinx-coroutines-core/jvm/src/internal/ExceptionsConstructor.kt index de15225266..03308f6137 100644 --- a/kotlinx-coroutines-core/jvm/src/internal/ExceptionsConstructor.kt +++ b/kotlinx-coroutines-core/jvm/src/internal/ExceptionsConstructor.kt @@ -32,42 +32,48 @@ internal fun tryCopyException(exception: E): E? { private fun createConstructor(clz: Class): Ctor { val nullResult: Ctor = { null } // Pre-cache class - // Skip reflective copy if an exception has additional fields (that are usually populated in user-defined constructors) + // Skip reflective copy if an exception has additional fields (that are typically populated in user-defined constructors) if (throwableFields != clz.fieldsCountOrDefault(0)) return nullResult /* - * Try to reflectively find constructor(), constructor(message, cause), constructor(cause) or constructor(message). - * Exceptions are shared among coroutines, so we should copy exception before recovering current stacktrace. - */ - val constructors = clz.constructors.sortedByDescending { it.parameterTypes.size } - for (constructor in constructors) { - val result = createSafeConstructor(constructor) - if (result != null) return result - } - return nullResult -} - -private fun createSafeConstructor(constructor: Constructor<*>): Ctor? { - val p = constructor.parameterTypes - return when (p.size) { - 2 -> when { - p[0] == String::class.java && p[1] == Throwable::class.java -> - safeCtor { e -> constructor.newInstance(e.message, e) as Throwable } - else -> null + * Try to reflectively find constructor(message, cause), constructor(message), constructor(cause), or constructor(), + * in that order of priority. + * Exceptions are shared among coroutines, so we should copy exception before recovering current stacktrace. + * + * By default, Java's reflection iterates over ctors in the source-code order and the sorting is stable, so we can + * not rely on the order of iteration. Instead, we assign a unique priority to each ctor type. + */ + return clz.constructors.map { constructor -> + val p = constructor.parameterTypes + when (p.size) { + 2 -> when { + p[0] == String::class.java && p[1] == Throwable::class.java -> + safeCtor { e -> constructor.newInstance(e.message, e) as Throwable } to 3 + else -> null to -1 + } + 1 -> when (p[0]) { + String::class.java -> + safeCtor { e -> (constructor.newInstance(e.message) as Throwable).also { it.initCause(e) } } to 2 + Throwable::class.java -> + safeCtor { e -> constructor.newInstance(e) as Throwable } to 1 + else -> null to -1 + } + 0 -> safeCtor { e -> (constructor.newInstance() as Throwable).also { it.initCause(e) } } to 0 + else -> null to -1 } - 1 -> when (p[0]) { - Throwable::class.java -> - safeCtor { e -> constructor.newInstance(e) as Throwable } - String::class.java -> - safeCtor { e -> (constructor.newInstance(e.message) as Throwable).also { it.initCause(e) } } - else -> null - } - 0 -> safeCtor { e -> (constructor.newInstance() as Throwable).also { it.initCause(e) } } - else -> null - } + }.maxByOrNull(Pair<*, Int>::second)?.first ?: nullResult } -private inline fun safeCtor(crossinline block: (Throwable) -> Throwable): Ctor = - { e -> runCatching { block(e) }.getOrNull() } +private fun safeCtor(block: (Throwable) -> Throwable): Ctor = { e -> + runCatching { + val result = block(e) + /* + * Verify that the new exception has the same message as the original one (bail out if not, see #1631) + * or if the new message complies the contract from `Throwable(cause).message` contract. + */ + if (e.message != result.message && result.message != e.toString()) null + else result + }.getOrNull() +} private fun Class<*>.fieldsCountOrDefault(defaultValue: Int) = kotlin.runCatching { fieldsCount() }.getOrDefault(defaultValue) diff --git a/kotlinx-coroutines-core/jvm/src/internal/StackTraceRecovery.kt b/kotlinx-coroutines-core/jvm/src/internal/StackTraceRecovery.kt index 5193b0dc26..afc9989646 100644 --- a/kotlinx-coroutines-core/jvm/src/internal/StackTraceRecovery.kt +++ b/kotlinx-coroutines-core/jvm/src/internal/StackTraceRecovery.kt @@ -33,16 +33,16 @@ private val stackTraceRecoveryClassName = runCatching { internal actual fun recoverStackTrace(exception: E): E { if (!RECOVER_STACK_TRACES) return exception // No unwrapping on continuation-less path: exception is not reported multiple times via slow paths - val copy = tryCopyAndVerify(exception) ?: return exception + val copy = tryCopyException(exception) ?: return exception return copy.sanitizeStackTrace() } private fun E.sanitizeStackTrace(): E { val stackTrace = stackTrace val size = stackTrace.size - val lastIntrinsic = stackTrace.frameIndex(stackTraceRecoveryClassName) + val lastIntrinsic = stackTrace.indexOfLast { stackTraceRecoveryClassName == it.className } val startIndex = lastIntrinsic + 1 - val endIndex = stackTrace.frameIndex(baseContinuationImplClassName) + val endIndex = stackTrace.firstFrameIndex(baseContinuationImplClassName) val adjustment = if (endIndex == -1) 0 else size - endIndex val trace = Array(size - lastIntrinsic - adjustment) { if (it == 0) { @@ -70,7 +70,7 @@ private fun recoverFromStackFrame(exception: E, continuation: Co val (cause, recoveredStacktrace) = exception.causeAndStacktrace() // Try to create an exception of the same type and get stacktrace from continuation - val newException = tryCopyAndVerify(cause) ?: return exception + val newException = tryCopyException(cause) ?: return exception // Update stacktrace val stacktrace = createStackTrace(continuation) if (stacktrace.isEmpty()) return exception @@ -82,14 +82,6 @@ private fun recoverFromStackFrame(exception: E, continuation: Co return createFinalException(cause, newException, stacktrace) } -private fun tryCopyAndVerify(exception: E): E? { - val newException = tryCopyException(exception) ?: return null - // Verify that the new exception has the same message as the original one (bail out if not, see #1631) - // CopyableThrowable has control over its message and thus can modify it the way it wants - if (exception !is CopyableThrowable<*> && newException.message != exception.message) return null - return newException -} - /* * Here we partially copy original exception stackTrace to make current one much prettier. * E.g. for @@ -109,7 +101,7 @@ private fun tryCopyAndVerify(exception: E): E? { private fun createFinalException(cause: E, result: E, resultStackTrace: ArrayDeque): E { resultStackTrace.addFirst(ARTIFICIAL_FRAME) val causeTrace = cause.stackTrace - val size = causeTrace.frameIndex(baseContinuationImplClassName) + val size = causeTrace.firstFrameIndex(baseContinuationImplClassName) if (size == -1) { result.stackTrace = resultStackTrace.toTypedArray() return result @@ -157,7 +149,6 @@ private fun mergeRecoveredTraces(recoveredStacktrace: Array, } } -@Suppress("NOTHING_TO_INLINE") internal actual suspend inline fun recoverAndThrow(exception: Throwable): Nothing { if (!RECOVER_STACK_TRACES) throw exception suspendCoroutineUninterceptedOrReturn { @@ -198,7 +189,7 @@ private fun createStackTrace(continuation: CoroutineStackFrame): ArrayDeque.frameIndex(methodName: String) = indexOfFirst { methodName == it.className } +private fun Array.firstFrameIndex(methodName: String) = indexOfFirst { methodName == it.className } private fun StackTraceElement.elementWiseEquals(e: StackTraceElement): Boolean { /* diff --git a/kotlinx-coroutines-core/jvm/test-resources/stacktraces/channels/testReceiveFromChannel.txt b/kotlinx-coroutines-core/jvm/test-resources/stacktraces/channels/testReceiveFromChannel.txt index 64085ad329..da3558ba30 100644 --- a/kotlinx-coroutines-core/jvm/test-resources/stacktraces/channels/testReceiveFromChannel.txt +++ b/kotlinx-coroutines-core/jvm/test-resources/stacktraces/channels/testReceiveFromChannel.txt @@ -1,5 +1,4 @@ kotlinx.coroutines.RecoverableTestException - at kotlinx.coroutines.internal.StackTraceRecoveryKt.recoverStackTrace(StackTraceRecovery.kt) at kotlinx.coroutines.channels.BufferedChannel.receive$suspendImpl(BufferedChannel.kt) at kotlinx.coroutines.channels.BufferedChannel.receive(BufferedChannel.kt) at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest.channelReceive(StackTraceRecoveryChannelsTest.kt) @@ -7,4 +6,4 @@ kotlinx.coroutines.RecoverableTestException at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$channelReceive$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt) Caused by: kotlinx.coroutines.RecoverableTestException at kotlinx.coroutines.exceptions.StackTraceRecoveryChannelsTest$testReceiveFromChannel$1$job$1.invokeSuspend(StackTraceRecoveryChannelsTest.kt) - at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt) \ No newline at end of file + at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt) diff --git a/kotlinx-coroutines-core/jvm/test-resources/stacktraces/resume-mode/testUnconfined.txt b/kotlinx-coroutines-core/jvm/test-resources/stacktraces/resume-mode/testUnconfined.txt index a8461556d1..9fc7167833 100644 --- a/kotlinx-coroutines-core/jvm/test-resources/stacktraces/resume-mode/testUnconfined.txt +++ b/kotlinx-coroutines-core/jvm/test-resources/stacktraces/resume-mode/testUnconfined.txt @@ -1,5 +1,4 @@ kotlinx.coroutines.RecoverableTestException - at kotlinx.coroutines.internal.StackTraceRecoveryKt.recoverStackTrace(StackTraceRecovery.kt) at kotlinx.coroutines.channels.BufferedChannel.receive$suspendImpl(BufferedChannel.kt) at kotlinx.coroutines.channels.BufferedChannel.receive(BufferedChannel.kt) at kotlinx.coroutines.exceptions.StackTraceRecoveryResumeModeTest$withContext$2.invokeSuspend(StackTraceRecoveryResumeModeTest.kt) @@ -7,4 +6,4 @@ Caused by: kotlinx.coroutines.RecoverableTestException at kotlinx.coroutines.exceptions.StackTraceRecoveryResumeModeTest.testResumeModeFastPath(StackTraceRecoveryResumeModeTest.kt) at kotlinx.coroutines.exceptions.StackTraceRecoveryResumeModeTest.access$testResumeModeFastPath(StackTraceRecoveryResumeModeTest.kt) at kotlinx.coroutines.exceptions.StackTraceRecoveryResumeModeTest$testUnconfined$1.invokeSuspend(StackTraceRecoveryResumeModeTest.kt) - at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt) \ No newline at end of file + at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt) diff --git a/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryCustomExceptionsTest.kt b/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryCustomExceptionsTest.kt index d4e19040a5..0f987e56e0 100644 --- a/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryCustomExceptionsTest.kt +++ b/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryCustomExceptionsTest.kt @@ -87,6 +87,26 @@ class StackTraceRecoveryCustomExceptionsTest : TestBase() { assertEquals("Token OK", ex.message) } + @Test + fun testNestedExceptionWithCause() = runTest { + val result = runCatching { + coroutineScope { + throw NestedException(IllegalStateException("ERROR")) + } + } + val ex = result.exceptionOrNull() ?: error("Expected to fail") + assertIs(ex) + assertIs(ex.cause) + val originalCause = ex.cause?.cause + assertIs(originalCause) + assertEquals("ERROR", originalCause.message) + } + + class NestedException : RuntimeException { + constructor(cause: Throwable) : super(cause) + constructor() : super() + } + @Test fun testWrongMessageExceptionInChannel() = runTest { val result = produce(SupervisorJob() + Dispatchers.Unconfined) {