Skip to content

Commit 4ecf469

Browse files
committed
Do not throw from JobListenableFuture.isCancelled
This properly handles ExecutionException that can be thrown from getUninterruptibly. Fixed Kotlin#2421.
1 parent 2ec1290 commit 4ecf469

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

integration/kotlinx-coroutines-guava/src/ListenableFuture.kt

+16-1
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,24 @@ private class JobListenableFuture<T>(private val jobToCancel: Job): ListenableFu
385385
// this Future hasn't itself been successfully cancelled, the Future will return
386386
// isCancelled() == false. This is the only discovered way to reconcile the two different
387387
// cancellation contracts.
388-
return auxFuture.isCancelled || (isDone && Uninterruptibles.getUninterruptibly(auxFuture) is Cancelled)
388+
return auxFuture.isCancelled || auxFuture.completedWithCancellation
389389
}
390390

391+
/**
392+
* Helper for [isCancelled] that takes into account that
393+
* our auxiliary future can complete with [Cancelled] instance.
394+
*/
395+
private val SettableFuture<*>.completedWithCancellation: Boolean
396+
get() = isDone && try {
397+
Uninterruptibles.getUninterruptibly(this) is Cancelled
398+
} catch (e: CancellationException) {
399+
true
400+
} catch (t: Throwable) {
401+
// In theory appart from CancellationException, getUninterruptibly can only
402+
// throw ExecutionException, but to be safe we catch Throwable here.
403+
false
404+
}
405+
391406
/**
392407
* Waits for [auxFuture] to complete by blocking, then uses its `result`
393408
* to get the `T` value `this` [ListenableFuture] is pointing to or throw a [CancellationException].

integration/kotlinx-coroutines-guava/test/ListenableFutureTest.kt

+27
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,33 @@ class ListenableFutureTest : TestBase() {
680680
finish(5)
681681
}
682682

683+
@Test
684+
fun testFutureCompletedExceptionally() = runTest {
685+
val testException = TestException()
686+
// NonCancellable to not propagate error to this scope.
687+
val future = future(context = NonCancellable) {
688+
throw testException
689+
}
690+
yield()
691+
assertTrue(future.isDone)
692+
assertFalse(future.isCancelled)
693+
val thrown = assertFailsWith<ExecutionException> { future.get() }
694+
assertEquals(testException, thrown.cause)
695+
}
696+
697+
@Test
698+
fun testAsListenableFutureCompletedExceptionally() = runTest {
699+
val testException = TestException()
700+
val deferred = CompletableDeferred<String>().apply {
701+
completeExceptionally(testException)
702+
}
703+
val asListenableFuture = deferred.asListenableFuture()
704+
assertTrue(asListenableFuture.isDone)
705+
assertFalse(asListenableFuture.isCancelled)
706+
val thrown = assertFailsWith<ExecutionException> { asListenableFuture.get() }
707+
assertEquals(testException, thrown.cause)
708+
}
709+
683710
private inline fun <reified T: Throwable> ListenableFuture<*>.checkFutureException() {
684711
val e = assertFailsWith<ExecutionException> { get() }
685712
val cause = e.cause!!

0 commit comments

Comments
 (0)