diff --git a/docs/getting-started.md b/docs/getting-started.md index a7c52547..0a5dd318 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -62,7 +62,7 @@ nightcrow update nightcrow update --version 0.1.1 ``` -By default this downloads and verifies the latest binary from the official GitHub Release. Use `--version VER` to roll back to a published patch. Use `--path DIR` for a local checkout or `--git URL` for another source repository; those explicit development modes require Rust and run a locked, forced `cargo install`. Restart the session after updating so the daemon and its panes use the new binary. On Windows, `update` moves the installed executable aside before replacing it. +By default this downloads and verifies the latest binary from the official GitHub Release. Use `--version VER` to roll back to a published patch. Use `--path DIR` for a local checkout or `--git URL` for another source repository; those explicit development modes require Rust and run a locked, forced `cargo install`. Restart a running session after updating with `nightcrow stop`, then start nightcrow again so the daemon and its panes use the new binary. On Windows, `update` moves the installed executable aside before replacing it; if that parked copy is still in use, cleanup waits for the session or updater to exit. The new binary is already installed, so a second `update` is not needed. ## Building and testing diff --git a/src/cli/stop.rs b/src/cli/stop.rs index 71b1ce6c..75c1bf98 100644 --- a/src/cli/stop.rs +++ b/src/cli/stop.rs @@ -12,6 +12,7 @@ use crate::daemon::protocol::ServerMessage; // bounded cleanup of a full configured session while still rejecting a lost // request instead of waiting forever. const SHUTDOWN_ACK_TIMEOUT: Duration = Duration::from_secs(20); +const SHUTDOWN_POLL_INTERVAL: Duration = Duration::from_millis(25); /// Send a graceful shutdown request to a running daemon. pub(crate) fn run_stop(socket: Option) -> Result<()> { @@ -41,9 +42,22 @@ pub(crate) fn run_stop(socket: Option) -> Result<()> { .set_read_timeout(Some(SHUTDOWN_ACK_TIMEOUT)) .context("setting the shutdown acknowledgment timeout")?; - wait_for_shutdown_ack(&mut stream, Instant::now() + SHUTDOWN_ACK_TIMEOUT)?; + let deadline = Instant::now() + SHUTDOWN_ACK_TIMEOUT; + wait_for_shutdown_ack(&mut stream, deadline)?; + wait_for_daemon_exit(&path, deadline)?; - println!("nightcrow: daemon is shutting down"); + println!("nightcrow: daemon stopped"); + Ok(()) +} + +/// Wait until the daemon has dropped its socket and instance lock. +fn wait_for_daemon_exit(path: &std::path::Path, deadline: Instant) -> Result<()> { + while path.exists() { + if Instant::now() >= deadline { + anyhow::bail!("timed out waiting for the daemon to stop"); + } + std::thread::sleep(SHUTDOWN_POLL_INTERVAL); + } Ok(()) } diff --git a/src/cli/stop_tests.rs b/src/cli/stop_tests.rs index 557def6f..d19dde5f 100644 --- a/src/cli/stop_tests.rs +++ b/src/cli/stop_tests.rs @@ -70,6 +70,38 @@ fn clean_eof_is_a_shutdown_acknowledgment() { assert!(wait_for_shutdown_ack(&mut Cursor::new(Vec::new()), future_deadline()).is_ok()); } +#[test] +fn stop_waits_until_the_daemon_socket_is_released() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("daemon.sock"); + std::fs::write(&path, b"socket").unwrap(); + let release_path = path.clone(); + let releaser = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(50)); + std::fs::remove_file(release_path).unwrap(); + }); + + wait_for_daemon_exit(&path, future_deadline()).expect("waits for socket release"); + + releaser.join().unwrap(); + assert!(!path.exists()); +} + +#[test] +fn stop_fails_when_the_daemon_socket_is_not_released() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("daemon.sock"); + std::fs::write(&path, b"socket").unwrap(); + + let error = wait_for_daemon_exit(&path, Instant::now()).expect_err("expired deadline"); + + assert!( + error + .to_string() + .contains("timed out waiting for the daemon to stop") + ); +} + #[test] fn reset_and_abort_are_shutdown_acknowledgments() { for kind in [ diff --git a/src/cli/update/replace.rs b/src/cli/update/replace.rs index 45b50421..4a3700c4 100644 --- a/src/cli/update/replace.rs +++ b/src/cli/update/replace.rs @@ -14,11 +14,6 @@ pub(super) fn replace_target( target.display() ) })?; - if parked.is_some() { - println!( - "nightcrow: moved the installed binary aside — a running session keeps using it until it exits" - ); - } match install(target).and_then(|()| { if target.is_file() { @@ -47,18 +42,19 @@ pub(super) fn replace_target( } fn finish_success(parked: Option<&Path>) -> Result<()> { - if let Some(parked) = parked - && !self_replace::discard(parked) - { - println!( - "nightcrow: the previous binary is still in use and was left at {} — it is removed on a later start", - parked.display() - ); - } - println!("nightcrow: updated — restart the session to run the new version"); + let cleanup_pending = parked.is_some_and(|parked| !self_replace::discard(parked)); + println!("{}", success_message(cleanup_pending)); Ok(()) } +fn success_message(cleanup_pending: bool) -> &'static str { + if cleanup_pending { + "nightcrow: update installed successfully.\nnightcrow: the parked old binary is still in use by the running session or updater; cleanup is pending.\nnightcrow: no second update is needed. If a session is running, run `nightcrow stop`, then start nightcrow again to use the new version." + } else { + "nightcrow: update installed successfully.\nnightcrow: no second update is needed. If a session is running, run `nightcrow stop`, then start nightcrow again to use the new version." + } +} + fn rollback(target: &Path, parked: Option<&Path>) -> Result<()> { if target.exists() { std::fs::remove_file(target).with_context(|| { diff --git a/src/cli/update/replace_tests.rs b/src/cli/update/replace_tests.rs index 1ac9caee..e834ec46 100644 --- a/src/cli/update/replace_tests.rs +++ b/src/cli/update/replace_tests.rs @@ -30,3 +30,19 @@ fn a_successful_install_replaces_the_previous_binary() { assert_eq!(std::fs::read(&target).unwrap(), b"new"); } + +#[test] +fn successful_update_message_explains_pending_cleanup_without_a_path() { + assert_eq!( + success_message(true), + "nightcrow: update installed successfully.\nnightcrow: the parked old binary is still in use by the running session or updater; cleanup is pending.\nnightcrow: no second update is needed. If a session is running, run `nightcrow stop`, then start nightcrow again to use the new version." + ); +} + +#[test] +fn successful_update_message_explains_restart_after_cleanup() { + assert_eq!( + success_message(false), + "nightcrow: update installed successfully.\nnightcrow: no second update is needed. If a session is running, run `nightcrow stop`, then start nightcrow again to use the new version." + ); +} diff --git a/src/daemon/serve_tests/status.rs b/src/daemon/serve_tests/status.rs index 09546fa1..c4954745 100644 --- a/src/daemon/serve_tests/status.rs +++ b/src/daemon/serve_tests/status.rs @@ -81,18 +81,21 @@ fn an_invalid_first_request_is_refused_without_attaching() { fn stop_request_is_still_accepted_before_attach() { let dir = tempfile::TempDir::new().unwrap(); let daemon = daemon(&dir, &[]); - - crate::cli::run_stop(Some(daemon.path().to_path_buf())).expect("stop request succeeds"); - - assert_eq!( - daemon - .shutdown_rx - .recv_timeout(std::time::Duration::from_millis(100)) - .expect("the daemon receives the stop signal"), - crate::platform::signals::Shutdown::Terminate - ); + let path = daemon.path().to_path_buf(); + let stopping = std::thread::spawn(move || crate::cli::run_stop(Some(path))); + + let signal = daemon + .shutdown_rx + .recv_timeout(std::time::Duration::from_millis(100)) + .expect("the daemon receives the stop signal"); + assert_eq!(signal, crate::platform::signals::Shutdown::Terminate); assert_eq!(daemon.session.clients.len(), 0); assert!(daemon.session.bridges.lock().unwrap().is_empty()); + drop(daemon); + stopping + .join() + .expect("stop request thread does not panic") + .expect("stop request succeeds"); } #[test]