awaitAll and joinAll extensions #323
Conversation
| * this function immediately resumes with [CancellationException]. | ||
| */ | ||
| public suspend fun Iterable<Job>.awaitAll(): Unit { | ||
| val jobs = this as? Collection<Job> ?: this.toList() |
There was a problem hiding this comment.
What if jobs collection is changed after calling .awaitAll() extension on it?
There was a problem hiding this comment.
Depends on mutation. In the worst case, awaitAll will hang (if concurrent mutator removes an element while awaitAll prepares its handles), but a non-trivial effort is required to write such code.
Behaviour in such case is undefined, what does user expect if he creates a collection of jobs, shares it and mutates in one coroutine while awaiting them in another?
There was a problem hiding this comment.
So, semantically we want awaitAll to operate on a some kind of snapshot. The challenge is how far we are ready to go for an optimal implementation. The easy, but not efficient, way it so to always take a snapshot of collection first, then await on it.
| @@ -20,7 +20,8 @@ public suspend fun awaitAll(vararg jobs: Job): Unit = jobs.asList().awaitAll() | |||
| */ | |||
| public suspend fun Collection<Job>.awaitAll() { | |||
| if (isEmpty()) return | |||
| AwaitAll(this).await() | |||
| val snapshot = ArrayList(this) | |||
There was a problem hiding this comment.
Better to use toList, it can exploit collection immutability, when we have one.
| * If the [Job] of the current coroutine is cancelled or completed while this suspending function is waiting, | ||
| * this function immediately resumes with [CancellationException]. | ||
| */ | ||
| public suspend fun awaitAll(vararg jobs: Job): Unit = jobs.asList().awaitAll() |
There was a problem hiding this comment.
This list can be awaited without taking snapshot.
…lias to a cause. CompletionHandler receives JobCancellationException if it was invoked during regular cancellation. Test coverage for cancellation paths improved.
…disabling stacktraces of JobCancellationException in production mode
…handle concurrent mutations, make handler JobNode to avoid extra object allocation
JobCancellationException.job is made internal.
# Conflicts: # common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/channels/ArrayBroadcastChannelTest.kt # common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/channels/ArrayChannelTest.kt # common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/channels/ConflatedChannelTest.kt # common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/channels/LinkedListChannelTest.kt # common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/channels/ProduceTest.kt # common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/channels/RendezvousChannelTest.kt
Invoke clearTimeout on cancellation in JSDispatcher Remove delayed task on cancellation from ThreadSafeHeap on cancellation to avoid memory leak
|
Merged! |
|
Hi! I was wondering when this will be released. And if You could update the coroutines guidel saying awaitAll is coming, I just spent hours trying select with onAwait() combo to accomplish similar thing from https://github.com/Kotlin/kotlinx.coroutines/blob/master/coroutines-guide.md#switch-over-a-channel-of-deferred-values and ended up implementing my own version of awaitAll() . Let me try too uptake new version of coroutines instead. Thanks! |
|
Hi @olgachotinun
We can't really do it, having "what's coming in the next release" in the guide is slightly confusing, especially for "planned" public API which is frequently changed during initial implementation |
Fixes #171