Skip to content

Commit 1b22af7

Browse files
committed
Fix lost exception during cancellation in produce, fix integration with play services after rebase
1 parent 1e0a2f0 commit 1b22af7

File tree

5 files changed

+94
-36
lines changed

5 files changed

+94
-36
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ internal expect fun handleCoroutineExceptionImpl(context: CoroutineContext, exce
2020
* If invocation returned `true`, method terminates: now [Job] is responsible for handling an exception.
2121
* Otherwise, If there is [CoroutineExceptionHandler] in the context, it is used. If it throws an exception during handling
2222
* or is absent, all instances of [CoroutineExceptionHandler] found via [ServiceLoader] and [Thread.uncaughtExceptionHandler] are invoked
23-
* todo: Deprecate/hide this function.
2423
*/
2524
@JvmOverloads // binary compatibility
2625
@InternalCoroutinesApi

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,10 @@ internal open class JobSupport constructor(active: Boolean) : Job, ChildJob, Sel
197197
var suppressed = false
198198
val finalException = synchronized(state) {
199199
val exceptions = state.sealLocked(proposedException)
200-
val rootCause = getFinalRootCause(state, exceptions)
201-
if (rootCause != null) suppressed = suppressExceptions(rootCause, exceptions)
202-
rootCause
200+
val finalCause = getFinalRootCause(state, exceptions)
201+
// Report suppressed exceptions if initial cause doesn't match final cause (due to JCE unwrapping)
202+
if (finalCause != null) suppressed = suppressExceptions(finalCause, exceptions) || finalCause !== state.rootCause
203+
finalCause
203204
}
204205
// Create the final state object
205206
val finalState = when {
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
* Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package kotlinx.coroutines.experimental.exceptions
6+
7+
import kotlinx.coroutines.experimental.*
8+
import kotlinx.coroutines.experimental.channels.*
9+
import org.junit.Test
10+
import kotlin.test.*
11+
12+
class ProduceExceptionsTest : TestBase() {
13+
14+
@Test
15+
fun testFailingProduce() = runTest(unhandled = listOf({ e -> e is TestException })) {
16+
expect(1)
17+
val producer = produce<Int>(Job()) {
18+
expect(2)
19+
try {
20+
yield()
21+
} finally {
22+
expect(3)
23+
throw TestException()
24+
25+
}
26+
}
27+
28+
yield()
29+
producer.cancel()
30+
yield()
31+
finish(4)
32+
}
33+
34+
@Test
35+
fun testSuppressedExceptionUncaught() =
36+
runTest(unhandled = listOf({ e -> e is TestException && e.suppressed()[0] is TestException2 })) {
37+
val produce = produce<Int>(Job()) {
38+
launch(start = CoroutineStart.ATOMIC) {
39+
throw TestException()
40+
}
41+
try {
42+
delay(Long.MAX_VALUE)
43+
} finally {
44+
throw TestException2()
45+
}
46+
}
47+
48+
yield()
49+
produce.cancel()
50+
}
51+
52+
@Test
53+
fun testSuppressedException() = runTest {
54+
val produce = produce<Int>(Job()) {
55+
launch(start = CoroutineStart.ATOMIC) {
56+
throw TestException()
57+
}
58+
try {
59+
delay(Long.MAX_VALUE)
60+
} finally {
61+
throw TestException2()
62+
}
63+
}
64+
65+
try {
66+
produce.receive()
67+
expectUnreached()
68+
} catch (e: TestException) {
69+
assertTrue(e.suppressed()[0] is TestException2)
70+
}
71+
}
72+
73+
class TestException : Exception()
74+
class TestException2 : Exception()
75+
}

integration/kotlinx-coroutines-play-services/src/Tasks.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public fun <T> Deferred<T>.asTask(): Task<T> {
2525
val source = TaskCompletionSource<T>(cancellation.token)
2626

2727
invokeOnCompletion callback@{
28-
if (isCancelled) {
28+
if (it is CancellationException) {
2929
cancellation.cancel()
3030
return@callback
3131
}

integration/kotlinx-coroutines-play-services/test/TaskTest.kt

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import kotlinx.coroutines.experimental.TestBase
1414
import kotlinx.coroutines.experimental.async
1515
import kotlinx.coroutines.experimental.delay
1616
import kotlinx.coroutines.experimental.ignoreLostThreads
17-
import kotlinx.coroutines.experimental.runBlocking
1817
import org.hamcrest.core.IsEqual
1918
import org.junit.Assert
2019
import org.junit.Before
@@ -29,9 +28,9 @@ class TaskTest : TestBase() {
2928
}
3029

3130
@Test
32-
fun `Completed deferred as task`() = runBlocking {
31+
fun testCompletedDeferredAsTask() = runTest {
3332
expect(1)
34-
val deferred = async(coroutineContext, CoroutineStart.UNDISPATCHED) {
33+
val deferred = async(start = CoroutineStart.UNDISPATCHED) {
3534
expect(2) // Completed immediately
3635
"OK"
3736
}
@@ -42,9 +41,9 @@ class TaskTest : TestBase() {
4241
}
4342

4443
@Test
45-
fun `Deferred as task`() = runBlocking {
44+
fun testDeferredAsTask() = runTest {
4645
expect(1)
47-
val deferred = async(coroutineContext) {
46+
val deferred = async {
4847
expect(3) // Completed later
4948
"OK"
5049
}
@@ -55,53 +54,37 @@ class TaskTest : TestBase() {
5554
}
5655

5756
@Test
58-
fun `Threw cancellation exception as task`() {
59-
val deferred = GlobalScope.async {
60-
throw CancellationException()
61-
}
62-
63-
val task = deferred.asTask()
64-
try {
65-
runBlocking { task.await() }
66-
} catch (e: Exception) {
67-
Assert.assertTrue(e is CancellationException)
68-
Assert.assertFalse(task.isSuccessful)
69-
Assert.assertFalse(task.isCanceled)
70-
}
71-
}
72-
73-
@Test
74-
fun `Cancelled as task`() {
57+
fun testCancelledAsTask() {
7558
val deferred = GlobalScope.async {
7659
delay(100)
7760
}.apply { cancel() }
7861

7962
val task = deferred.asTask()
8063
try {
81-
runBlocking { task.await() }
64+
runTest { task.await() }
8265
} catch (e: Exception) {
8366
Assert.assertTrue(e is CancellationException)
8467
Assert.assertTrue(task.isCanceled)
8568
}
8669
}
8770

8871
@Test
89-
fun `Threw as task`() {
72+
fun testThrowingAsTask() {
9073
val deferred = GlobalScope.async {
9174
throw OutOfMemoryError()
9275
}
9376

9477
val task = deferred.asTask()
9578
try {
96-
runBlocking { task.await() }
79+
runTest { task.await() }
9780
} catch (e: RuntimeExecutionException) {
9881
Assert.assertFalse(task.isSuccessful)
9982
Assert.assertTrue(e.cause is OutOfMemoryError)
10083
}
10184
}
10285

10386
@Test
104-
fun `Task stages as task`() = runBlocking {
87+
fun testStateAsTask() = runTest {
10588
val lock = ReentrantLock().apply { lock() }
10689

10790
val deferred: Deferred<Int> = Tasks.call {
@@ -116,13 +99,13 @@ class TaskTest : TestBase() {
11699
}
117100

118101
@Test
119-
fun `Task as deferred`() = runBlocking {
102+
fun testTaskAsDeferred() = runTest {
120103
val deferred = Tasks.forResult(42).asDeferred()
121104
Assert.assertEquals(42, deferred.await())
122105
}
123106

124107
@Test
125-
fun `Cancelled task as deferred`() = runBlocking {
108+
fun testCancelledTaskAsDeferred() = runTest {
126109
val deferred = Tasks.forCanceled<Int>().asDeferred()
127110

128111
Assert.assertTrue(deferred.isCancelled)
@@ -135,10 +118,10 @@ class TaskTest : TestBase() {
135118
}
136119

137120
@Test
138-
fun `Failed task as deferred`() = runBlocking {
121+
fun testFailedTaskAsDeferred() = runTest {
139122
val deferred = Tasks.forException<Int>(TestException("something went wrong")).asDeferred()
140123

141-
Assert.assertTrue(deferred.isCompletedExceptionally)
124+
Assert.assertTrue(deferred.isCancelled && deferred.isCompleted)
142125
val completionException = deferred.getCompletionExceptionOrNull()!!
143126
Assert.assertTrue(completionException is TestException)
144127
Assert.assertEquals("something went wrong", completionException.message)
@@ -153,7 +136,7 @@ class TaskTest : TestBase() {
153136
}
154137

155138
@Test
156-
fun `Failed task stages as deferred`() = runBlocking {
139+
fun testFailingTaskAsDeferred() = runTest {
157140
val lock = ReentrantLock().apply { lock() }
158141

159142
val deferred: Deferred<Int> = Tasks.call {

0 commit comments

Comments
 (0)