From a729968ee47b066e6d55dd9e0f8c2a1f1aef79e0 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Thu, 16 Jan 2025 17:53:09 -0800 Subject: [PATCH] fix: window state preservation by soft-killing the process (#3579) --- packages/cli/src/cli/run.rs | 2 +- packages/cli/src/serve/mod.rs | 6 ++-- packages/cli/src/serve/runner.rs | 59 ++++++++++++++++++++++++++++---- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/cli/run.rs b/packages/cli/src/cli/run.rs index 7e6537343..2c0e83d5b 100644 --- a/packages/cli/src/cli/run.rs +++ b/packages/cli/src/cli/run.rs @@ -46,7 +46,7 @@ impl RunArgs { tracing::info!("[{platform}]: {msg}") } ServeUpdate::ProcessExited { platform, status } => { - runner.kill(platform); + runner.kill(platform).await; tracing::info!("[{platform}]: process exited with status: {status:?}"); break; } diff --git a/packages/cli/src/serve/mod.rs b/packages/cli/src/serve/mod.rs index efaf9c43c..e4bc0b300 100644 --- a/packages/cli/src/serve/mod.rs +++ b/packages/cli/src/serve/mod.rs @@ -109,7 +109,7 @@ pub(crate) async fn serve_all(mut args: ServeArgs) -> Result<()> { // Kill any running executables on Windows if cfg!(windows) { - runner.kill_all(); + runner.kill_all().await; } // We're going to kick off a new build, interrupting the current build if it's ongoing @@ -200,7 +200,7 @@ pub(crate) async fn serve_all(mut args: ServeArgs) -> Result<()> { ); } - runner.kill(platform); + runner.kill(platform).await; } ServeUpdate::StdoutReceived { platform, msg } => { @@ -223,7 +223,7 @@ pub(crate) async fn serve_all(mut args: ServeArgs) -> Result<()> { // Kill any running executables on Windows if cfg!(windows) { - runner.kill_all(); + runner.kill_all().await; } builder.rebuild(args.build_arguments.clone()); diff --git a/packages/cli/src/serve/runner.rs b/packages/cli/src/serve/runner.rs index e2d696cc4..4d9817899 100644 --- a/packages/cli/src/serve/runner.rs +++ b/packages/cli/src/serve/runner.rs @@ -5,7 +5,7 @@ use crate::{ use dioxus_core::internal::TemplateGlobalKey; use dioxus_devtools_types::HotReloadMsg; use dioxus_html::HtmlCtx; -use futures_util::{future::OptionFuture, stream::FuturesUnordered}; +use futures_util::{future::OptionFuture, stream::FuturesUnordered, FutureExt}; use ignore::gitignore::Gitignore; use std::{ collections::{HashMap, HashSet}, @@ -105,7 +105,7 @@ impl AppRunner { // Drop the old handle // todo(jon): we should instead be sending the kill signal rather than dropping the process // This would allow a more graceful shutdown and fix bugs like desktop not retaining its size - self.kill(platform); + self.kill(platform).await; // wait a tiny sec for the processes to die so we don't have fullstack servers on top of each other // todo(jon): we should allow rebinding to the same port in fullstack itself @@ -137,12 +137,59 @@ impl AppRunner { Ok(self.running.get(&platform).unwrap()) } - pub(crate) fn kill(&mut self, platform: Platform) { - self.running.remove(&platform); + /// Gracefully kill the process and all of its children + /// + /// Uses the `SIGTERM` signal on unix and `taskkill` on windows. + /// This complex logic is necessary for things like window state preservation to work properly. + pub(crate) async fn kill(&mut self, platform: Platform) { + use tokio::process::Command; + + let Some(mut process) = self.running.remove(&platform) else { + return; + }; + + let server_process = process.server_child.take(); + let client_process = process.app_child.take(); + let processes = [server_process, client_process] + .into_iter() + .flatten() + .collect::>(); + + for mut process in processes { + let Some(pid) = process.id() else { + _ = process.kill().await; + continue; + }; + + // on unix, we can send a signal to the process to shut down + #[cfg(unix)] + { + _ = Command::new("kill") + .args(["-s", "TERM", &pid.to_string()]) + .spawn(); + } + + // on windows, use the `taskkill` command + #[cfg(windows)] + { + _ = Command::new("taskkill") + .args(["/F", "/PID", &pid.to_string()]) + .spawn(); + } + + // join the wait with a 100ms timeout + futures_util::select! { + _ = process.wait().fuse() => {} + _ = tokio::time::sleep(std::time::Duration::from_millis(1000)).fuse() => {} + }; + } } - pub(crate) fn kill_all(&mut self) { - self.running.clear(); + pub(crate) async fn kill_all(&mut self) { + let keys = self.running.keys().cloned().collect::>(); + for platform in keys { + self.kill(platform).await; + } } /// Open an existing app bundle, if it exists