Skip to content

Commit

Permalink
Job unwraps all instances of CancellationException
Browse files Browse the repository at this point in the history
* This makes exception handling fully consistent, since CancellationException
  is considered "normal", never goes to logs and does not cause
  parent cancellation.
* Solves exception transparency of withTimeout/withTimeoutOrNull --
  exception throws from inside gets rethrown outside
* Makes sure that closed/cancelled producer that throws
  ClosedSendChannelException is not considered to be failed
  • Loading branch information
elizarov committed Sep 25, 2018
1 parent 852fae5 commit 541a9b6
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 86 deletions.
4 changes: 2 additions & 2 deletions common/kotlinx-coroutines-core-common/src/JobSupport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ internal open class JobSupport constructor(active: Boolean) : Job, ChildJob, Sel
* Note that it's only happening when both parent and child throw exception simultaneously.
*/
var rootCause = exceptions[0]
if (rootCause is JobCancellationException) {
if (rootCause is CancellationException) {
val cause = unwrap(rootCause)
rootCause = if (cause !== null) {
cause
Expand Down Expand Up @@ -275,7 +275,7 @@ internal open class JobSupport constructor(active: Boolean) : Job, ChildJob, Sel
}

private tailrec fun unwrap(exception: Throwable): Throwable? =
if (exception is JobCancellationException) {
if (exception is CancellationException) {
val cause = exception.cause
if (cause !== null) unwrap(cause) else null
} else {
Expand Down
4 changes: 0 additions & 4 deletions common/kotlinx-coroutines-core-common/src/Timeout.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import kotlin.coroutines.experimental.intrinsics.*
*
* The code that is executing inside the [block] is cancelled on timeout and the active or next invocation of
* cancellable suspending function inside the block throws [TimeoutCancellationException].
* Even if the code in the block suppresses [TimeoutCancellationException], it
* is still thrown by `withTimeout` invocation.
*
* The sibling function that does not throw exception on timeout is [withTimeoutOrNull].
* Note, that timeout action can be specified for [select] invocation with [onTimeout][SelectBuilder.onTimeout] clause.
Expand All @@ -40,8 +38,6 @@ public suspend fun <T> withTimeout(timeMillis: Long, block: suspend CoroutineSco
*
* The code that is executing inside the [block] is cancelled on timeout and the active or next invocation of
* cancellable suspending function inside the block throws [TimeoutCancellationException].
* **NB**: this method is exception-unsafe. Even if the code in the block suppresses [TimeoutCancellationException], this
* invocation of `withTimeoutOrNull` still returns `null` and thrown exception will be ignored.
*
* The sibling function that throws exception on timeout is [withTimeout].
* Note, that timeout action can be specified for [select] invocation with [onTimeout][SelectBuilder.onTimeout] clause.
Expand Down
25 changes: 15 additions & 10 deletions common/kotlinx-coroutines-core-common/test/WithTimeoutOrNullTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -190,19 +190,24 @@ class WithTimeoutOrNullTest : TestBase() {
@Test
fun testSuppressExceptionWithAnotherException() = runTest {
expect(1)
val value = withTimeoutOrNull(100) {
expect(2)
try {
delay(1000)
} catch (e: CancellationException) {
finish(3)
throw TestException()
try {
withTimeoutOrNull(100) {
expect(2)
try {
delay(1000)
} catch (e: CancellationException) {
expect(3)
throw TestException()
}
expectUnreached()
"OK"
}
expectUnreached()
"OK"
}
} catch (e: TestException) {
// catches TestException
finish(4)

assertNull(value)
}
}

private class TestException : Exception()
Expand Down
25 changes: 14 additions & 11 deletions common/kotlinx-coroutines-core-common/test/WithTimeoutTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -147,21 +147,24 @@ class WithTimeoutTest : TestBase() {
}

@Test
fun testSuppressExceptionWithAnotherException() = runTest(expected = { it is TimeoutCancellationException }) {
fun testSuppressExceptionWithAnotherException() = runTest{
expect(1)
withTimeout(100) {
expect(2)
try {
delay(1000)
} catch (e: CancellationException) {
finish(3)
throw TestException()
try {
withTimeout(100) {
expect(2)
try {
delay(1000)
} catch (e: CancellationException) {
expect(3)
throw TestException()
}
expectUnreached()
"OK"
}
expectUnreached()
"OK"
} catch (e: TestException) {
finish(4)
}

expectUnreached()
}

private class TestException : Exception()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class ProduceTest : TestBase() {
send(2) // will get cancelled
} catch (e: Throwable) {
finish(7)
check(e is JobCancellationException && e.job == coroutineContext[Job])
check(e is ClosedSendChannelException)
throw e
}
expectUnreached()
Expand Down
36 changes: 0 additions & 36 deletions core/kotlinx-coroutines-core/test/WithTimeoutJvmTest.kt

This file was deleted.

22 changes: 0 additions & 22 deletions core/kotlinx-coroutines-core/test/WithTimeoutOrNullJvmTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,6 @@ import java.io.*
import kotlin.test.*

class WithTimeoutOrNullJvmTest : TestBase() {


@Test
fun testCancellationSuppression() = runTest {

expect(1)
val value = withTimeoutOrNull(100) {
expect(2)
try {
delay(1000)
} catch (e: CancellationException) {
expect(3)
throw IOException()
}
expectUnreached()
"OK"
}

assertNull(value)
finish(4)
}

@Test
fun testOuterTimeoutFiredBeforeInner() = runTest {
val result = withTimeoutOrNull(100) {
Expand Down

0 comments on commit 541a9b6

Please sign in to comment.