More advanced check for self-suppression during the final exception b…

…uilding in Job with enabled stacktrace recovery * Efficiently prevent cycles for recovered1(original).addSuppressed(recovered2(original)) * Disable virtual time output in regular tests
StormGens · Apr 23, 2019 · f1710a7 · f1710a7
1 parent 26a14ee
commit f1710a7
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 13 deletions.
diff --git a/kotlinx-coroutines-core/common/src/JobSupport.kt b/kotlinx-coroutines-core/common/src/JobSupport.kt
@@ -250,9 +250,16 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
  private fun addSuppressedExceptions(rootCause: Throwable, exceptions: List<Throwable>) {
  if (exceptions.size <= 1) return // nothing more to do here
  val seenExceptions = identitySet<Throwable>(exceptions.size)
+ /*
+ * Note that root cause may be a recovered exception as well.
+ * To avoid cycles we unwrap the root cause and check for self-suppression against unwrapped cause,
+ * but add suppressed exceptions to the recovered root cause (as it is our final exception)
+ */
+ val unwrappedCause = unwrap(rootCause)
  for (exception in exceptions) {
  val unwrapped = unwrap(exception)
- if (unwrapped !== rootCause && unwrapped !is CancellationException && seenExceptions.add(unwrapped)) {
+ if (unwrapped !== rootCause && unwrapped !== unwrappedCause &&
+ unwrapped !is CancellationException && seenExceptions.add(unwrapped)) {
  rootCause.addSuppressedThrowable(unwrapped)
  }
  }

diff --git a/kotlinx-coroutines-core/jvm/test/VirtualTimeSource.kt b/kotlinx-coroutines-core/jvm/test/VirtualTimeSource.kt
@@ -10,7 +10,7 @@ import java.util.concurrent.locks.*
 
 private const val SHUTDOWN_TIMEOUT = 1000L
 
-internal inline fun withVirtualTimeSource(log: PrintStream = System.`out`, block: () -> Unit) {
+internal inline fun withVirtualTimeSource(log: PrintStream? = null, block: () -> Unit) {
  DefaultExecutor.shutdown(SHUTDOWN_TIMEOUT) // shutdown execution with old time source (in case it was working)
  val testTimeSource = VirtualTimeSource(log)
  timeSource = testTimeSource
@@ -41,7 +41,7 @@ private const val REAL_PARK_NANOS = 10_000_000L // 10 ms -- park for a little to
 
 @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN")
 internal class VirtualTimeSource(
- private val log: PrintStream
+ private val log: PrintStream?
 ) : TimeSource {
  private val mainThread: Thread = Thread.currentThread()
  private var checkpointNanos: Long = System.nanoTime()
@@ -138,7 +138,7 @@ internal class VirtualTimeSource(
  }
 
  private fun logTime(s: String) {
- log.println("[$s: Time = ${TimeUnit.NANOSECONDS.toMillis(time)} ms]")
+ log?.println("[$s: Time = ${TimeUnit.NANOSECONDS.toMillis(time)} ms]")
  }
 
  private fun minParkedTill(): Long =

diff --git a/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryTest.kt b/kotlinx-coroutines-core/jvm/test/exceptions/StackTraceRecoveryTest.kt
@@ -20,7 +20,7 @@ class StackTraceRecoveryTest : TestBase() {
  fun testAsync() = runTest {
  fun createDeferred(depth: Int): Deferred<*> {
  return if (depth == 0) {
- async(coroutineContext + NonCancellable) {
+ async<Unit>(coroutineContext + NonCancellable) {
  throw ExecutionException(null)
  }
  } else {
@@ -47,7 +47,7 @@ class StackTraceRecoveryTest : TestBase() {
 
  @Test
  fun testCompletedAsync() = runTest {
- val deferred = async(coroutineContext + NonCancellable) {
+ val deferred = async<Unit>(coroutineContext + NonCancellable) {
  throw ExecutionException(null)
  }
 
@@ -133,7 +133,7 @@ class StackTraceRecoveryTest : TestBase() {
 
  @Test
  fun testWithContext() = runTest {
- val deferred = async(NonCancellable, start = CoroutineStart.LAZY) {
+ val deferred = async<Unit>(NonCancellable, start = CoroutineStart.LAZY) {
  throw RecoverableTestException()
  }
 
@@ -152,25 +152,26 @@ class StackTraceRecoveryTest : TestBase() {
  deferred.join()
  }
 
- private suspend fun outerMethod(deferred: Deferred<Nothing>, vararg traces: String) {
+ private suspend fun outerMethod(deferred: Deferred<*>, vararg traces: String) {
  withContext(Dispatchers.IO) {
  innerMethod(deferred, *traces)
  }
 
  assertTrue(true)
  }
 
- private suspend fun innerMethod(deferred: Deferred<Nothing>, vararg traces: String) {
+ private suspend fun innerMethod(deferred: Deferred<*>, vararg traces: String) {
  try {
  deferred.await()
+ expectUnreached()
  } catch (e: RecoverableTestException) {
  verifyStackTrace(e, *traces)
  }
  }
 
  @Test
  fun testCoroutineScope() = runTest {
- val deferred = async(NonCancellable, start = CoroutineStart.LAZY) {
+ val deferred = async<Unit>(NonCancellable, start = CoroutineStart.LAZY) {
  throw RecoverableTestException()
  }
 
@@ -203,7 +204,7 @@ class StackTraceRecoveryTest : TestBase() {
 
  @Test
  fun testThrowingInitCause() = runTest {
- val deferred = async(NonCancellable) {
+ val deferred = async<Unit>(NonCancellable) {
  expect(2)
  throw TrickyException()
  }
@@ -217,7 +218,7 @@ class StackTraceRecoveryTest : TestBase() {
  }
  }
 
- private suspend fun outerScopedMethod(deferred: Deferred<Nothing>, vararg traces: String) = coroutineScope {
+ private suspend fun outerScopedMethod(deferred: Deferred<*>, vararg traces: String) = coroutineScope {
  supervisorScope {
  innerMethod(deferred, *traces)
  assertTrue(true)
@@ -228,7 +229,7 @@ class StackTraceRecoveryTest : TestBase() {
  @Test
  fun testSelect() = runTest {
  expect(1)
- val result = kotlin.runCatching { doSelect() }
+ val result = runCatching { doSelect() }
  expect(3)
  verifyStackTrace(result.exceptionOrNull()!!,
  "kotlinx.coroutines.RecoverableTestException\n" +
@@ -251,4 +252,23 @@ class StackTraceRecoveryTest : TestBase() {
  }
  }
  }
+
+ @Test
+ fun testSelfSuppression() = runTest {
+ try {
+ runBlocking {
+ val job = launch {
+ coroutineScope<Unit> {
+ throw RecoverableTestException()
+ }
+ }
+
+ job.join()
+ expectUnreached()
+ }
+ expectUnreached()
+ } catch (e: RecoverableTestException) {
+ checkCycles(e)
+ }
+ }
 }