Provide the remaining buffer when the NIOAsyncSequenceProducer termin… #3111
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ates # Motivation Our `NIOAsyncChannel` abstractions is running into crashes in the bootstraps and multiplexer when a `NIOAsyncChannel` is buffered inside an async stream and the consuming task gets cancelled. # Modification This PR modifies the `NIOAsyncSequenceProducer` to hand over the remaining elements once it is finished to `didTerminate` to allow the user to run any manual clean up such as closing the underling channel of a `NIOAsyncChannel`. # Result A path forward to avoid the crashes we are seeing.
Lukasa
reviewed
Feb 7, 2025
| /// - The consuming `Task` is cancelled (e.g. `for await let element in`). | ||
| /// - The source finished and all remaining buffered elements have been consumed. | ||
| /// | ||
| /// - Note: This is guaranteed to be called _exactly_ once. |
There was a problem hiding this comment.
Let's update the note on the other didTerminate to indicate that it is no longer called, but if no implementation of didTerminate(remainingBuffer:) is provided this will be called from there in the default implementation.
| /// - The source finished and all remaining buffered elements have been consumed. | ||
| /// | ||
| /// - Note: This is guaranteed to be called _exactly_ once. | ||
| func didTerminate(remainingBuffer: some Collection<Any>) |
There was a problem hiding this comment.
Why do we need to box the elements? Why isn't this just some Collection?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ates
Motivation
Our
NIOAsyncChannelabstractions is running into crashes in the bootstraps and multiplexer when aNIOAsyncChannelis buffered inside an async stream and the consuming task gets cancelled.Modification
This PR modifies the
NIOAsyncSequenceProducerto hand over the remaining elements once it is finished todidTerminateto allow the user to run any manual clean up such as closing the underling channel of aNIOAsyncChannel.Result
A path forward to avoid the crashes we are seeing.
[One line description of your change]
Motivation:
[Explain here the context, and why you're making that change. What is the problem you're trying to solve.]
Modifications:
[Describe the modifications you've done.]
Result:
[After your change, what will change.]