Skip to content

Commit 338b231

Browse files
qwwdfsadqwwdfsad
committed
Merge branch 'cancel-exception' into develop
# Conflicts: # kotlinx-coroutines-core/common/src/Job.kt
2 parents a38e7d5 + d202ed9 commit 338b231

File tree

74 files changed

+729
-501
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

74 files changed

+729
-501
lines changed

‎binary-compatibility-validator/reference-public-api/kotlinx-coroutines-core.txt

Lines changed: 53 additions & 35 deletions
Large diffs are not rendered by default.

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,8 @@ public fun <T> CoroutineScope.future(
4747

4848
private class ListenableFutureCoroutine<T>(
4949
context: CoroutineContext,
50-
private val completion: SettableFuture<T>
50+
private val future: SettableFuture<T>
5151
) : AbstractCoroutine<T>(context), FutureCallback<T> {
52-
5352
/*
5453
* We register coroutine as callback to the future this coroutine completes.
5554
* But when future is cancelled externally, we'd like to cancel coroutine,
@@ -66,12 +65,13 @@ private class ListenableFutureCoroutine<T>(
6665
}
6766

6867
override fun onCompleted(value: T) {
69-
completion.set(value)
68+
future.set(value)
7069
}
7170

72-
override fun onCompletedExceptionally(exception: Throwable) {
73-
if (!completion.setException(exception)) {
74-
handleCoroutineException(parentContext, exception, this)
71+
override fun handleJobException(exception: Throwable, handled: Boolean) {
72+
if (!future.setException(exception) && !handled) {
73+
// prevents loss of exception that was not handled by parent & could not be set to SettableFuture
74+
handleCoroutineException(context, exception)
7575
}
7676
}
7777
}

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

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,17 @@ class ListenableFutureTest : TestBase() {
4646
}
4747

4848
@Test
49-
fun testAwaitWithContextCancellation() = runTest(expected = {it is IOException}) {
49+
fun testAwaitWithCancellation() = runTest(expected = {it is TestCancellationException}) {
5050
val future = SettableFuture.create<Int>()
5151
val deferred = async {
5252
withContext(Dispatchers.Default) {
5353
future.await()
5454
}
5555
}
5656

57-
deferred.cancel(IOException())
58-
deferred.await()
57+
deferred.cancel(TestCancellationException())
58+
deferred.await() // throws TCE
59+
expectUnreached()
5960
}
6061

6162
@Test
@@ -258,13 +259,24 @@ class ListenableFutureTest : TestBase() {
258259
}
259260

260261
@Test
261-
fun testChildException() = runTest {
262+
fun testStructuredException() = runTest(
263+
expected = { it is TestException } // exception propagates to parent with structured concurrency
264+
) {
265+
val result = future<Int>(Dispatchers.Unconfined) {
266+
throw TestException("FAIL")
267+
}
268+
result.checkFutureException<TestException>()
269+
}
270+
271+
@Test
272+
fun testChildException() = runTest(
273+
expected = { it is TestException } // exception propagates to parent with structured concurrency
274+
) {
262275
val result = future(Dispatchers.Unconfined) {
263276
// child crashes
264277
launch { throw TestException("FAIL") }
265278
42
266279
}
267-
268280
result.checkFutureException<TestException>()
269281
}
270282

@@ -295,7 +307,26 @@ class ListenableFutureTest : TestBase() {
295307
throw TestException()
296308
}
297309
}
310+
result.cancel(true)
311+
finish(3)
312+
}
298313

314+
@Test
315+
fun testUnhandledExceptionOnExternalCancellation() = runTest(
316+
unhandled = listOf(
317+
{ it -> it is TestException } // exception is unhandled because there is no parent
318+
)
319+
) {
320+
expect(1)
321+
// No parent here (NonCancellable), so nowhere to propagate exception
322+
val result = future(NonCancellable + Dispatchers.Unconfined) {
323+
try {
324+
delay(Long.MAX_VALUE)
325+
} finally {
326+
expect(2)
327+
throw TestException() // this exception cannot be handled
328+
}
329+
}
299330
result.cancel(true)
300331
finish(3)
301332
}

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package kotlinx.coroutines.future
66

77
import kotlinx.coroutines.*
8+
import kotlinx.coroutines.CancellationException
89
import java.util.concurrent.*
910
import java.util.function.*
1011
import kotlin.coroutines.*
@@ -46,20 +47,20 @@ public fun <T> CoroutineScope.future(
4647

4748
private class CompletableFutureCoroutine<T>(
4849
context: CoroutineContext,
49-
private val completion: CompletableFuture<T>
50+
private val future: CompletableFuture<T>
5051
) : AbstractCoroutine<T>(context), BiConsumer<T?, Throwable?> {
51-
5252
override fun accept(value: T?, exception: Throwable?) {
5353
cancel()
5454
}
5555

5656
override fun onCompleted(value: T) {
57-
completion.complete(value)
57+
future.complete(value)
5858
}
5959

60-
override fun onCompletedExceptionally(exception: Throwable) {
61-
if (!completion.completeExceptionally(exception)) {
62-
handleCoroutineException(parentContext, exception, this)
60+
override fun handleJobException(exception: Throwable, handled: Boolean) {
61+
if (!future.completeExceptionally(exception) && !handled) {
62+
// prevents loss of exception that was not handled by parent & could not be set to CompletableFuture
63+
handleCoroutineException(context, exception)
6364
}
6465
}
6566
}
@@ -70,7 +71,11 @@ private class CompletableFutureCoroutine<T>(
7071
*/
7172
public fun <T> Deferred<T>.asCompletableFuture(): CompletableFuture<T> {
7273
val future = CompletableFuture<T>()
73-
future.whenComplete { _, exception -> cancel(exception) }
74+
future.whenComplete { _, exception ->
75+
cancel(exception?.let {
76+
it as? CancellationException ?: CancellationException("CompletableFuture was completed exceptionally", it)
77+
})
78+
}
7479
invokeOnCompletion {
7580
try {
7681
future.complete(getCompleted())

‎integration/kotlinx-coroutines-jdk8/test/examples/ExplicitJob-example.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ fun main(args: Array<String>) {
2626
log("g should not execute this line")
2727
}
2828
log("Started futures f && g... will not wait -- cancel them!!!")
29-
job.cancel(CancellationException("I don't want it"))
29+
job.cancel()
3030
check(f.isCancelled)
3131
check(g.isCancelled)
3232
log("f result = ${Try<Unit> { f.get() }}")

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

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -368,29 +368,39 @@ class FutureTest : TestBase() {
368368
}
369369

370370
@Test
371-
fun testChildException() = runTest {
371+
fun testStructuredException() = runTest(
372+
expected = { it is TestException } // exception propagates to parent with structured concurrency
373+
) {
374+
val result = future<Int>(Dispatchers.Unconfined) {
375+
throw TestException("FAIL")
376+
}
377+
result.checkFutureException<TestException>()
378+
}
379+
380+
@Test
381+
fun testChildException() = runTest(
382+
expected = { it is TestException } // exception propagates to parent with structured concurrency
383+
) {
372384
val result = future(Dispatchers.Unconfined) {
373385
// child crashes
374386
launch { throw TestException("FAIL") }
375387
42
376388
}
377-
378389
result.checkFutureException<TestException>()
379390
}
380391

381392
@Test
382-
fun testExceptionAggregation() = runTest {
393+
fun testExceptionAggregation() = runTest(
394+
expected = { it is TestException } // exception propagates to parent with structured concurrency
395+
) {
383396
val result = future(Dispatchers.Unconfined) {
384397
// child crashes
385398
launch(start = CoroutineStart.ATOMIC) { throw TestException1("FAIL") }
386399
launch(start = CoroutineStart.ATOMIC) { throw TestException2("FAIL") }
387400
throw TestException()
388401
}
389-
390-
expect(1)
391402
result.checkFutureException<TestException>(TestException1::class, TestException2::class)
392-
yield()
393-
finish(2) // we are not cancelled
403+
finish(1)
394404
}
395405

396406
@Test
@@ -409,7 +419,9 @@ class FutureTest : TestBase() {
409419
}
410420

411421
@Test
412-
fun testExceptionOnExternalCompletion() = runTest(expected = {it is TestException}) {
422+
fun testExceptionOnExternalCompletion() = runTest(
423+
expected = { it is TestException } // exception propagates to parent with structured concurrency
424+
) {
413425
expect(1)
414426
val result = future(Dispatchers.Unconfined) {
415427
try {
@@ -419,7 +431,26 @@ class FutureTest : TestBase() {
419431
throw TestException()
420432
}
421433
}
434+
result.complete(Unit)
435+
finish(3)
436+
}
422437

438+
@Test
439+
fun testUnhandledExceptionOnExternalCompletion() = runTest(
440+
unhandled = listOf(
441+
{ it -> it is TestException } // exception is unhandled because there is no parent
442+
)
443+
) {
444+
expect(1)
445+
// No parent here (NonCancellable), so nowhere to propagate exception
446+
val result = future(NonCancellable + Dispatchers.Unconfined) {
447+
try {
448+
delay(Long.MAX_VALUE)
449+
} finally {
450+
expect(2)
451+
throw TestException() // this exception cannot be handled
452+
}
453+
}
423454
result.complete(Unit)
424455
finish(3)
425456
}

‎kotlinx-coroutines-core/common/src/AbstractCoroutine.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public abstract class AbstractCoroutine<in T>(
117117
}
118118

119119
internal final override fun handleOnCompletionException(exception: Throwable) {
120-
handleCoroutineException(parentContext, exception, this)
120+
handleCoroutineException(context, exception)
121121
}
122122

123123
internal override fun nameString(): String {

‎kotlinx-coroutines-core/common/src/Builders.common.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ private open class DeferredCoroutine<T>(
9494
parentContext: CoroutineContext,
9595
active: Boolean
9696
) : AbstractCoroutine<T>(parentContext, active), Deferred<T>, SelectClause1<T> {
97-
override val cancelsParent: Boolean get() = true
9897
override fun getCompleted(): T = getCompletedInternal() as T
9998
override suspend fun await(): T = awaitInternal() as T
10099
override val onAwait: SelectClause1<T> get() = this
@@ -180,8 +179,9 @@ private open class StandaloneCoroutine(
180179
parentContext: CoroutineContext,
181180
active: Boolean
182181
) : AbstractCoroutine<Unit>(parentContext, active) {
183-
override val cancelsParent: Boolean get() = true
184-
override fun handleJobException(exception: Throwable) = handleExceptionViaHandler(parentContext, exception)
182+
override fun handleJobException(exception: Throwable, handled: Boolean) {
183+
if (!handled) handleCoroutineException(context, exception)
184+
}
185185
}
186186

187187
private class LazyStandaloneCoroutine(

‎kotlinx-coroutines-core/common/src/CancellableContinuationImpl.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ internal open class CancellableContinuationImpl<in T>(
121121
try {
122122
block()
123123
} catch (ex: Throwable) {
124+
// Handler should never fail, if it does -- it is an unhandled exception
124125
handleCoroutineException(
125126
context,
126127
CompletionHandlerException("Exception in cancellation handler for $this", ex)

‎kotlinx-coroutines-core/common/src/CompletableDeferred.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ private class CompletableDeferredImpl<T>(
6666
parent: Job?
6767
) : JobSupport(true), CompletableDeferred<T>, SelectClause1<T> {
6868
init { initParentJobInternal(parent) }
69-
override val cancelsParent: Boolean get() = true
7069
override val onCancelComplete get() = true
7170
override fun getCompleted(): T = getCompletedInternal() as T
7271
override suspend fun await(): T = awaitInternal() as T

0 commit comments

Comments
 (0)