Make JobCancellationException copyable in debug mode

This simplifies trouble-shooting of "what caused cancellation", as you
can print out exception and see its recovered stack trace.

Author: elizarov

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

@@ -25,4 +25,7 @@ internal expect class JobCancellationException(
 
 internal expect class DispatchException(message: String, cause: Throwable) : RuntimeException
 
-internal expect fun Throwable.addSuppressedThrowable(other: Throwable)
+internal expect fun Throwable.addSuppressedThrowable(other: Throwable)
+
+// For use in tests
+internal expect val RECOVER_STACK_TRACES: Boolean

kotlinx-coroutines-core/common/test/CoroutineScopeTest.kt

@@ -120,9 +120,14 @@ class CoroutineScopeTest : TestBase() {
             try {
                 callJobScoped()
                 expectUnreached()
-            } catch (e: CancellationException) {
+            } catch (e: JobCancellationException) {
                 expect(5)
-                assertNull(e.cause)
+                if (RECOVER_STACK_TRACES) {
+                    val cause = e.cause as JobCancellationException // shall be recovered JCE
+                    assertNull(cause.cause)
+                } else {
+                    assertNull(e.cause)
+                }
             }
         }
         repeat(3) { yield() } // let everything to start properly

kotlinx-coroutines-core/common/test/NonCancellableTest.kt

@@ -29,8 +29,13 @@ class NonCancellableTest : TestBase() {
         try {
             job.await()
             expectUnreached()
-        } catch (e: CancellationException) {
-            assertNull(e.cause)
+        } catch (e: JobCancellationException) {
+            if (RECOVER_STACK_TRACES) {
+                val cause = e.cause as JobCancellationException // shall be recovered JCE
+                assertNull(cause.cause)
+            } else {
+                assertNull(e.cause)
+            }
             finish(6)
         }
     }
@@ -119,8 +124,13 @@ class NonCancellableTest : TestBase() {
         try {
             job.await()
             expectUnreached()
-        } catch (e: CancellationException) {
-            assertNull(e.cause)
+        } catch (e: JobCancellationException) {
+            if (RECOVER_STACK_TRACES) {
+                val cause = e.cause as JobCancellationException // shall be recovered JCE
+                assertNull(cause.cause)
+            } else {
+                assertNull(e.cause)
+            }
             finish(7)
         }
     }

kotlinx-coroutines-core/js/src/Exceptions.kt

@@ -58,4 +58,7 @@ private fun String?.withCause(cause: Throwable?) =
     if (cause == null) this else "$this; caused by $cause"
 
 @Suppress("NOTHING_TO_INLINE")
-internal actual inline fun Throwable.addSuppressedThrowable(other: Throwable) { /* empty */ }
+internal actual inline fun Throwable.addSuppressedThrowable(other: Throwable) { /* empty */ }
+
+// For use in tests
+internal actual val RECOVER_STACK_TRACES: Boolean = false

kotlinx-coroutines-core/jvm/src/Debug.kt

@@ -48,8 +48,9 @@ public interface CopyableThrowable<T> where T : Throwable, T : CopyableThrowable
      * Creates a copy of the current instance.
      * For better debuggability, it is recommended to use original exception as [cause][Throwable.cause] of the resulting one.
      * Stacktrace of copied exception will be overwritten by stacktrace recovery machinery by [Throwable.setStackTrace] call.
+     * An exception can opt-out of copying by returning `null` from this function.
      */
-    public fun createCopy(): T
+    public fun createCopy(): T?
 }
 
 /**
@@ -77,8 +78,9 @@ internal val DEBUG = systemProp(DEBUG_PROPERTY_NAME).let { value ->
     }
 }
 
+// Note: stack-trace recovery is enabled only in debug mode
 @JvmField
-internal val RECOVER_STACKTRACES = systemProp(STACKTRACE_RECOVERY_PROPERTY_NAME, true)
+internal actual val RECOVER_STACK_TRACES = DEBUG && systemProp(STACKTRACE_RECOVERY_PROPERTY_NAME, true)
 
 // internal debugging tools
 

kotlinx-coroutines-core/jvm/src/Exceptions.kt

@@ -41,7 +41,7 @@ internal actual class JobCancellationException public actual constructor(
     message: String,
     cause: Throwable?,
     @JvmField internal actual val job: Job
-) : CancellationException(message) {
+) : CancellationException(message), CopyableThrowable<JobCancellationException> {
 
     init {
         if (cause != null) initCause(cause)
@@ -60,6 +60,17 @@ internal actual class JobCancellationException public actual constructor(
         return this
     }
 
+    override fun createCopy(): JobCancellationException? {
+        if (DEBUG) {
+            return JobCancellationException(message!!, this, job)
+        }
+
+        /*
+         * In non-debug mode we don't copy JCE for speed as it does not have the stack trace anyway.
+         */
+        return null
+    }
+
     override fun toString(): String = "${super.toString()}; job=$job"
 
     @Suppress("DEPRECATION")

kotlinx-coroutines-core/jvm/src/internal/ExceptionsConstuctor.kt

@@ -20,7 +20,7 @@ private val exceptionCtors: WeakHashMap<Class<out Throwable>, Ctor> = WeakHashMa
 internal fun <E : Throwable> tryCopyException(exception: E): E? {
     // Fast path for CopyableThrowable
     if (exception is CopyableThrowable<*>) {
-        return runCatching { exception.createCopy() as E }.getOrNull()
+        return runCatching { exception.createCopy() as E? }.getOrNull()
     }
     // Use cached ctor if found
     cacheLock.read { exceptionCtors[exception.javaClass] }?.let { cachedCtor ->

kotlinx-coroutines-core/jvm/src/internal/StackTraceRecovery.kt

@@ -12,9 +12,7 @@ import kotlin.coroutines.*
 import kotlin.coroutines.intrinsics.*
 
 internal actual fun <E : Throwable> recoverStackTrace(exception: E): E {
-    if (recoveryDisabled(exception)) {
-        return exception
-    }
+    if (recoveryDisabled(exception)) 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()
@@ -41,10 +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 (recoveryDisabled(exception) || continuation !is CoroutineStackFrame) return exception
     return recoverFromStackFrame(exception, continuation)
 }
 
@@ -146,26 +141,23 @@ internal actual suspend inline fun recoverAndThrow(exception: Throwable): Nothin
 }
 
 internal actual fun <E : Throwable> unwrap(exception: E): E {
-    if (recoveryDisabled(exception)) {
-        return exception
-    }
-
+    if (recoveryDisabled(exception)) return exception
     val cause = exception.cause
     // Fast-path to avoid array cloning
     if (cause == null || cause.javaClass != exception.javaClass) {
         return exception
     }
-
+    // Slow path looks for artificial frames in a stack-trace
     if (exception.stackTrace.any { it.isArtificial() }) {
         @Suppress("UNCHECKED_CAST")
-        return exception.cause as? E ?: exception
+        return cause as E
     } else {
         return exception
     }
 }
 
 private fun <E : Throwable> recoveryDisabled(exception: E) =
-    !RECOVER_STACKTRACES || !DEBUG || exception is NonRecoverableThrowable
+    !RECOVER_STACK_TRACES || exception is NonRecoverableThrowable
 
 private fun createStackTrace(continuation: CoroutineStackFrame): ArrayDeque<StackTraceElement> {
     val stack = ArrayDeque<StackTraceElement>()

kotlinx-coroutines-core/jvm/test/exceptions/WithContextExceptionHandlingTest.kt

@@ -171,8 +171,10 @@ class WithContextExceptionHandlingTest(private val mode: Mode) : TestBase() {
     @Test
     fun testCancel() = runTest {
         runOnlyCancellation(null) { e ->
-            assertNull(e.cause)
+            val cause = e.cause as JobCancellationException // shall be recovered JCE
+            assertNull(cause.cause)
             assertTrue(e.suppressed.isEmpty())
+            assertTrue(cause.suppressed.isEmpty())
         }
     }
 

kotlinx-coroutines-core/native/src/Exceptions.kt

@@ -59,3 +59,6 @@ private fun String?.withCause(cause: Throwable?) =
 
 @Suppress("NOTHING_TO_INLINE")
 internal actual inline fun Throwable.addSuppressedThrowable(other: Throwable) { /* empty */ }
+
+// For use in tests
+internal actual val RECOVER_STACK_TRACES: Boolean = false