Skip to content

fix(server): drain Handle::data backlog before select! - #729

Closed
Yaminyam wants to merge 1 commit into
Eugeny:mainfrom
Yaminyam:fix/handle-data-from-spawned-tasks
Closed

fix(server): drain Handle::data backlog before select!#729
Yaminyam wants to merge 1 commit into
Eugeny:mainfrom
Yaminyam:fix/handle-data-from-spawned-tasks

Conversation

@Yaminyam

Copy link
Copy Markdown
Contributor

Summary

tokio::select! in the server session loop doesn't always wake up promptly for messages produced by spawned tasks via Handle::data() when the internal mpsc channel is being filled faster than the select cycle drains it. In interactive PTY servers where a separate task pumps shell output through Handle::data(), this shows up as the client missing chunks of output until some other event happens to wake select.

This PR drains the receiver with try_recv() before entering select!, with a 64-message cap so select still gets to run between batches and client input (Ctrl+C, window resize, …) stays responsive.

Reproduction

Any russh-based SSH server with a PTY session whose shell output is generated from a spawned task pushing via Handle::data() faster than the session loop's select cycle reads from its mpsc channel — e.g. yes | head -n 100000 over an interactive PTY. The client sees periodic stalls / dropped output until further activity.

Change

In run_inner (server session loop), before the existing tokio::select!, batch-drain up to MAX_MESSAGES_PER_BATCH = 64 messages from self.receiver with try_recv() and dispatch through the same handlers select! would have used: data / extended_data / eof / close / channel_success / channel_failure / xon_xoff_request / exit_status_request / exit_signal_request / WindowAdjusted / ChannelOpen{Agent,Session,DirectTcpip,...}. Gated on !self.kex.active() so the rekey state machinery isn't perturbed.

The cap exists so high-throughput output can't starve client input — once a batch is full we fall through to select!, which picks up any client-side event (input, channel close, keepalive timeout) before the next batch.

Origin

This is the same fix bssh has been carrying as crates/bssh-russh/patches/handle-data-fix.patch. Its README pins at inureyes/russh:fix/handle-data-from-spawned-tasks and notes "When merged upstream, this fork will be deprecated" — this PR is the upstream half of that contract. Original author of the fix: @inureyes (bssh maintainer).

Test plan

  • cargo build -p russh → clean
  • Manual PTY soak test recommended before merge: spawn-task-driven Handle::data() server, confirm no output stalls under yes | head -n 100000-style load.

`tokio::select!` in the server session loop does not always wake up
promptly for messages produced by spawned tasks via `Handle::data()`
when the channel is being filled faster than the select cycle reads
from it. In interactive PTY sessions where a separate task is pumping
shell output, this manifests as the client missing chunks of output
until the next event happens to wake select.

Before entering `select!`, drain at most 64 pending messages from the
session receiver with `try_recv()`, dispatching them to the appropriate
`Session::{data,extended_data,eof,close,channel_success,channel_failure,...}`
handlers. The 64-message cap ensures select! still gets a chance to run
between batches so client input (e.g. Ctrl+C) stays responsive.

This is the same fix bssh has been carrying as
`crates/bssh-russh/patches/handle-data-fix.patch`. Its README pins at
`inureyes/russh:fix/handle-data-from-spawned-tasks` and notes the fork
will be dropped once it merges upstream — this PR is the upstream half
of that contract. Original author of the fix: @inureyes.

Co-authored-by: Jeongkyu Shin <inureyes@gmail.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Eugeny

Eugeny commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Thanks for bringing it to my attention - I've added my own cleaner fix in #731

@all-contributors please add @inureyes for code

@allcontributors

Copy link
Copy Markdown
Contributor

@Eugeny

I've put up a pull request to add @inureyes! 🎉

Eugeny added a commit that referenced this pull request Jul 2, 2026
Eugeny pushed a commit that referenced this pull request Jul 3, 2026
Adds @inureyes as a contributor for code.

This was requested by Eugeny [in this
comment](#729 (comment))

[skip ci]

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants