Stop losing outbound channel data when the peer's window fills (claude - testing) - #9
Open
redvers wants to merge 2 commits into
Open
Stop losing outbound channel data when the peer's window fills (claude - testing)#9redvers wants to merge 2 commits into
redvers wants to merge 2 commits into
Conversation
channel_send sent as much of a buffer as the peer's flow-control window allowed, dropped the rest, and reported SshWindowExhausted through ssh_channel_error. That error carried no byte count, so a consumer could not tell how much had gone out or where to resume, and ssh_channel_error does nothing unless overridden. A server writing more output to a channel than the peer's window allowed lost the tail of it without a trace. Data past the window is now held in a per-channel queue and sent as the peer grants window, drained both when data is queued and on an inbound CHANNEL_WINDOW_ADJUST. A buffer is accepted whole or not at all: over the 256 KiB per-channel cap, none of it is sent and SshWindowExhausted is reported, so a caller is never left working out which part went out. SshClientNotify and SshServerNotify gain ssh_channel_writeable, called when a channel whose queue filled has drained empty, so a consumer whose write was refused knows when to resume. Both supply a default that does nothing, so existing implementations still compile. Closing a channel with data still queued reports SshChannelClosed for the unsent part instead of discarding it silently. The drain ignores unknown channel ids: one caller passes an id from a peer's CHANNEL_WINDOW_ADJUST, and reporting it would let a peer raise channel errors for ids that never existed.
The per-channel send queue was a List[Array[U8] val] with a separate byte counter and hand-written segment splitting. buffered.Reader is that same structure — a list of (chunk, offset) pairs with a running total — already written and tested, so append/size/block replace the bookkeeping. Segments are now drawn from the queue rather than from a single write, so a packet may carry the end of one channel_send and the start of the next. A channel is a byte stream, so framing is a consumer's problem; the release note says so and a test pins the coalescing. block() copies the segment where the previous code returned a trim view of the queued buffer: one memcpy per outbound segment, against a framing copy that already happens downstream.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
channel_send sent as much of a buffer as the peer's flow-control window allowed, dropped the rest, and reported SshWindowExhausted through ssh_channel_error. That error carried no byte count, so a consumer could not tell how much had gone out or where to resume, and ssh_channel_error does nothing unless overridden. A server writing more output to a channel than the peer's window allowed lost the tail of it without a trace.
Data past the window is now held in a per-channel queue and sent as the peer grants window, drained both when data is queued and on an inbound CHANNEL_WINDOW_ADJUST. A buffer is accepted whole or not at all: over the 256 KiB per-channel cap, none of it is sent and SshWindowExhausted is reported, so a caller is never left working out which part went out.
SshClientNotify and SshServerNotify gain ssh_channel_writeable, called when a channel whose queue filled has drained empty, so a consumer whose write was refused knows when to resume. Both supply a default that does nothing, so existing implementations still compile.
Closing a channel with data still queued reports SshChannelClosed for the unsent part instead of discarding it silently. The drain ignores unknown channel ids: one caller passes an id from a peer's CHANNEL_WINDOW_ADJUST, and reporting it would let a peer raise channel errors for ids that never existed.