max / makenotwork

Fix git push hang by dropping unread SSH channels russh delivers every inbound CHANNEL_DATA twice: first with a blocking send onto the Channel's bounded queue, then to Handler::data. The handler parked every session Channel in a map and only SFTP ever took one back out, so a git exec left a receiver nobody read. Past 100 packets the queue filled, the send blocked the session loop for good, and Handler::data -- which feeds git-receive-pack's stdin -- stopped being called. The client sat in "Writing objects" against a server that would never read again. Drop the parked channel in exec_request and shell_request, both of which drive the session through the Handle instead. Add channel_close to clear a channel opened with no request, plus the git stdin and pipe-upload buffer when a client closes without EOF. Only bites above ~100 data packets, which is why a small repo pushed and shop (4.5 MiB) stalled around 12%.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: Max Johnson <me@maxj.phd> · 2026-08-04 00:13 UTC

Signed with PGP, not checked

Commit: 234c105be4c8f712888c11a7d33257a1182ba808

Parent: 70b6e69

1 file changed, +47 insertions, -1 deletion

M mnw-cli/src/ssh/handler.rs +47 -1

			@@ -34,7 +34,9 @@
34	34		pty_requested: bool,
35	35		/// TUI application handle for forwarding keypresses.
36	36		app: Option<tui::AppHandle>,
37		-	/// Channels stored between open and shell/subsystem request.
	37	+	/// Channels stored between open and shell/subsystem request. Only SFTP
	38	+	/// consumes one; see [`MnwHandler::release_channel`] for why every other
	39	+	/// path has to drop its entry rather than leave it parked here.
38	40		channels: Arc<Mutex<HashMap<ChannelId, Channel<Msg>>>>,
39	41		/// Active git subprocess stdin handles, keyed by channel.
40	42		git_processes: HashMap<ChannelId, tokio::process::ChildStdin>,
			@@ -75,6 +77,25 @@
75	77		pipe_uploads: HashMap::new(),
76	78		}
77	79		}
	80	+
	81	+	/// Drop the parked [`Channel`] for a session we are going to drive through
	82	+	/// the [`Handle`](russh::server::Handle) and the handler callbacks instead.
	83	+	///
	84	+	/// Not bookkeeping: it is what keeps a push from hanging. russh delivers
	85	+	/// every inbound `CHANNEL_DATA` twice, first by `send().await` onto the
	86	+	/// `Channel`'s bounded queue (`channel_buffer_size`, 100 messages) and only
	87	+	/// then to `Handler::data`. Holding a `Channel` nobody reads means that
	88	+	/// queue fills, the `send().await` blocks the session loop forever, and the
	89	+	/// handler stops being called at all. The client sits in `Writing objects`
	90	+	/// with a window the server already granted and never spends.
	91	+	///
	92	+	/// It only bites above ~100 packets, which is why a small repo pushed fine
	93	+	/// and `shop` (4.5 MiB) stalled around 12%. Dropping the receiver makes
	94	+	/// russh's `send(...).unwrap_or(())` a no-op, and the channel keeps working
	95	+	/// through the handle.
	96	+	async fn release_channel(&self, channel: ChannelId) {
	97	+	self.channels.lock().await.remove(&channel);
	98	+	}
78	99		}
79	100
80	101		impl russh::server::Handler for MnwHandler {
			@@ -191,6 +212,10 @@
191	212		channel: ChannelId,
192	213		session: &mut Session,
193	214		) -> Result<(), Self::Error> {
	215	+	// The TUI writes through the session handle and reads keypresses from
	216	+	// `Handler::data`, so the parked channel would only queue up.
	217	+	self.release_channel(channel).await;
	218	+
194	219		let Some(ref user) = self.user else {
195	220		tracing::warn!("shell_request without authenticated user");
196	221		session.close(channel)?;
			@@ -296,6 +321,12 @@
296	321		let command_line = String::from_utf8_lossy(data);
297	322		let handle = session.handle();
298	323
	324	+	// Every exec path below (git, pipe upload, scp notice, plain command)
	325	+	// streams through `handle` and takes its stdin from `Handler::data`.
	326	+	// A push is the one that moves enough bytes to deadlock on the parked
	327	+	// channel's queue, but none of them read it.
	328	+	self.release_channel(channel).await;
	329	+
299	330		let Some(ref user) = self.user else {
300	331		let _ = handle.close(channel).await;
301	332		return Ok(());
			@@ -547,6 +578,21 @@
547	578		Ok(())
548	579		}
549	580
	581	+	async fn channel_close(
	582	+	&mut self,
	583	+	channel: ChannelId,
	584	+	_session: &mut Session,
	585	+	) -> Result<(), Self::Error> {
	586	+	// A channel that was opened and then closed without a shell, exec or
	587	+	// subsystem request still has an entry parked in `channels`; a client
	588	+	// that closes without sending EOF still has a git child holding a pipe.
	589	+	// Both live for the length of the connection otherwise.
	590	+	self.release_channel(channel).await;
	591	+	self.git_processes.remove(&channel);
	592	+	self.pipe_uploads.remove(&channel);
	593	+	Ok(())
	594	+	}
	595	+
550	596		async fn window_change_request(
551	597		&mut self,
552	598		_channel: ChannelId,