Skip to content

Commit 1cbe8f0

Browse files
qwwdfsadelizarov
authored andcommitted
Unwrap exception on CompletionStage#await slow-path to provide consistent results
Fixes #375
1 parent bb19a26 commit 1cbe8f0

File tree

3 files changed

+174
-4
lines changed

3 files changed

+174
-4
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package kotlinx.coroutines.experimental.guava
2+
3+
import com.google.common.util.concurrent.*
4+
import kotlinx.coroutines.experimental.*
5+
import org.junit.Test
6+
import java.io.*
7+
import java.util.concurrent.*
8+
import kotlin.coroutines.experimental.*
9+
import kotlin.test.*
10+
11+
class ListenableFutureExceptionsTest : TestBase() {
12+
13+
@Test
14+
fun testAwait() {
15+
testException(IOException(), { it is IOException })
16+
}
17+
18+
@Test
19+
fun testAwaitChained() {
20+
testException(IOException(), { it is IOException }, { i -> i!! + 1 })
21+
}
22+
23+
@Test
24+
fun testAwaitCompletionException() {
25+
testException(CompletionException("test", IOException()), { it is CompletionException })
26+
}
27+
28+
@Test
29+
fun testAwaitChainedCompletionException() {
30+
testException(
31+
CompletionException("test", IOException()),
32+
{ it is CompletionException },
33+
{ i -> i!! + 1 })
34+
}
35+
36+
@Test
37+
fun testAwaitTestException() {
38+
testException(TestException(), { it is TestException })
39+
}
40+
41+
@Test
42+
fun testAwaitChainedTestException() {
43+
testException(TestException(), { it is TestException }, { i -> i!! + 1 })
44+
}
45+
46+
class TestException : CompletionException("test2")
47+
48+
private fun testException(
49+
exception: Exception,
50+
expected: ((Throwable) -> Boolean),
51+
transformer: ((Int?) -> Int?)? = null
52+
) {
53+
54+
// Fast path
55+
runTest {
56+
val future = SettableFuture.create<Int>()
57+
val chained = if (transformer == null) future else Futures.transform(future, transformer)
58+
future.setException(exception)
59+
try {
60+
chained.await()
61+
} catch (e: Exception) {
62+
assertTrue(expected(e))
63+
}
64+
}
65+
66+
// Slow path
67+
runTest {
68+
val future = SettableFuture.create<Int>()
69+
val chained = if (transformer == null) future else Futures.transform(future, transformer)
70+
launch(coroutineContext) {
71+
future.setException(exception)
72+
}
73+
74+
try {
75+
chained.await()
76+
} catch (e: Exception) {
77+
assertTrue(expected(e))
78+
}
79+
}
80+
}
81+
}

integration/kotlinx-coroutines-jdk8/src/main/kotlin/kotlinx/coroutines/experimental/future/Future.kt

+7-4
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,13 @@ private class ContinuationConsumer<T>(
188188
@Suppress("UNCHECKED_CAST")
189189
override fun accept(result: T?, exception: Throwable?) {
190190
val cont = this.cont ?: return // atomically read current value unless null
191-
if (exception == null) // the future has been completed normally
191+
if (exception == null) {
192+
// the future has been completed normally
192193
cont.resume(result as T)
193-
else // the future has completed with an exception
194-
cont.resumeWithException(exception)
194+
} else {
195+
// the future has completed with an exception, unwrap it to provide consistent view of .await() result and to propagate only original exception
196+
cont.resumeWithException((exception as? CompletionException)?.cause ?: exception)
197+
}
195198
}
196199
}
197200

@@ -214,4 +217,4 @@ public fun <T> Deferred<T>.toCompletableFuture(): CompletableFuture<T> = asCompl
214217
public fun <T> future(
215218
context: CoroutineContext = CommonPool,
216219
block: suspend () -> T
217-
): CompletableFuture<T> = future(context=context) { block() }
220+
): CompletableFuture<T> = future(context = context) { block() }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package kotlinx.coroutines.experimental.future
2+
3+
import kotlinx.coroutines.experimental.*
4+
import org.junit.Test
5+
import java.io.*
6+
import java.util.concurrent.*
7+
import kotlin.coroutines.experimental.*
8+
import kotlin.test.*
9+
10+
class FutureExceptionsTest : TestBase() {
11+
12+
@Test
13+
fun testAwait() {
14+
testException(IOException(), { it is IOException })
15+
}
16+
17+
@Test
18+
fun testAwaitChained() {
19+
testException(IOException(), { it is IOException }, { f -> f.thenApply { it + 1 } })
20+
}
21+
22+
@Test
23+
fun testAwaitDeepChain() {
24+
testException(IOException(), { it is IOException },
25+
{ f -> f
26+
.thenApply { it + 1 }
27+
.thenApply { it + 2 } })
28+
}
29+
30+
@Test
31+
fun testAwaitCompletionException() {
32+
testException(CompletionException("test", IOException()), { it is IOException })
33+
}
34+
35+
@Test
36+
fun testAwaitChainedCompletionException() {
37+
testException(CompletionException("test", IOException()), { it is IOException }, { f -> f.thenApply { it + 1 } })
38+
}
39+
40+
@Test
41+
fun testAwaitTestException() {
42+
testException(TestException(), { it is TestException })
43+
}
44+
45+
@Test
46+
fun testAwaitChainedTestException() {
47+
testException(TestException(), { it is TestException }, { f -> f.thenApply { it + 1 } })
48+
}
49+
50+
class TestException : CompletionException("test2")
51+
52+
private fun testException(
53+
exception: Exception,
54+
expected: ((Throwable) -> Boolean),
55+
transformer: (CompletableFuture<Int>) -> CompletableFuture<Int> = { it }
56+
) {
57+
58+
// Fast path
59+
runTest {
60+
val future = CompletableFuture<Int>()
61+
val chained = transformer(future)
62+
future.completeExceptionally(exception)
63+
try {
64+
chained.await()
65+
} catch (e: Exception) {
66+
assertTrue(expected(e))
67+
}
68+
}
69+
70+
// Slow path
71+
runTest {
72+
val future = CompletableFuture<Int>()
73+
val chained = transformer(future)
74+
75+
launch(coroutineContext) {
76+
future.completeExceptionally(exception)
77+
}
78+
79+
try {
80+
chained.await()
81+
} catch (e: Exception) {
82+
assertTrue(expected(e))
83+
}
84+
}
85+
}
86+
}

0 commit comments

Comments
 (0)