Add AsyncThrowingStream.init(unfolding:onCancel:) to handle cancellation. #78723
Conversation
…ion. It shares implementation with and becomes the general case of the existing `init(unfolding:)`, and given the current implementation's use of `withTaskCancellationHandler`, the `onCancel` closure can run concurrently with the `unfolding` closure if cancellation happens during production, or ahead of it when cancellation has already taken place. See doc and the last 3 test cases added. Also adds tests for `init(unfolding:)`. Resolves swiftlang#77974.
adlich
force-pushed
the
onCancel-77974
branch
from
9f3f235 to
45de2d6
Compare
| @@ -72,6 +72,17 @@ class NotSendable {} | |||
| } | |||
| } | |||
|
|
|||
| tests.test("unfold with no awaiting next throwing") { | |||
| _ = AsyncThrowingStream(unfolding: { return "hello" }) | |||
There was a problem hiding this comment.
Apologies if I didn't quite grasp the underlying philosophy of test naming and test case matrix in this file. Please let me know if you'd suggest fewer, more, different and/or differently named tests.
|
|
||
| tests.test("unfold awaiting next and cancel throwing") { | ||
| let expectation = Expectation() | ||
| let task = Task.detached { |
There was a problem hiding this comment.
Please let me know if you have better suggestions for testing cancellation.
| /// | ||
| /// `onCancel` may be executed concurrently with `produce` and will be | ||
| /// executed ahead of it if the task had already been cancelled an execution | ||
| /// of `produce`. |
There was a problem hiding this comment.
FYI: Behavior mentioned in description (concurrency of cancellation call itself.)
| /// print(error) | ||
| /// } | ||
| /// | ||
| @available(SwiftStdlib 6.1, *) |
There was a problem hiding this comment.
I read [1] and noticed [2] (6.1 and 6.2 defined) but am still unsure which is appropriate.
[1] https://github.com/swiftlang/swift/blob/main/docs/LibraryEvolution.rst
[2] https://github.com/swiftlang/swift/blob/main/utils/availability-macros.def
| ) where Failure == Error { | ||
| let storage: _AsyncStreamCriticalStorage< | ||
| Optional<() async throws -> Element?> | ||
| > = .create(produce) |
There was a problem hiding this comment.
My best interpretation of line-breaking guidelines in [1]. Some other possibly "questionable" ones in tests as well. Happy to change as preferred.
[1] https://github.com/swiftlang/swift/blob/main/docs/StandardLibraryProgrammersManual.md#line-breaking
|
Please review and trigger tests. Thank you! |
| @@ -353,6 +353,59 @@ class NotSendable {} | |||
| expectTrue(expectation.fulfilled) | |||
| } | |||
|
|
|||
| tests.test("unfold awaiting next with ignored cancellation throwing") { | |||
There was a problem hiding this comment.
Perhaps intentionally skipped before, these may be more important now, as regressions could hide in the logic that starts becoming more complex than simply resetting storage state. Once again, I'm very open to feedback w.r.t. testing.
|
cc: @phausler |
It shares implementation with and becomes the general case of the existing
init(unfolding:), and given the current implementation's use ofwithTaskCancellationHandler, theonCancelclosure can run concurrently with theunfoldingclosure if cancellation happens during production, or ahead of it when cancellation has already taken place. See doc and the last 3 test cases added in the 1st commit.Also adds tests for
init(unfolding:).Resolves #77974.