Simplify internal handling of suppressed exceptions (#4007)

Author: dkhalanskyjb

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

@@ -35,7 +35,7 @@ public fun handleCoroutineException(context: CoroutineContext, exception: Throwa
 internal fun handlerException(originalException: Throwable, thrownException: Throwable): Throwable {
     if (originalException === thrownException) return originalException
     return RuntimeException("Exception while trying to handle coroutine exception", thrownException).apply {
-        addSuppressedThrowable(originalException)
+        addSuppressed(originalException)
     }
 }
 

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

@@ -26,6 +26,5 @@ internal expect class JobCancellationException(
 
 internal class CoroutinesInternalError(message: String, cause: Throwable) : Error(message, cause)
 
-internal expect fun Throwable.addSuppressedThrowable(other: Throwable)
 // For use in tests
 internal expect val RECOVER_STACK_TRACES: Boolean

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

@@ -281,7 +281,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
             val unwrapped = unwrap(exception)
             if (unwrapped !== rootCause && unwrapped !== unwrappedCause &&
                 unwrapped !is CancellationException && seenExceptions.add(unwrapped)) {
-                rootCause.addSuppressedThrowable(unwrapped)
+                rootCause.addSuppressed(unwrapped)
             }
         }
     }
@@ -369,7 +369,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
             try {
                 node.invoke(cause)
             } catch (ex: Throwable) {
-                exception?.apply { addSuppressedThrowable(ex) } ?: run {
+                exception?.apply { addSuppressed(ex) } ?: run {
                     exception =  CompletionHandlerException("Exception in completion handler $node for $this", ex)
                 }
             }

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

@@ -56,7 +56,7 @@ internal fun consumesAll(vararg channels: ReceiveChannel<*>): CompletionHandler
                 if (exception == null) {
                     exception = e
                 } else {
-                    exception.addSuppressedThrowable(e)
+                    exception.addSuppressed(e)
                 }
             }
         exception?.let { throw it }

kotlinx-coroutines-core/common/src/flow/operators/Emitters.kt

@@ -215,7 +215,7 @@ private suspend fun <T> FlowCollector<T>.invokeSafely(
     try {
         action(cause)
     } catch (e: Throwable) {
-        if (cause !== null && cause !== e) e.addSuppressedThrowable(cause)
+        if (cause !== null && cause !== e) e.addSuppressed(cause)
         throw e
     }
 }

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

@@ -139,7 +139,7 @@ internal abstract class DispatchedTask<in T> internal constructor(
     internal fun handleFatalException(exception: Throwable?, finallyException: Throwable?) {
         if (exception === null && finallyException === null) return
         if (exception !== null && finallyException !== null) {
-            exception.addSuppressedThrowable(finallyException)
+            exception.addSuppressed(finallyException)
         }
 
         val cause = exception ?: finallyException

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

@@ -19,7 +19,7 @@ internal fun <E> OnUndeliveredElement<E>.callUndeliveredElementCatchingException
         // undeliveredElementException.cause !== ex is an optimization in case the same exception is thrown
         // over and over again by on OnUndeliveredElement
         if (undeliveredElementException != null && undeliveredElementException.cause !== ex) {
-            undeliveredElementException.addSuppressedThrowable(ex)
+            undeliveredElementException.addSuppressed(ex)
         } else {
             return UndeliveredElementException("Exception in undelivered element handler for $element", ex)
         }

kotlinx-coroutines-core/concurrent/test/ConcurrentExceptionsStressTest.kt

@@ -48,7 +48,7 @@ class ConcurrentExceptionsStressTest : TestBase() {
         val completionException = deferred.getCompletionExceptionOrNull()
         val cause = completionException as? StressException
             ?: unexpectedException("completion", completionException)
-        val suppressed = cause.suppressed
+        val suppressed = cause.suppressedExceptions
         val indices = listOf(cause.index) + suppressed.mapIndexed { index, e ->
             (e as? StressException)?.index ?: unexpectedException("suppressed $index", e)
         }
@@ -62,6 +62,6 @@ class ConcurrentExceptionsStressTest : TestBase() {
         throw IllegalStateException("Unexpected $msg exception", e)
     }
 
-    private class StressException(val index: Int) : SuppressSupportingThrowable()
+    private class StressException(val index: Int) : Throwable()
 }
 

kotlinx-coroutines-core/concurrent/test/ConcurrentTestUtilities.common.kt

@@ -5,13 +5,26 @@
 package kotlinx.coroutines.exceptions
 
 import kotlinx.coroutines.*
+import kotlin.concurrent.Volatile
+import kotlin.random.*
 
-internal expect open class SuppressSupportingThrowable() : Throwable
-expect val Throwable.suppressed: Array<Throwable>
-// Unused on purpose, used manually during debugging sessions
-expect fun Throwable.printStackTrace()
+fun randomWait() {
+    val n = Random.nextInt(1000)
+    if (n < 500) return // no wait 50% of time
+    repeat(n) {
+        BlackHole.sink *= 3
+    }
+    // use the BlackHole value somehow, so even if the compiler gets smarter, it won't remove the object
+    val sinkValue = if (BlackHole.sink > 16) 1 else 0
+    if (n + sinkValue > 900) yieldThread()
+}
+
+private object BlackHole {
+    @Volatile
+    var sink = 1
+}
 
-expect fun randomWait()
+expect inline fun yieldThread()
 
 expect fun currentThreadName(): String
 

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

@@ -35,8 +35,5 @@ internal actual class JobCancellationException public actual constructor(
         (message!!.hashCode() * 31 + job.hashCode()) * 31 + (cause?.hashCode() ?: 0)
 }
 
-@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

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

@@ -68,7 +68,3 @@ internal actual class JobCancellationException public actual constructor(
     override fun hashCode(): Int =
         (message!!.hashCode() * 31 + job.hashCode()) * 31 + (cause?.hashCode() ?: 0)
 }
-
-@Suppress("NOTHING_TO_INLINE")
-internal actual inline fun Throwable.addSuppressedThrowable(other: Throwable) =
-    addSuppressed(other)

kotlinx-coroutines-core/jvm/test/ConcurrentTestUtilities.kt

@@ -4,26 +4,6 @@
 
 package kotlinx.coroutines.exceptions
 
-import kotlin.random.*
-
-actual fun randomWait() {
-    val n = Random.nextInt(1000)
-    if (n < 500) return // no wait 50% of time
-    repeat(n) {
-        BlackHole.sink *= 3
-    }
-    if (n > 900) Thread.yield()
-}
-
-private object BlackHole {
-    @Volatile
-    var sink = 1
-}
-
-@Suppress("ACTUAL_WITHOUT_EXPECT")
-internal actual typealias SuppressSupportingThrowable = Throwable
-
-@Suppress("EXTENSION_SHADOWED_BY_MEMBER", "unused")
-actual fun Throwable.printStackTrace() = printStackTrace()
+actual inline fun yieldThread() { Thread.yield() }
 
 actual fun currentThreadName(): String = Thread.currentThread().name

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

@@ -10,17 +10,6 @@ import java.util.*
 import kotlin.coroutines.*
 import kotlin.test.*
 
-/**
- * Proxy for [Throwable.getSuppressed] for tests, which are compiled for both JDK 1.6 and JDK 1.8,
- * but run only under JDK 1.8
- */
-@Suppress("ConflictingExtensionProperty")
-actual val Throwable.suppressed: Array<Throwable> get() {
-    val method = this::class.java.getMethod("getSuppressed") ?: error("This test can only be run using JDK 1.7")
-    @Suppress("UNCHECKED_CAST")
-    return method.invoke(this) as Array<Throwable>
-}
-
 internal inline fun <reified T : Throwable> checkException(exception: Throwable): Boolean {
     assertTrue(exception is T)
     assertTrue(exception.suppressed.isEmpty())

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

@@ -4,9 +4,6 @@
 
 package kotlinx.coroutines
 
-import kotlinx.coroutines.internal.*
-import kotlinx.coroutines.internal.SuppressSupportingThrowableImpl
-
 /**
  * Thrown by cancellable suspending functions if the [Job] of the coroutine is cancelled while it is suspending.
  * It indicates _normal_ cancellation of a coroutine.
@@ -38,10 +35,5 @@ internal actual class JobCancellationException public actual constructor(
         (message!!.hashCode() * 31 + job.hashCode()) * 31 + (cause?.hashCode() ?: 0)
 }
 
-@Suppress("NOTHING_TO_INLINE")
-internal actual inline fun Throwable.addSuppressedThrowable(other: Throwable) {
-    if (this is SuppressSupportingThrowableImpl) addSuppressed(other)
-}
-
 // For use in tests
 internal actual val RECOVER_STACK_TRACES: Boolean = false

kotlinx-coroutines-core/native/src/internal/Concurrent.kt

@@ -16,20 +16,5 @@ internal actual inline fun <T> ReentrantLock.withLock(action: () -> T): T = this
 
 internal actual fun <E> identitySet(expectedSize: Int): MutableSet<E> = HashSet()
 
-
-// "Suppress-supporting throwable" is currently used for tests only
-internal open class SuppressSupportingThrowableImpl : Throwable() {
-    private val _suppressed = atomic<Array<Throwable>>(emptyArray())
-
-    val suppressed: Array<Throwable>
-        get() = _suppressed.value
-
-    fun addSuppressed(other: Throwable) {
-        _suppressed.update { current ->
-            current + other
-        }
-    }
-}
-
 @Suppress("ACTUAL_WITHOUT_EXPECT") // This suppress can be removed in 2.0: KT-59355
 internal actual typealias BenignDataRace = kotlin.concurrent.Volatile

kotlinx-coroutines-core/native/test/ConcurrentTestUtilities.kt

@@ -4,33 +4,9 @@
 
 package kotlinx.coroutines.exceptions
 
-import kotlinx.coroutines.internal.*
 import platform.posix.*
 import kotlin.native.concurrent.*
-import kotlin.random.*
 
-actual fun randomWait() {
-    val n = Random.nextInt(1000)
-    if (n < 500) return // no wait 50% of time
-    repeat(n) {
-        BlackHole.sink *= 3
-    }
-    // use the BlackHole value somehow, so even if the compiler gets smarter, it won't remove the object
-    val sinkValue = if (BlackHole.sink > 16) 1 else 0
-    if (n + sinkValue > 900) sched_yield()
-}
-
-@ThreadLocal
-private object BlackHole {
-    var sink = 1
-}
-
-internal actual typealias SuppressSupportingThrowable = SuppressSupportingThrowableImpl
-
-actual val Throwable.suppressed: Array<Throwable>
-    get() = (this as? SuppressSupportingThrowableImpl)?.suppressed ?: emptyArray()
-
-@Suppress("EXTENSION_SHADOWED_BY_MEMBER", "unused")
-actual fun Throwable.printStackTrace() = printStackTrace()
+actual inline fun yieldThread() { sched_yield() }
 
 actual fun currentThreadName(): String = Worker.current.name