Skip to content

Commit b4f00c0

Browse files
committed
Consistently unwrap exception in slow path of CompletionStage.asDeferred
* A dedicated test is added that checks consistency of unwrapping between slow and fast paths of both CompletionStage.await and CompletionStage.asDeferred implementations. * In the fast-path for both of them implementation of CompletableFuture.get() does unwrapping of CompletionException, taking its cause when it is not null. To mimic this behavior, the slow path of both await and asDeferred shall perform similar unwrapping. Fixes #1479
1 parent 7672c52 commit b4f00c0

File tree

2 files changed

+48
-8
lines changed

2 files changed

+48
-8
lines changed

integration/kotlinx-coroutines-jdk8/src/future/Future.kt

+6-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
2+
* Copyright 2016-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
33
*/
44

55
package kotlinx.coroutines.future
@@ -124,9 +124,12 @@ public fun <T> CompletionStage<T>.asDeferred(): Deferred<T> {
124124
val result = CompletableDeferred<T>()
125125
whenComplete { value, exception ->
126126
if (exception == null) {
127+
// the future has completed normally
127128
result.complete(value)
128129
} else {
129-
result.completeExceptionally(exception)
130+
// the future has completed with an exception, unwrap it consistently with fast path
131+
// Note: In the fast-path the implementation of CompletableFuture.get() does unwrapping
132+
result.completeExceptionally((exception as? CompletionException)?.cause ?: exception)
130133
}
131134
}
132135
if (this is Future<*>) result.cancelFutureOnCompletion(this)
@@ -171,7 +174,7 @@ private class ContinuationConsumer<T>(
171174
override fun accept(result: T?, exception: Throwable?) {
172175
val cont = this.cont ?: return // atomically read current value unless null
173176
if (exception == null) {
174-
// the future has been completed normally
177+
// the future has completed normally
175178
cont.resume(result as T)
176179
} else {
177180
// the future has completed with an exception, unwrap it to provide consistent view of .await() result and to propagate only original exception

integration/kotlinx-coroutines-jdk8/test/future/FutureTest.kt

+42-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
2+
* Copyright 2016-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
33
*/
44

55
package kotlinx.coroutines.future
@@ -264,11 +264,9 @@ class FutureTest : TestBase() {
264264
try {
265265
deferred.await()
266266
fail("deferred.await() should throw an exception")
267-
} catch (e: CompletionException) {
267+
} catch (e: TestException) {
268268
assertTrue(deferred.isCancelled)
269-
val cause = e.cause?.cause!! // Stacktrace augmentation
270-
assertTrue(cause is TestException)
271-
assertEquals("something went wrong", cause.message)
269+
assertEquals("something went wrong", e.message)
272270
}
273271
}
274272

@@ -437,6 +435,45 @@ class FutureTest : TestBase() {
437435
}
438436
}
439437

438+
/**
439+
* Tests that both [CompletionStage.await] and [CompletionStage.asDeferred] consistently unwrap
440+
* [CompletionException] both in their slow and fast paths.
441+
* See [issue #1479](https://github.com/Kotlin/kotlinx.coroutines/issues/1479).
442+
*/
443+
@Test
444+
fun testConsistentExceptionUnwrapping() = runTest {
445+
expect(1)
446+
// Check the fast path
447+
val fFast = CompletableFuture.supplyAsync {
448+
expect(2)
449+
throw TestException()
450+
}
451+
fFast.checkFutureException<TestException>() // wait until it completes
452+
// Fast path in await and asDeferred.await() shall produce TestException
453+
expect(3)
454+
val dFast = fFast.asDeferred()
455+
assertFailsWith<TestException> { fFast.await() }
456+
assertFailsWith<TestException> { dFast.await() }
457+
// Same test, but future has not completed yet, check the slow path
458+
expect(4)
459+
val barrier = CyclicBarrier(2)
460+
val fSlow = CompletableFuture.supplyAsync {
461+
barrier.await()
462+
expect(6)
463+
throw TestException()
464+
}
465+
val dSlow = fSlow.asDeferred()
466+
launch(start = CoroutineStart.UNDISPATCHED) {
467+
expect(5)
468+
// Slow path on await shall produce TestException, too
469+
assertFailsWith<TestException> { fSlow.await() } // will suspend here
470+
assertFailsWith<TestException> { dSlow.await() }
471+
finish(7)
472+
}
473+
barrier.await()
474+
fSlow.checkFutureException<TestException>() // now wait until it completes
475+
}
476+
440477
private inline fun <reified T: Throwable> CompletableFuture<*>.checkFutureException(vararg suppressed: KClass<out Throwable>) {
441478
val e = assertFailsWith<ExecutionException> { get() }
442479
val cause = e.cause!!

0 commit comments

Comments
 (0)