Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: window state preservation by soft-killing the process #3579

Merged
merged 1 commit into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/cli/src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/serve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 } => {
Expand All @@ -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());
Expand Down
59 changes: 53 additions & 6 deletions packages/cli/src/serve/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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::<Vec<_>>();

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::<Vec<_>>();
for platform in keys {
self.kill(platform).await;
}
}

/// Open an existing app bundle, if it exists
Expand Down
Loading