Skip to content

Commit

Permalink
Remove requirement that job of the pre-created JobCancelNode have to …
Browse files Browse the repository at this point in the history
…be equal to outer job

Job is supposed to be "sealed" interface, but we also have non-sealed Deferred that can be successfully implemented via delegation and such delegation may break code (if assertions are enabled!) in a very subtle ways.

This assertion is our internal invariant that we're preserving anyway, so it's worth to lift it to simplify life of our users in the future

Fixes #2423
  • Loading branch information
qwwdfsad committed Dec 3, 2020
1 parent 556f07a commit fac5ec4
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
10 changes: 5 additions & 5 deletions kotlinx-coroutines-core/common/src/JobSupport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -509,12 +509,12 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
}

private fun makeNode(handler: CompletionHandler, onCancelling: Boolean): JobNode<*> {
return if (onCancelling)
(handler as? JobCancellingNode<*>)?.also { assert { it.job === this } }
?: InvokeOnCancelling(this, handler)
else
(handler as? JobNode<*>)?.also { assert { it.job === this && it !is JobCancellingNode } }
return if (onCancelling) {
handler as? JobCancellingNode<*> ?: InvokeOnCancelling(this, handler)
} else {
(handler as? JobNode<*>)?.also { assert { it !is JobCancellingNode } }
?: InvokeOnCompletion(this, handler)
}
}

private fun addLastAtomic(expect: Any, list: NodeList, node: JobNode<*>) =
Expand Down
14 changes: 14 additions & 0 deletions kotlinx-coroutines-core/common/test/AwaitTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -351,4 +351,18 @@ class AwaitTest : TestBase() {
async(NonCancellable) { throw TestException() }
joinAll(job, job, job)
}

@Test
fun testAwaitAllDelegates() = runTest {
expect(1)
val deferred = CompletableDeferred<String>()
val delegate = object : Deferred<String> by deferred {}
launch {
expect(3)
deferred.complete("OK")
}
expect(2)
awaitAll(delegate)
finish(4)
}
}

0 comments on commit fac5ec4

Please sign in to comment.