Skip to main content

gunnar_ssh/
channel.rs

1//! An **owned** byte stream over a russh channel that does *not* close the
2//! channel when it is dropped.
3//!
4//! # The bug this module exists to make impossible
5//!
6//! russh's [`Channel::into_stream`](russh::Channel::into_stream) wraps the
7//! channel in `ChannelCloseOnDrop`, whose `Drop` does
8//!
9//! ```text
10//! tokio::spawn(async move { sender.send(ChannelMsg::Close).await })
11//! ```
12//!
13//! (`russh-0.61.2/src/channels/io/mod.rs`). The server hands that stream to the
14//! transport, so the `Close` is queued **from a task nobody awaits** the moment
15//! the transfer finishes, racing the `exit-status` the server sends next.
16//! OpenSSH frees a channel on `Close` and discards anything that arrives after
17//! it, so the race decided whether a client saw an exit status at all.
18//!
19//! Measured 2026-08-03, before this module existed: a raw
20//! `ssh … git-upload-pack 'team/project.git'` returned a complete 244-byte
21//! advertisement and exit status **255**, and `git push` printed
22//! `* [new branch] HEAD -> main` followed by `error: failed to push some refs`
23//! for a push that had fully succeeded server-side. `gunnar-cli` carried a
24//! 250 ms `linger` timer to invert the race; it is deleted.
25//!
26//! # The fix, by construction (one implementation)
27//!
28//! [`Channel::split`](russh::Channel::split) hands back a
29//! [`ChannelReadHalf`] and a [`ChannelWriteHalf`], **neither of which closes the
30//! channel on drop**. The write half stays with the server, which sends
31//! `exit-status` and then `close` through *the same sender the data went
32//! through*, so the ordering is a property of one FIFO queue rather than of a
33//! sleep. The read half is moved in here and turned into an owned
34//! [`AsyncRead`], because russh's own `make_reader` borrows the half and the
35//! transport needs a `'static` stream (it runs the blocking git protocol on
36//! `spawn_blocking`, which will not accept a borrow).
37//!
38//! There is no timing dependency left anywhere on this path.
39
40use std::future::Future;
41use std::io;
42use std::pin::Pin;
43use std::task::{Context, Poll};
44
45use russh::{ChannelMsg, ChannelReadHalf};
46use tokio::io::{AsyncRead, ReadBuf};
47
48/// The future that owns the read half while it waits for the next message.
49type Pending = Pin<Box<dyn Future<Output = (ChannelReadHalf, Option<ChannelMsg>)> + Send>>;
50
51/// Where the reader is between two `poll_read` calls.
52enum State {
53    /// Nothing is in flight; the half is parked here.
54    Idle(ChannelReadHalf),
55    /// A `wait()` is in flight, and it owns the half until it resolves.
56    Waiting(Pending),
57    /// The peer sent EOF (or the channel went away). Every further read is EOF.
58    Ended,
59}
60
61/// The data band of an SSH channel, as an owned [`AsyncRead`].
62///
63/// Dropping this drops a [`ChannelReadHalf`], which sends nothing at all. That
64/// is the whole point. See the module docs.
65pub struct ChannelReader {
66    state: State,
67    /// A `ChannelMsg::Data` that did not fit in the last `poll_read`, and how
68    /// much of it has already been handed out.
69    partial: Option<(ChannelMsg, usize)>,
70}
71
72impl ChannelReader {
73    /// Take ownership of a channel's read half.
74    pub fn new(half: ChannelReadHalf) -> Self {
75        ChannelReader {
76            state: State::Idle(half),
77            partial: None,
78        }
79    }
80}
81
82impl std::fmt::Debug for ChannelReader {
83    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
84        f.debug_struct("ChannelReader").finish_non_exhaustive()
85    }
86}
87
88/// The bytes of a `ChannelMsg::Data`, and nothing for anything else.
89///
90/// [`next_data`] only ever parks a `Data`, so the fallback arm is unreachable
91/// in practice; it is a `&[]` rather than a panic because `ChannelMsg` is
92/// `#[non_exhaustive]` and a future russh could add a variant here.
93fn payload(message: &ChannelMsg) -> &[u8] {
94    match message {
95        ChannelMsg::Data { data } => data,
96        _ => &[],
97    }
98}
99
100/// Wait for the next `Data` message, handing the half back with it.
101///
102/// Anything that is not `Data` is skipped **inside** the future rather than by
103/// returning `Poll::Pending` after a self-wake, so a chatty peer cannot spin
104/// the reactor. `Eof`, `Close` and a dead channel all mean end-of-stream.
105fn next_data(mut half: ChannelReadHalf) -> Pending {
106    Box::pin(async move {
107        loop {
108            match half.wait().await {
109                Some(ChannelMsg::Data { data }) => {
110                    return (half, Some(ChannelMsg::Data { data }));
111                }
112                Some(ChannelMsg::Eof) | Some(ChannelMsg::Close) | None => return (half, None),
113                Some(_) => continue,
114            }
115        }
116    })
117}
118
119impl AsyncRead for ChannelReader {
120    fn poll_read(
121        self: Pin<&mut Self>,
122        cx: &mut Context<'_>,
123        buf: &mut ReadBuf<'_>,
124    ) -> Poll<io::Result<()>> {
125        let me = self.get_mut();
126        loop {
127            if let Some((message, taken)) = me.partial.as_mut() {
128                let data = payload(message);
129                // Clamped by `min`, so neither index can be out of range.
130                let readable = buf.remaining().min(data.len().saturating_sub(*taken));
131                buf.put_slice(&data[*taken..*taken + readable]);
132                *taken += readable;
133                if *taken >= data.len() {
134                    me.partial = None;
135                }
136                return Poll::Ready(Ok(()));
137            }
138
139            match std::mem::replace(&mut me.state, State::Ended) {
140                // Reading past EOF is not an error, it is EOF again: a zero-fill
141                // return is exactly how `AsyncRead` says so.
142                State::Ended => return Poll::Ready(Ok(())),
143                State::Idle(half) => me.state = State::Waiting(next_data(half)),
144                State::Waiting(mut waiting) => match waiting.as_mut().poll(cx) {
145                    Poll::Pending => {
146                        me.state = State::Waiting(waiting);
147                        return Poll::Pending;
148                    }
149                    Poll::Ready((half, Some(message))) => {
150                        me.state = State::Idle(half);
151                        me.partial = Some((message, 0));
152                    }
153                    Poll::Ready((_, None)) => {
154                        me.state = State::Ended;
155                        return Poll::Ready(Ok(()));
156                    }
157                },
158            }
159        }
160    }
161}
162
163// ── the writer that hears the session end ───────────────────────────────────
164
165/// **The session is over**, as a value every transfer on it can watch.
166///
167/// The [`crate::server`] connection handler owns the sender and sets it from
168/// `Drop` — and russh drops the handler on EVERY way a session ends: the client
169/// hung up, its keepalives went unanswered, the inactivity bound, a protocol
170/// error, the server's own shutdown.
171pub type SessionOver = tokio::sync::watch::Receiver<bool>;
172
173/// Resolve once `over` says the session has ended (or its sender is gone,
174/// which is the same thing said by `Drop`).
175pub async fn session_over(mut over: SessionOver) {
176    let _ = over.wait_for(|ended| *ended).await;
177}
178
179/// The error a transfer gets when the client's session ended under it.
180pub fn session_ended_error() -> io::Error {
181    io::Error::new(
182        io::ErrorKind::ConnectionAborted,
183        "the client's ssh session ended mid-transfer (it hung up, or its link went silent \
184         and its keepalives went unanswered)",
185    )
186}
187
188/// How long a transfer whose session has ended is given to end by itself.
189///
190/// The session's end also ends the channel, so a transfer that was only
191/// waiting for the client's next request reads EOF at once and ends cleanly;
192/// one still working (main building a pack) is cut when this runs out.
193pub const SESSION_END_GRACE: std::time::Duration = std::time::Duration::from_secs(2);
194
195/// The grace when the transfer's fan has others queued behind it: a worker a
196/// hung-up client holds is a worker a waiting client does not get.
197pub const CROWDED_SESSION_END_GRACE: std::time::Duration = std::time::Duration::from_millis(200);
198
199/// **The grace a transfer whose session ended is given**: `calm`, or
200/// `crowded` when `queued` operations wait for its fan.
201pub fn grace_for(calm: std::time::Duration, crowded: std::time::Duration, queued: usize) -> std::time::Duration {
202    if queued > 0 { crowded.min(calm) } else { calm }
203}
204
205/// **The word a transfer's git work hears when its client is gone.**
206///
207/// Set by [`serve_until_session_ends`] when the grace runs out, and by a
208/// transport whose serve future is dropped. The git work checks it at its safe
209/// points — every read from and write to the client, and before it opens the
210/// store — and ends there. It is never checked between the ref updates of a
211/// receive-pack transaction: those land together or not at all.
212///
213/// Cheap to clone; every clone is the same flag.
214#[derive(Clone, Default)]
215pub struct Cancel(std::sync::Arc<CancelInner>);
216
217#[derive(Default)]
218struct CancelInner {
219    set: std::sync::atomic::AtomicBool,
220    wake: tokio::sync::Notify,
221}
222
223impl Cancel {
224    /// A flag nobody has set.
225    pub fn new() -> Self {
226        Self::default()
227    }
228
229    /// Tell the transfer to stop at its next safe point.
230    pub fn cancel(&self) {
231        self.0.set.store(true, std::sync::atomic::Ordering::SeqCst);
232        self.0.wake.notify_waiters();
233    }
234
235    /// Has the transfer been told to stop?
236    pub fn is_cancelled(&self) -> bool {
237        self.0.set.load(std::sync::atomic::Ordering::SeqCst)
238    }
239
240    /// Resolves once the transfer has been told to stop.
241    pub async fn cancelled(&self) {
242        loop {
243            let notified = self.0.wake.notified();
244            if self.is_cancelled() {
245                return;
246            }
247            notified.await;
248        }
249    }
250}
251
252impl std::fmt::Debug for Cancel {
253    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
254        f.debug_tuple("Cancel").field(&self.is_cancelled()).finish()
255    }
256}
257
258/// **A transfer, bound to its session**: what `serve` returns, unless the
259/// session ends first and the transfer does not end by itself within
260/// `grace()` (asked when the session ends; [`grace_for`] — the server gives
261/// [`SESSION_END_GRACE`], or [`CROWDED_SESSION_END_GRACE`] when others wait
262/// for the fan), in which case it ends with `ended()`.
263///
264/// The server's transfer task runs every `git-upload-pack` /
265/// `git-receive-pack` through this, so a client that went silent while main
266/// still builds its pack gives back its fan worker, admission slot and store
267/// handle when its session ends (the UpCloud mockup's LD1/LD2, 2026-10-09).
268///
269/// The grace is what keeps a SERVED client honest: `ls-remote` reads the ref
270/// list and hangs up, and its session's end used to win the race against
271/// upload-pack's own clean EOF, so every such ls-remote was logged as a failed
272/// transfer (test2 install receipt 2026-10-09: WARN spam, REFUSED
273/// [log-verdict]). The protocol decides whether the client was served, not
274/// which of the two wakes first.
275///
276/// # The cut waits for the work (stinky row 1, 2026-10-10)
277///
278/// When the grace runs out, `cancel` is set and `serve` is **awaited to its
279/// end**, not dropped. Dropping it dropped only the `JoinHandle` of the
280/// transport's blocking task: the git work went on (a pack being built, refs
281/// being moved) while the fan worker running this was given back, so the fan
282/// bounded nothing. The work ends at its next safe point once it reads the
283/// flag ([`Cancel`]); the worker is held until it has.
284pub async fn serve_until_session_ends<T, E>(
285    serve: impl Future<Output = Result<T, E>>,
286    over: SessionOver,
287    cancel: &Cancel,
288    grace: impl FnOnce() -> std::time::Duration,
289    ended: impl FnOnce() -> E,
290) -> Result<T, E> {
291    let mut serve = std::pin::pin!(serve);
292    tokio::select! {
293        served = &mut serve => served,
294        () = session_over(over) => match tokio::time::timeout(grace(), &mut serve).await {
295            Ok(served) => served,
296            Err(_) => {
297                cancel.cancel();
298                tracing::debug!(target: "gunnar::ssh", event = "ssh.transfer.told_to_stop", "the session ended and the transfer did not end within the grace; it is told to stop and its worker waits for it");
299                match serve.await {
300                    Ok(served) => Ok(served),
301                    Err(_) => Err(ended()),
302                }
303            }
304        },
305    }
306}
307
308/// A channel writer that **ends with an error when the session does**.
309///
310/// # The bug this exists to make impossible (cov-13 finding 4, 2026-10-06)
311///
312/// russh's channel writer (`ChannelTx`, what `make_writer` returns) waits for
313/// SSH window on a `Notify` that only a window-adjust from the client fires. A
314/// session that ENDS fires nothing. So a clone or fetch streaming a pack to a
315/// client whose link died — the window shut, the session then ended by the
316/// keepalive — left the transfer parked forever: its fan worker, its admission
317/// slot, its memory-ledger charge, its store handle, all held for good. One
318/// dead client per leak, until the fan refuses everyone.
319///
320/// Every poll here also polls the session's end, so the parked write wakes and
321/// fails with [`session_ended_error`] the moment the session is gone.
322pub struct SessionBound<W> {
323    inner: W,
324    over: SessionOver,
325    /// The pending "session over" wait, kept across polls so its waker stays
326    /// registered while the inner write is parked.
327    ending: Option<Pin<Box<dyn Future<Output = ()> + Send>>>,
328}
329
330impl<W> SessionBound<W> {
331    /// Bind `inner` to the session `over` watches.
332    pub fn new(inner: W, over: SessionOver) -> Self {
333        SessionBound { inner, over, ending: None }
334    }
335
336    /// `Ready` once the session has ended; registers the waker otherwise.
337    fn poll_over(&mut self, cx: &mut Context<'_>) -> Poll<()> {
338        if *self.over.borrow() {
339            return Poll::Ready(());
340        }
341        let over = self.over.clone();
342        let ending = self.ending.get_or_insert_with(|| Box::pin(session_over(over)));
343        ending.as_mut().poll(cx)
344    }
345}
346
347impl<W> std::fmt::Debug for SessionBound<W> {
348    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
349        f.debug_struct("SessionBound").finish_non_exhaustive()
350    }
351}
352
353impl<W: tokio::io::AsyncWrite + Unpin> tokio::io::AsyncWrite for SessionBound<W> {
354    fn poll_write(self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &[u8]) -> Poll<io::Result<usize>> {
355        let me = self.get_mut();
356        match Pin::new(&mut me.inner).poll_write(cx, buf) {
357            Poll::Ready(r) => Poll::Ready(r),
358            Poll::Pending => match me.poll_over(cx) {
359                Poll::Ready(()) => Poll::Ready(Err(session_ended_error())),
360                Poll::Pending => Poll::Pending,
361            },
362        }
363    }
364
365    fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<io::Result<()>> {
366        let me = self.get_mut();
367        match Pin::new(&mut me.inner).poll_flush(cx) {
368            Poll::Ready(r) => Poll::Ready(r),
369            Poll::Pending => match me.poll_over(cx) {
370                Poll::Ready(()) => Poll::Ready(Err(session_ended_error())),
371                Poll::Pending => Poll::Pending,
372            },
373        }
374    }
375
376    fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<io::Result<()>> {
377        let me = self.get_mut();
378        match Pin::new(&mut me.inner).poll_shutdown(cx) {
379            Poll::Ready(r) => Poll::Ready(r),
380            Poll::Pending => match me.poll_over(cx) {
381                Poll::Ready(()) => Poll::Ready(Err(session_ended_error())),
382                Poll::Pending => Poll::Pending,
383            },
384        }
385    }
386}
387
388#[cfg(test)]
389mod tests {
390    use super::*;
391    use std::time::Duration;
392
393    fn ended_session() -> (tokio::sync::watch::Sender<bool>, SessionOver) {
394        let (tx, rx) = tokio::sync::watch::channel(false);
395        tx.send_replace(true);
396        (tx, rx)
397    }
398
399    /// ★ **A client that hung up after it was served is no failed transfer**
400    /// (test2 install receipt 2026-10-09 18:35 UTC, REFUSED [log-verdict]):
401    /// `ls-remote` reads the ref list and hangs up. Its session ends while
402    /// upload-pack waits for a next command that never comes, and upload-pack
403    /// ends cleanly on that EOF a moment later. Every such ls-remote was logged
404    /// as `git.transfer.failed` + `git.exit_status.undelivered` at WARN, and the
405    /// kit's functest (7 repositories listed in 38 s) tripped the WARN-spam rule.
406    #[tokio::test]
407    async fn a_client_that_hung_up_after_being_served_is_no_failed_transfer() {
408        let (_tx, over) = ended_session();
409        let serve = async {
410            // upload-pack reading the EOF the ended session leaves behind
411            tokio::time::sleep(Duration::from_millis(50)).await;
412            Ok::<(), io::Error>(())
413        };
414        let served = serve_until_session_ends(serve, over, &Cancel::new(), || SESSION_END_GRACE, session_ended_error).await;
415        assert!(served.is_ok(), "a transfer that ended cleanly was called failed: {served:?}");
416    }
417
418    /// Stinky row 19 (2026-10-10): the 2 s grace was a constant, so every
419    /// client that hung up held a fan worker up to 2 s more — however many
420    /// waited behind it, which is what a connect-and-hang-up flood feeds on.
421    /// The grace is the caller's, and shorter when the fan is crowded.
422    #[test]
423    fn the_grace_is_shorter_when_others_wait_for_the_fan() {
424        let (calm, crowded) = (Duration::from_secs(2), Duration::from_millis(200));
425        assert_eq!(grace_for(calm, crowded, 0), calm, "nobody waits: the whole grace");
426        assert_eq!(grace_for(calm, crowded, 1), crowded, "one waits: the crowded grace");
427        assert_eq!(grace_for(Duration::from_millis(100), crowded, 5), Duration::from_millis(100), "never longer than the calm grace");
428    }
429
430    #[tokio::test]
431    async fn a_transfer_is_cut_after_the_grace_it_is_given() {
432        let (_tx, over) = ended_session();
433        let cancel = Cancel::new();
434        let heard = cancel.clone();
435        let serve = async move {
436            heard.cancelled().await;
437            Err::<(), io::Error>(io::Error::new(io::ErrorKind::ConnectionAborted, "told to stop"))
438        };
439        let started = std::time::Instant::now();
440        let served = serve_until_session_ends(serve, over, &cancel, || Duration::from_millis(50), session_ended_error).await;
441        assert!(served.is_err());
442        assert!(started.elapsed() < Duration::from_secs(1), "cut after {:?}, not the 50 ms it was given", started.elapsed());
443    }
444
445    /// The bug the binding exists for stays fixed: a transfer that is still
446    /// working (main building the pack) when the session ends is told to stop,
447    /// and gives back what it holds, within a bound, once it has.
448    #[tokio::test]
449    async fn a_transfer_still_working_when_the_session_ends_is_cut() {
450        let (_tx, over) = ended_session();
451        let cancel = Cancel::new();
452        let heard = cancel.clone();
453        let serve = async move {
454            heard.cancelled().await;
455            Err::<(), io::Error>(io::Error::new(io::ErrorKind::ConnectionAborted, "told to stop"))
456        };
457        let served = tokio::time::timeout(Duration::from_secs(10), serve_until_session_ends(serve, over, &cancel, || SESSION_END_GRACE, session_ended_error))
458            .await
459            .expect("a transfer outlived its session by 10 s");
460        let err = served.expect_err("a transfer still working when its session ended was called served");
461        assert_eq!(err.kind(), io::ErrorKind::ConnectionAborted, "{err}");
462    }
463
464    /// ★ **A hung-up client cannot make blocking work exceed the fan** (stinky
465    /// row 1, 2026-10-10). The transport's git work runs on tokio's blocking
466    /// pool, and when the grace ran out only its `JoinHandle` was dropped: the
467    /// fan worker was released while the work (a pack being built, refs being
468    /// moved) went on, so the fan bounded nothing and a stream of clients that
469    /// hung up mid-clone could put up to 512 blocking threads to work. Here a
470    /// fan of ONE worker serves two transfers whose clients hung up while main
471    /// still works; at no moment may two pieces of that work run together.
472    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
473    async fn a_hung_up_client_cannot_make_blocking_work_exceed_the_fan() {
474        use crate::governor::OpClass;
475        use crate::pipeline::{Job, Pipeline};
476        use std::sync::atomic::{AtomicUsize, Ordering};
477        use std::sync::Arc;
478
479        // Longer than the grace, so the work is still running when it runs out.
480        const WORK: Duration = Duration::from_secs(3);
481        let fan: Pipeline<Job> = Pipeline::start(OpClass::Clone, 1, 4, |job: Job| job());
482        let live = Arc::new(AtomicUsize::new(0));
483        let peak = Arc::new(AtomicUsize::new(0));
484        let (done_tx, mut done_rx) = tokio::sync::mpsc::unbounded_channel::<()>();
485        for _ in 0..2 {
486            let (live, peak, done) = (Arc::clone(&live), Arc::clone(&peak), done_tx.clone());
487            let handle = tokio::runtime::Handle::current();
488            let (session, over) = ended_session();
489            fan.submit(Box::new(move || {
490                handle.block_on(async move {
491                    let _session = session;
492                    let serve = async move {
493                        tokio::task::spawn_blocking(move || {
494                            let now = live.fetch_add(1, Ordering::SeqCst) + 1;
495                            peak.fetch_max(now, Ordering::SeqCst);
496                            let until = std::time::Instant::now() + WORK;
497                            while std::time::Instant::now() < until {
498                                std::thread::sleep(Duration::from_millis(10));
499                            }
500                            live.fetch_sub(1, Ordering::SeqCst);
501                            Ok::<(), io::Error>(())
502                        })
503                        .await
504                        .map_err(io::Error::other)?
505                    };
506                    let _ = serve_until_session_ends(serve, over, &Cancel::new(), || SESSION_END_GRACE, session_ended_error).await;
507                });
508                let _ = done.send(());
509            }))
510            .expect("the fan takes the transfer");
511        }
512        for _ in 0..2 {
513            done_rx.recv().await.expect("a transfer finished");
514        }
515        // Whatever the fan let go of early is still working; let it end so the
516        // peak is the whole story.
517        while live.load(Ordering::SeqCst) > 0 {
518            tokio::time::sleep(Duration::from_millis(10)).await;
519        }
520        let peak = peak.load(Ordering::SeqCst);
521        assert!(
522            peak <= 1,
523            "a fan of one worker had {peak} transfers' blocking work running at once: the worker \
524             was released while the work of a client that hung up went on"
525        );
526    }
527
528    /// A transfer that ends while its session is alive is what it says.
529    #[tokio::test]
530    async fn a_transfer_with_its_session_alive_is_its_own_verdict() {
531        let (_tx, over) = tokio::sync::watch::channel(false);
532        let ok = serve_until_session_ends(async { Ok::<u8, io::Error>(7) }, over.clone(), &Cancel::new(), || SESSION_END_GRACE, session_ended_error).await;
533        assert_eq!(ok.unwrap(), 7);
534        let err = serve_until_session_ends(async { Err::<u8, io::Error>(io::Error::other("store")) }, over, &Cancel::new(), || SESSION_END_GRACE, session_ended_error).await;
535        assert_eq!(err.unwrap_err().to_string(), "store");
536    }
537}