Skip to content

Commit f2b4e0e

Browse files
qwwdfsadelizarov
authored andcommitted
Make exception handling in AbstractContinuation consistent: always prefer exception thrown from coroutine as exceptional reason, add cancellation cause as suppressed exception
Add workaround to work with suppressed exceptions in tests
1 parent e185ed6 commit f2b4e0e

File tree

5 files changed

+390
-10
lines changed

5 files changed

+390
-10
lines changed

common/kotlinx-coroutines-core-common/src/main/kotlin/kotlinx/coroutines/experimental/AbstractContinuation.kt

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,12 @@ internal abstract class AbstractContinuation<in T>(
234234
} else {
235235
/*
236236
* If already cancelled block is resumed with an exception,
237-
* then we should properly merge them to avoid information loss
237+
* then we should properly merge them to avoid information loss.
238+
*
239+
* General rule:
240+
* Thrown exception always becomes a result and cancellation reason
241+
* is added to suppressed exceptions if necessary.
242+
* Basic duplicate/cycles check is performed
238243
*/
239244
val update: CompletedExceptionally
240245

@@ -244,15 +249,15 @@ internal abstract class AbstractContinuation<in T>(
244249
* ```
245250
* T1: ctxJob.cancel(e1) // -> cancelling
246251
* T2:
247-
* withContext(ctx) {
252+
* withContext(ctx, Mode.ATOMIC) {
248253
* // -> resumed with cancellation exception
249254
* }
250255
* ```
251256
*/
252257
if (proposedUpdate.cause is CancellationException) {
253258
// Keep original cancellation cause and try add to suppressed exception from proposed cancel
254-
update = state.cancel
255-
coerceWithCancellation(state, proposedUpdate, update)
259+
update = proposedUpdate
260+
coerceWithException(state, update)
256261
} else {
257262
/*
258263
* Proposed update is exception => transition to terminal state
@@ -295,16 +300,16 @@ internal abstract class AbstractContinuation<in T>(
295300
}
296301
}
297302

298-
// Coerce current cancelling state with proposed cancellation
299-
private fun coerceWithCancellation(state: Cancelling, proposedUpdate: CompletedExceptionally, update: CompletedExceptionally) {
303+
// Coerce current cancelling state with proposed exception
304+
private fun coerceWithException(state: Cancelling, proposedUpdate: CompletedExceptionally) {
300305
val originalCancellation = state.cancel
301306
val originalException = originalCancellation.cause
302307
val updateCause = proposedUpdate.cause
303-
// Cause of proposed update is present and differs from one in current state TODO clashes with await all
308+
// Cause of proposed update is present and differs from one in current state
304309
val isSameCancellation = originalCancellation.cause is CancellationException
305310
&& originalException.cause === updateCause.cause
306-
if (!isSameCancellation && originalException.cause !== updateCause) {
307-
update.cause.addSuppressedThrowable(updateCause)
311+
if (!isSameCancellation && (originalException.cause !== updateCause)) {
312+
proposedUpdate.cause.addSuppressedThrowable(originalException)
308313
}
309314
}
310315

core/kotlinx-coroutines-core/build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ task lockFreedomTest(type: Test, dependsOn: testClasses) {
3737
include '**/*LFTest.*'
3838
}
3939

40-
4140
task jdk16Test(type: Test, dependsOn: [testClasses, checkJdk16]) {
4241
executable = "$System.env.JDK_16/bin/java"
4342
exclude '**/*LinearizabilityTest.*'
4443
exclude '**/*LFTest.*'
44+
exclude '**/*CancellableContinuationExceptionHandlingTest.*'
4545
}
4646

4747
task moreTest(dependsOn: [lockFreedomTest, jdk16Test])

0 commit comments

Comments
 (0)