termalHere’s a shape you’ll recognise if you’ve ever put “a bunch of live things” behind one lock. An SSH client keeps its open sessions in a single map:
struct SessionManager {
sessions: HashMap<String, SshSession>,
// ...
}
static SESSION_MANAGER: OnceLock<Arc<Mutex<SessionManager>>> = OnceLock::new();One Arc<Mutex<HashMap<Id, Session>>>, shared by every tab. Reads are cheap — grab the lock, find the session, write some bytes, drop the lock. It worked fine until someone opened a tenth tab and connected to a new host, and every other tab froze for three seconds. Through a jump host, longer.
The bug is one .await too many
The connect path looked like the obvious thing:
async fn connect_session(id: &str, creds: Creds) -> Result<(), String> {
let mgr = get_session_manager();
let mut mgr = mgr.lock().await;
let session = mgr.sessions.get_mut(id).ok_or("not found")?;
session.connect(creds).await // <-- SSH handshake: seconds
}The problem is the last line. session.connect(...).await is the full SSH handshake — key exchange, auth, maybe an agent trying keys in batches, maybe a whole second hop through a jump host. And we’re holding the manager lock across it. Every keystroke in every other session, every stats poll, every “write these bytes to the terminal” now waits in line behind a network round-trip to a machine that might be on the other side of the planet.
Holding a lock across .await isn’t a deadlock here — tokio’s Mutex is fair and it’ll get released. It’s head-of-line blocking: the lock is held for as long as the slowest operation under it takes, and everyone else pays for it.
Move ownership out, do the slow work unlocked
The fix is to stop treating the session as something you borrow from the map for the duration of a slow call. Take it out:
/// Remove a session from the map so a slow `&mut self` op can run on it
/// without holding the manager lock. The caller MUST hand it back.
async fn take_session_for_op(id: &str) -> Result<SshSession, String> {
let mut mgr = get_session_manager().lock().await;
if mgr.in_flight.contains_key(id) {
return Err("an operation is already in progress for this session".into());
}
match mgr.sessions.remove(id) {
Some(session) => {
mgr.in_flight.insert(id.to_string(), false); // false = not cancelled
Ok(session)
}
None => Err("session not found".into()),
}
}Now the lock is held only long enough to remove the entry and record that an operation is running. The connect becomes:
async fn connect_session(id: String, creds: Creds) -> Result<(), String> {
let mut session = take_session_for_op(&id).await?; // brief lock
let result = session.connect(creds).await; // NO lock held
finish_session_op(&id, session).await; // brief lock
result.map_err(|e| e.to_string())
}Between the first and third line the manager lock is free. The ten other tabs keep echoing keystrokes and drawing output while the new connection does its handshake in the background, holding nothing but its own owned SshSession. The slow work is exactly as slow as before — it just stops being everyone else’s problem.
Now handle the two races you just created
The moment a session can be “outside the map,” two things can go wrong, and both have bitten real users.
A second operation on the same session. Someone double-clicks connect, or a reconnect fires while the first is still handshaking. That’s what the in_flight map is for: take_session_for_op refuses if an entry is already there. One in-flight &mut self operation per session, enforced at the door.
A disconnect arrives mid-handshake. This is the nasty one. The user closes the tab while the connection is still coming up. disconnect grabs the lock, looks in sessions… and it’s not there, because we took it out. If you do nothing, the handshake finishes a moment later, finish_session_op puts a now-fully-connected session back into the map, and you have a live connection nobody is watching — a zombie.
So the in-flight marker doubles as a cancel flag:
async fn disconnect_session(id: String) -> Result<(), String> {
let owned = {
let mut mgr = get_session_manager().lock().await;
// Not in the map? It's mid-connect. Flag the in-flight op to tear
// its fresh connection down instead of re-inserting it.
if !mgr.sessions.contains_key(&id) {
if let Some(cancel) = mgr.in_flight.get_mut(&id) {
*cancel = true;
}
}
mgr.sessions.remove(&id) // Some(..) if already connected; None if mid-connect
};
if let Some(mut session) = owned {
session.disconnect().await?; // slow part, lock released
}
Ok(())
}And the return path honours it:
async fn finish_session_op(id: &str, session: SshSession) {
let mut to_drop = None;
{
let mut mgr = get_session_manager().lock().await;
// unwrap_or(true): if the marker vanished, assume cancelled.
let cancelled = mgr.in_flight.remove(id).unwrap_or(true);
if cancelled {
to_drop = Some(session); // don't re-insert
} else {
mgr.sessions.insert(id.into(), session);
}
}
if let Some(mut session) = to_drop {
let _ = session.disconnect().await; // tear the fresh connection down
}
}The decision — reinsert or tear down — is made under the lock by reading one flag; the actual teardown happens after the lock is dropped. A disconnect that races a connect now always wins cleanly: either it removes an already-connected session, or it leaves a note that turns the in-flight connection into a no-op on arrival. No zombie either way.
Reaping the dead, without a background timer
Sessions also die on their own — the server hangs up, the network drops, the OS kills a backgrounded app. Those need collecting too, and the same “short lock, slow work released” discipline applies:
async fn cleanup_dead_sessions() -> Vec<String> {
let (dead, owned) = {
let mut mgr = get_session_manager().lock().await;
let dead: Vec<String> = mgr.sessions.iter()
.filter(|(_, s)| s.is_handle_closed())
.map(|(id, _)| id.clone())
.collect();
let owned: Vec<_> = dead.iter().filter_map(|id| mgr.sessions.remove(id)).collect();
(dead, owned)
};
for mut session in owned {
let _ = session.disconnect().await; // lock released
}
dead
}Two deliberate choices here. There’s no background task — this is the only GC path, and the UI just calls it on a cheap interval (and on demand). A background reaper is one more thing holding the lock on a timer you don’t control; calling it explicitly keeps GC on your schedule. And it only touches sessions that are actually in the map: anything mid-connect (owned out of the map) or never-connected is left alone, so the reaper and an in-flight handshake can never fight over the same session.
The shape, minus the SSH
Strip out the protocol and the pattern is reusable anywhere you keep live, individually-slow things in a shared map:
- Hold the shared lock only long enough to move ownership out (
remove), not for the duration of the work. - Do the slow
asyncwork on the owned value, lock released. - Re-acquire briefly to put it back — with a guard against a second concurrent operation, and a cancel flag so an op that raced the slow work can tear its result down instead of reinserting it.
It costs you a second map (in_flight) and a little bookkeeping, and it buys you a lock that’s never held longer than a HashMap::remove. For anything behind Arc<Mutex<HashMap<_, _>>> where the per-entry operations touch the network, that trade is almost always worth it.
Where this lives
This is from the Rust core of Termalin, a cross-platform SSH client (the protocol layer is russh). The full session manager has more moving parts — per-session output buffers, tunnels, stats collectors — but the connect/disconnect/reap paths all follow the take-and-return discipline above.
Termalin is a fast SSH client for you and your AI agents — have a look, or read how the agent side works.