diff --git a/src/bin/dosh-server/unix.rs b/src/bin/dosh-server/unix.rs index 4c96ba3..a33455a 100644 --- a/src/bin/dosh-server/unix.rs +++ b/src/bin/dosh-server/unix.rs @@ -398,6 +398,10 @@ struct Session { holder_control: Option, /// Whether this session's shell lives in a holder process (persistent). persistent: bool, + /// The holder survived a server restart but no client has reattached yet. + /// Such sessions need the full reconnect window because the client may be + /// asleep during an unattended server update. + restart_orphaned: bool, /// Bytes of session output since the screen was last mirrored to disk, used /// to throttle the (atomic) screen-persistence writes. bytes_since_persist: usize, @@ -569,6 +573,7 @@ impl ServerState { empty_since: None, holder_control: control, persistent, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now() - SCREEN_PERSIST_MAX_AGE, @@ -714,6 +719,7 @@ impl ServerState { empty_since: Some(Instant::now()), holder_control: Some(control), persistent: true, + restart_orphaned: true, bytes_since_persist: 0, last_persisted_seq: output_seq, last_screen_persist_at: Instant::now(), @@ -732,6 +738,7 @@ impl ServerState { if let Some(session) = self.sessions.get_mut(session_name) { session.clients.insert(client_id, client); session.empty_since = None; + session.restart_orphaned = false; self.client_index .insert(client_id, session_name.to_string()); } @@ -4457,7 +4464,8 @@ fn cleanup_disconnected_clients(state: &Arc>) { .filter(|(name, session)| { !prewarm.contains(name.as_str()) && session.empty_since.is_some_and(|since| { - now.duration_since(since) >= empty_session_timeout(name, timeout) + now.duration_since(since) + >= empty_session_timeout(name, timeout, session.restart_orphaned) }) }) .map(|(name, _)| name.clone()) @@ -4477,8 +4485,12 @@ fn cleanup_disconnected_clients(state: &Arc>) { } } -fn empty_session_timeout(name: &str, configured_timeout: Duration) -> Duration { - if protocol::is_implicit_session_name(name) { +fn empty_session_timeout( + name: &str, + configured_timeout: Duration, + restart_orphaned: bool, +) -> Duration { + if protocol::is_implicit_session_name(name) && !restart_orphaned { configured_timeout.min(Duration::from_secs(IMPLICIT_EMPTY_SESSION_GRACE_SECS)) } else { configured_timeout @@ -4623,6 +4635,7 @@ mod tests { empty_since: Some(Instant::now()), holder_control: None, persistent: true, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 7, last_screen_persist_at: Instant::now(), @@ -4650,6 +4663,7 @@ mod tests { empty_since: Some(Instant::now()), holder_control: None, persistent: true, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 7, last_screen_persist_at: Instant::now(), @@ -4697,6 +4711,7 @@ mod tests { empty_since: Some(Instant::now()), holder_control: None, persistent: true, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 7, last_screen_persist_at: Instant::now(), @@ -5046,25 +5061,76 @@ mod tests { } #[test] - fn implicit_empty_session_timeout_is_bounded_for_update_reconnect() { + fn implicit_empty_session_timeout_preserves_restart_orphans() { let configured = Duration::from_secs(2_592_000); let implicit = protocol::generate_implicit_session_name(); assert_eq!( - empty_session_timeout(&implicit, configured), + empty_session_timeout(&implicit, configured, false), Duration::from_secs(IMPLICIT_EMPTY_SESSION_GRACE_SECS) ); assert_eq!( - empty_session_timeout("work", configured), + empty_session_timeout(&implicit, configured, true), + configured, + "a sleeping client must retain its restart-surviving shell" + ); + assert_eq!( + empty_session_timeout("work", configured, false), configured, "named sessions keep the normal long timeout" ); assert_eq!( - empty_session_timeout(&implicit, Duration::from_secs(1)), + empty_session_timeout(&implicit, Duration::from_secs(1), false), Duration::from_secs(1), "tests/admins can still configure a shorter timeout" ); } + #[test] + fn cleanup_keeps_restart_orphan_then_reaps_after_reattach_disconnect() { + let (pty_tx, _pty_rx) = mpsc::channel(PTY_OUTPUT_QUEUE_CAPACITY); + let mut state = ServerState::new( + ServerConfig { + client_timeout_secs: 2_592_000, + prewarm_sessions: Vec::new(), + ..ServerConfig::default() + }, + [0u8; 32], + pty_tx, + ); + let implicit = protocol::generate_implicit_session_name(); + state + .ensure_session(&implicit, 80, 24, "forward-only", &[]) + .unwrap(); + { + let session = state.sessions.get_mut(&implicit).unwrap(); + session.empty_since = Some( + Instant::now() + - Duration::from_secs(IMPLICIT_EMPTY_SESSION_GRACE_SECS + 10), + ); + session.restart_orphaned = true; + } + + let state = Arc::new(Mutex::new(state)); + cleanup_disconnected_clients(&state); + assert!( + state.lock().unwrap().sessions.contains_key(&implicit), + "restart orphan was reaped before the reconnect timeout" + ); + + state + .lock() + .unwrap() + .sessions + .get_mut(&implicit) + .unwrap() + .restart_orphaned = false; + cleanup_disconnected_clients(&state); + assert!( + !state.lock().unwrap().sessions.contains_key(&implicit), + "ordinary abandoned implicit session was not reaped" + ); + } + #[tokio::test] async fn forged_plaintext_detach_does_not_remove_client() { let (pty_tx, _pty_rx) = mpsc::channel(PTY_OUTPUT_QUEUE_CAPACITY); @@ -5146,6 +5212,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5211,6 +5278,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5288,6 +5356,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5347,6 +5416,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5524,6 +5594,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5628,6 +5699,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5693,6 +5765,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5750,6 +5823,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5806,6 +5880,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5853,6 +5928,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5916,6 +5992,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -5992,6 +6069,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(), @@ -6099,6 +6177,7 @@ mod tests { empty_since: None, holder_control: None, persistent: false, + restart_orphaned: false, bytes_since_persist: 0, last_persisted_seq: 0, last_screen_persist_at: Instant::now(),