From 8c1b4765b16b1adc452cbf6ccfb97aa40b1222d9 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Thu, 29 Aug 2019 16:18:56 +0200 Subject: [PATCH] [squash] minor improvements, better null handling --- .../src/ListenableFuture.kt | 64 ++++++++----------- .../test/ListenableFutureTest.kt | 16 +++-- 2 files changed, 37 insertions(+), 43 deletions(-) diff --git a/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt b/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt index de47c760a8..0d7a859c7a 100644 --- a/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt +++ b/integration/kotlinx-coroutines-guava/src/ListenableFuture.kt @@ -5,12 +5,11 @@ package kotlinx.coroutines.guava import com.google.common.util.concurrent.* -import com.google.common.util.concurrent.internal.InternalFutureFailureAccess -import com.google.common.util.concurrent.internal.InternalFutures -import java.util.concurrent.* -import kotlin.coroutines.* +import com.google.common.util.concurrent.internal.* import kotlinx.coroutines.* +import java.util.concurrent.* import java.util.concurrent.CancellationException +import kotlin.coroutines.* /** * Starts [block] in a new coroutine and returns a [ListenableFuture] pointing to its result. @@ -121,13 +120,7 @@ public fun ListenableFuture.asDeferred(): Deferred { if (isDone) { return try { val value = Uninterruptibles.getUninterruptibly(this) - if (value == null) { - CompletableDeferred().also { - it.completeExceptionally(KotlinNullPointerException()) - } - } else { - CompletableDeferred(value) - } + CompletableDeferred(value as T) } catch (e: CancellationException) { CompletableDeferred().also { it.cancel(e) } } catch (e: ExecutionException) { @@ -142,7 +135,8 @@ public fun ListenableFuture.asDeferred(): Deferred { val deferred = CompletableDeferred() Futures.addCallback(this, object : FutureCallback { override fun onSuccess(result: T?) { - deferred.complete(result!!) + @Suppress("UNCHECKED_CAST") + deferred.complete(result as T) } override fun onFailure(t: Throwable) { @@ -237,8 +231,9 @@ public suspend fun ListenableFuture.await(): T { return suspendCancellableCoroutine { cont: CancellableContinuation -> addListener( - ToContinuation(this, cont), - MoreExecutors.directExecutor()) + ToContinuation(this, cont), + MoreExecutors.directExecutor() + ) cont.invokeOnCancellation { cancel(false) } @@ -253,22 +248,23 @@ public suspend fun ListenableFuture.await(): T { * [ExecutionException] when thrown. */ private class ToContinuation( - val futureToObserve: ListenableFuture, - val continuation: CancellableContinuation + private val futureToObserve: ListenableFuture, + private val continuation: CancellableContinuation ): Runnable { override fun run() { if (futureToObserve.isCancelled) { continuation.cancel() - } else { - try { - continuation.resumeWith( - Result.success(Uninterruptibles.getUninterruptibly(futureToObserve))) - } catch (e: ExecutionException) { - // ExecutionException is the only kind of exception that can be thrown from a gotten - // Future. Anything else showing up here indicates a very fundamental bug in a - // Future implementation. - continuation.resumeWithException(e.nonNullCause()) - } + return + } + try { + continuation.resumeWith( + Result.success(Uninterruptibles.getUninterruptibly(futureToObserve)) + ) + } catch (e: ExecutionException) { + // ExecutionException is the only kind of exception that can be thrown from a gotten + // Future. Anything else showing up here indicates a very fundamental bug in a + // Future implementation. + continuation.resumeWithException(e.nonNullCause()) } } } @@ -339,14 +335,14 @@ private class ListenableFutureCoroutine( * - Fully correct cancellation and listener happens-after obeying [Future] and * [ListenableFuture]'s documented and implicit contracts is surprisingly difficult to achieve. * The best way to be correct, especially given the fun corner cases from - * [AsyncFuture.setAsync], is to just use an [AsyncFuture]. - * - To maintain sanity, this class implements [ListenableFuture] and uses an inner [AsyncFuture] + * [AbstractFuture.setFuture], is to just use an [AbstractFuture]. + * - To maintain sanity, this class implements [ListenableFuture] and uses an inner [AbstractFuture] * around its input [deferred] as a state engine to establish happens-after-completion. This - * could probably be compressed into one subclass of [AsyncFuture] to save an allocation, at the + * could probably be compressed into one subclass of [AbstractFuture] to save an allocation, at the * cost of the implementation's readability. */ private class OuterFuture(private val deferred: Deferred): ListenableFuture { - val innerFuture = DeferredListenableFuture(deferred) + private val innerFuture = DeferredListenableFuture(deferred) // Adding the listener after initialization resolves partial construction hairpin problem. // @@ -458,11 +454,7 @@ private class DeferredListenableFuture( * that certain combinations of cancelled/cancelling states can't be observed. */ override fun cancel(mayInterruptIfRunning: Boolean): Boolean { - return if (super.cancel(mayInterruptIfRunning)) { - deferred.cancel() - true - } else { - false - } + deferred.cancel() // Job.cancel is idempotent + return super.cancel(mayInterruptIfRunning) } } diff --git a/integration/kotlinx-coroutines-guava/test/ListenableFutureTest.kt b/integration/kotlinx-coroutines-guava/test/ListenableFutureTest.kt index 1f96dfdbea..4803634b70 100644 --- a/integration/kotlinx-coroutines-guava/test/ListenableFutureTest.kt +++ b/integration/kotlinx-coroutines-guava/test/ListenableFutureTest.kt @@ -427,19 +427,21 @@ class ListenableFutureTest : TestBase() { @Test fun testFutureCompletedWithNullAsDeferred() = runTest { val executor = MoreExecutors.listeningDecorator(ForkJoinPool.commonPool()) - val future = executor.submit(Callable { null }) - val deferred = GlobalScope.async { - future.asDeferred().await() - } - + val future = executor.submit(Callable { null }) try { - deferred.await() - expectUnreached() + future.asDeferred().await() } catch (e: Throwable) { assertTrue(e is KotlinNullPointerException) } } + @Test + fun testNullableFutureCompletedWithNullAsDeferred() = runTest { + val executor = MoreExecutors.listeningDecorator(ForkJoinPool.commonPool()) + val future = executor.submit(Callable { null }) + assertNull(future.asDeferred().await()) + } + @Test fun testThrowingFutureAsDeferred() = runTest { val executor = MoreExecutors.listeningDecorator(ForkJoinPool.commonPool())