From 1a9214e2f04781f16abe15c1edaf918d276003f9 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Fri, 8 Nov 2024 08:42:18 -0500 Subject: [PATCH] fix(watch): ensure TUI is shutdown regardless of exit path (#9408) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Description I noticed in https://github.com/vercel/turborepo/issues/9389 that the TUI wasn't exiting if `watch` exited from anything other than a `Ctrl-C` (including a direct SIGINT to the process). This PR ensures that we shut down the TUI and the persistent task handle regardless of how `watch.start()` exits. ### Testing Instructions - Start up a watch task `turbo_dev watch dev` - Find the pid file via `turbo_dev daemon status` - Kill the daemon `kill $(cat /path/to/pid/file)` Before Screenshot 2024-11-07 at 5 42 22 PM After Screenshot 2024-11-07 at 5 39 08 PM --- crates/turborepo-lib/src/cli/mod.rs | 5 ++++- crates/turborepo-lib/src/process/child.rs | 2 +- crates/turborepo-lib/src/run/watch.rs | 21 ++++++++++++++------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/turborepo-lib/src/cli/mod.rs b/crates/turborepo-lib/src/cli/mod.rs index 0185d73f2adfa..77ae10e6b458d 100644 --- a/crates/turborepo-lib/src/cli/mod.rs +++ b/crates/turborepo-lib/src/cli/mod.rs @@ -1391,7 +1391,10 @@ pub async fn run( let base = CommandBase::new(cli_args, repo_root, version, color_config); let mut client = WatchClient::new(base, event).await?; - client.start().await?; + if let Err(e) = client.start().await { + client.shutdown().await; + return Err(e.into()); + } // We only exit if we get a signal, so we return a non-zero exit code return Ok(1); } diff --git a/crates/turborepo-lib/src/process/child.rs b/crates/turborepo-lib/src/process/child.rs index a5a4d3c6281e3..d610852205934 100644 --- a/crates/turborepo-lib/src/process/child.rs +++ b/crates/turborepo-lib/src/process/child.rs @@ -464,7 +464,7 @@ impl Child { // On Windows it is important that this gets dropped once the child process // exits let controller = controller; - debug!("waiting for task"); + debug!("waiting for task: {pid:?}"); let manager = ChildStateManager { shutdown_style, task_state, diff --git a/crates/turborepo-lib/src/run/watch.rs b/crates/turborepo-lib/src/run/watch.rs index 1059acca44eef..dab2c561c82bd 100644 --- a/crates/turborepo-lib/src/run/watch.rs +++ b/crates/turborepo-lib/src/run/watch.rs @@ -217,13 +217,6 @@ impl WatchClient { biased; _ = signal_subscriber.listen() => { tracing::info!("shutting down"); - if let Some(RunHandle { stopper, run_task }) = self.persistent_tasks_handle.take() { - // Shut down the tasks for the run - stopper.stop().await; - // Run should exit shortly after we stop all child tasks, wait for it to finish - // to ensure all messages are flushed. - let _ = run_task.await; - } Err(Error::SignalInterrupt) } result = event_fut => { @@ -267,6 +260,20 @@ impl WatchClient { Ok(()) } + /// Shut down any resources that run as part of watch. + pub async fn shutdown(&mut self) { + if let Some(sender) = &self.ui_sender { + sender.stop().await; + } + if let Some(RunHandle { stopper, run_task }) = self.persistent_tasks_handle.take() { + // Shut down the tasks for the run + stopper.stop().await; + // Run should exit shortly after we stop all child tasks, wait for it to finish + // to ensure all messages are flushed. + let _ = run_task.await; + } + } + /// Executes a run with the given changed packages. Splits the run into two /// parts: /// 1. The persistent tasks that are not allowed to be interrupted