Skip to content

Properly distinguish AbortFlowExceptions from different non-terminal … #1623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions kotlinx-coroutines-core/common/src/flow/internal/Combine.kt
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ internal fun <T1, T2, R> zipImpl(flow: Flow<T1>, flow2: Flow<T2>, transform: sus
* Invariant: this clause is invoked only when all elements from the channel were processed (=> rendezvous restriction).
*/
(second as SendChannel<*>).invokeOnClose {
if (!first.isClosedForReceive) first.cancel(AbortFlowException())
if (!first.isClosedForReceive) first.cancel(AbortFlowException(this@unsafeFlow))
}

val otherIterator = second.iterator()
Expand All @@ -126,9 +126,9 @@ internal fun <T1, T2, R> zipImpl(flow: Flow<T1>, flow2: Flow<T2>, transform: sus
emit(transform(NULL.unbox(value), NULL.unbox(otherIterator.next())))
}
} catch (e: AbortFlowException) {
// complete
e.checkOwnership(owner = this@unsafeFlow)
} finally {
if (!second.isClosedForReceive) second.cancel(AbortFlowException())
if (!second.isClosedForReceive) second.cancel(AbortFlowException(this@unsafeFlow))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,21 @@
package kotlinx.coroutines.flow.internal

import kotlinx.coroutines.*
import kotlinx.coroutines.flow.*

/**
* This exception is thrown when operator need no more elements from the flow.
* This exception should never escape outside of operator's implementation.
* This exception can be safely ignored by non-terminal flow operator if and only if it was caught by its owner
* (see usages of [checkOwnership]).
*/
internal expect class AbortFlowException() : CancellationException
internal expect class AbortFlowException(owner: FlowCollector<*>) : CancellationException {
public val owner: FlowCollector<*>
}

internal fun AbortFlowException.checkOwnership(owner: FlowCollector<*>) {
if (this.owner !== owner) throw this
}

/**
* Exception used to cancel child of [scopedFlow] without cancelling the whole scope.
Expand Down
8 changes: 4 additions & 4 deletions kotlinx-coroutines-core/common/src/flow/operators/Limit.kt
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ public fun <T> Flow<T>.take(count: Int): Flow<T> {
}
}
} catch (e: AbortFlowException) {
// Nothing, bail out
e.checkOwnership(owner = this)
}
}
}

private suspend fun <T> FlowCollector<T>.emitAbort(value: T) {
emit(value)
throw AbortFlowException()
throw AbortFlowException(this)
}

/**
Expand All @@ -80,9 +80,9 @@ public fun <T> Flow<T>.takeWhile(predicate: suspend (T) -> Boolean): Flow<T> = f
try {
collect { value ->
if (predicate(value)) emit(value)
else throw AbortFlowException()
else throw AbortFlowException(this)
}
} catch (e: AbortFlowException) {
// Nothing, bail out
e.checkOwnership(owner = this)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,5 +130,6 @@ public suspend fun <T> Flow<T>.collectLatest(action: suspend (value: T) -> Unit)
* Collects all the values from the given [flow] and emits them to the collector.
* It is a shorthand for `flow.collect { value -> emit(value) }`.
*/
@BuilderInference
@ExperimentalCoroutinesApi
public suspend inline fun <T> FlowCollector<T>.emitAll(flow: Flow<T>) = flow.collect(this)
4 changes: 2 additions & 2 deletions kotlinx-coroutines-core/common/src/flow/terminal/Reduce.kt
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public suspend fun <T> Flow<T>.first(): T {
try {
collect { value ->
result = value
throw AbortFlowException()
throw AbortFlowException(NopCollector)
}
} catch (e: AbortFlowException) {
// Do nothing
Expand All @@ -110,7 +110,7 @@ public suspend fun <T> Flow<T>.first(predicate: suspend (T) -> Boolean): T {
collect { value ->
if (predicate(value)) {
result = value
throw AbortFlowException()
throw AbortFlowException(NopCollector)
}
}
} catch (e: AbortFlowException) {
Expand Down
14 changes: 14 additions & 0 deletions kotlinx-coroutines-core/common/test/flow/operators/TakeTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,18 @@ class TakeTest : TestBase() {
}
finish(2)
}

@Test
fun testNestedTake() = runTest {
val inner = flow {
emit(1)
expectUnreached()
}.take(1)
val outer = flow {
while(true) {
emitAll(inner)
}
}
assertEquals(listOf(1, 1, 1), outer.take(3).toList())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class TransformLatestTest : TestBase() {
}

@Test
fun testSwitchRendevouzBuffer() = runTest {
fun testSwitchRendezvousBuffer() = runTest {
val flow = flowOf(1, 2, 3, 4, 5)
flow.transformLatest {
emit(it)
Expand Down Expand Up @@ -157,16 +157,4 @@ class TransformLatestTest : TestBase() {
val flow = flowOf(1, 2, 3, 4, 5).transformLatest { emit(it) }
assertEquals(listOf(1), flow.take(1).toList())
}

@Test
@Ignore // TODO separate branch and/or discuss
fun testTakeUpstreamCancellation() = runTest {
val flow = flow {
emit(1)
expectUnreached()
emit(2)
emit(3)
}.transformLatest { emit(it) }
assertEquals(listOf(1), flow.take(1).toList())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
package kotlinx.coroutines.flow.internal

import kotlinx.coroutines.*
import kotlinx.coroutines.flow.*

internal actual class AbortFlowException : CancellationException("Flow was aborted, no more elements needed")
internal actual class AbortFlowException actual constructor(
actual val owner: FlowCollector<*>
) : CancellationException("Flow was aborted, no more elements needed")
internal actual class ChildCancelledException : CancellationException("Child of the scoped flow was cancelled")
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
package kotlinx.coroutines.flow.internal

import kotlinx.coroutines.*
import kotlinx.coroutines.flow.*

internal actual class AbortFlowException actual constructor(
actual val owner: FlowCollector<*>
) : CancellationException("Flow was aborted, no more elements needed") {

internal actual class AbortFlowException : CancellationException("Flow was aborted, no more elements needed") {
override fun fillInStackTrace(): Throwable {
if (DEBUG) super.fillInStackTrace()
return this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
package kotlinx.coroutines.flow.internal

import kotlinx.coroutines.*
import kotlinx.coroutines.flow.*

internal actual class AbortFlowException : CancellationException("Flow was aborted, no more elements needed")
internal actual class AbortFlowException actual constructor(
actual val owner: FlowCollector<*>
) : CancellationException("Flow was aborted, no more elements needed")
internal actual class ChildCancelledException : CancellationException("Child of the scoped flow was cancelled")