SSH: deliver the channel close instead of attempting it - #2521
Open
janisdombr wants to merge 1 commit into
Open
Conversation
A session that has streamed a large amount of output can end without the client ever being told its channel is closed, and `ssh` then waits on a session that has already finished until something kills it. `events_tx` holds 1024 events and every chunk of target output takes one, so the queue is full at exactly the moment a high-output channel ends. `close()` used `try_send`, whose failure was discarded, and the close was lost. The call at the end of `run()` is in an async context and can wait for room. `Drop` keeps `try_send`, having nothing to await with.
4 tasks
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
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.
Description
This is what fails
Testsonmainabout a quarter of the time.test_recordings_index.py::Test::test_terminal_recording_indexis the visiblesymptom; the cause is below, and it is not in the test. Every pull request pays
for it, including ones that have nothing to do with SSH.
A session that has streamed a large amount of output can end without the client ever being told its channel is closed.
sshthen waits on a session that has already finished, until something kills it.events_txholds 1024 events and every chunk of target output takes one, so the queue is full at exactly the moment a high-output channel ends.close()usedtry_send, whose failure was discarded, and the close was lost. The call at the end ofrun()is in an async context and can wait for room;Dropkeepstry_send, having nothing to await with.How it shows up. That test streams ~1.5 MB through a recorded PTY and times out waiting for the client. In every failure the whole transfer had arrived — 1,489,491 bytes, last line
200000— so the data is not the problem; the client is simply never released.The gateway log names the difference. A successful run ends:
A failing one ends
ExitStatusthenEof, and then nothing at all until the client is killed and teardown follows.Measured. Each run below repeats the transfer twenty times and reports, for any that hangs, how many bytes had arrived.
c5f4106ffacada2afacada2aAt the observed rate, forty consecutive clean transfers is about one chance in eight thousand.
The reproduction and the loop that measures it are on
tooling/recording-stallin my fork, based onmainwith nothing else merged, and are deliberately not part of this change. GitHub truncates the larger logs, so the loop's exit status is what carries the result — it fails on the first hang.AI Usage
Choose the level of AI involvement for this PR.
This is not to block AI contributions but rather to speed up PR review (saves time on trying to deduce the logic behind AI hallucinations).