From 798cbfee453421eefd8c6ada24fb1fbe51a33c8b Mon Sep 17 00:00:00 2001 From: Federico Terzi Date: Mon, 17 May 2021 21:02:21 +0200 Subject: [PATCH] feat(core): improve exit code handling and investigate improvement of shell handling on Windows --- Cargo.lock | 6 +- espanso/Cargo.toml | 6 +- espanso/src/cli/daemon/ipc.rs | 56 ++++++++++++ espanso/src/cli/daemon/mod.rs | 157 ++++++++++++++++++++++++---------- espanso/src/cli/log.rs | 8 +- espanso/src/cli/mod.rs | 4 +- espanso/src/cli/path.rs | 4 +- espanso/src/cli/worker/mod.rs | 8 +- espanso/src/main.rs | 8 +- espanso/src/util.rs | 12 +++ packager/win/espanso.cmd | 1 + 11 files changed, 211 insertions(+), 59 deletions(-) create mode 100644 espanso/src/cli/daemon/ipc.rs create mode 100644 packager/win/espanso.cmd diff --git a/Cargo.lock b/Cargo.lock index ee2e45a..347a27e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -319,10 +319,12 @@ dependencies = [ "log-panics", "maplit", "markdown", + "named_pipe", "serde", "serde_json", "simplelog", "thiserror", + "winapi", ] [[package]] @@ -612,9 +614,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.85" +version = "0.2.94" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ccac4b00700875e6a07c6cde370d44d32fa01c5a65cdd2fca6858c479d28bb3" +checksum = "18794a8ad5b29321f790b55d93dfba91e125cb1a9edbd4f8e3150acc771c1a5e" [[package]] name = "libdbus-sys" diff --git a/espanso/Cargo.toml b/espanso/Cargo.toml index 2439ed6..89f246f 100644 --- a/espanso/Cargo.toml +++ b/espanso/Cargo.toml @@ -39,4 +39,8 @@ serde_json = "1.0.62" markdown = "0.3.0" html2text = "0.2.1" log-panics = "2.0.0" -fs2 = "0.4.3" \ No newline at end of file +fs2 = "0.4.3" + +[target.'cfg(windows)'.dependencies] +named_pipe = "0.4.1" +winapi = { version = "0.3.9", features = ["wincon"] } \ No newline at end of file diff --git a/espanso/src/cli/daemon/ipc.rs b/espanso/src/cli/daemon/ipc.rs new file mode 100644 index 0000000..efee57d --- /dev/null +++ b/espanso/src/cli/daemon/ipc.rs @@ -0,0 +1,56 @@ +/* + * This file is part of espanso. + * + * Copyright (C) 2019-2021 Federico Terzi + * + * espanso is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * espanso is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with espanso. If not, see . + */ + +use std::path::Path; + +use anyhow::Result; +use crossbeam::channel::{Sender}; +use log::{error, warn}; + +use crate::ipc::IPCEvent; + +use super::ExitCode; + +pub fn initialize_and_spawn(runtime_dir: &Path, exit_notify: Sender) -> Result<()> { + let receiver = crate::ipc::spawn_daemon_ipc_server(runtime_dir)?; + + std::thread::Builder::new() + .name("daemon-ipc-handler".to_string()) + .spawn(move || loop { + match receiver.recv() { + Ok(event) => { + match event { + IPCEvent::Exit => { + if let Err(err) = exit_notify.send(ExitCode::Success as i32) { + error!("experienced error while sending exit signal from daemon ipc handler: {}", err); + } + }, + unexpected_event => { + warn!("received unexpected event in daemon ipc handler: {:?}", unexpected_event); + }, + } + } + Err(err) => { + error!("experienced error while receiving ipc event from daemon handler: {}", err); + } + } + })?; + + Ok(()) +} diff --git a/espanso/src/cli/daemon/mod.rs b/espanso/src/cli/daemon/mod.rs index c843762..e35427e 100644 --- a/espanso/src/cli/daemon/mod.rs +++ b/espanso/src/cli/daemon/mod.rs @@ -19,20 +19,35 @@ use std::{path::Path, process::Command, time::Instant}; +use crossbeam::{ + channel::{unbounded, Sender}, + select, +}; use espanso_ipc::IPCClient; +use espanso_path::Paths; use log::{error, info, warn}; -use crate::{ipc::{IPCEvent, create_ipc_client_to_worker}, lock::{acquire_daemon_lock, acquire_worker_lock}}; +use crate::{ + ipc::{create_ipc_client_to_worker, IPCEvent}, + lock::{acquire_daemon_lock, acquire_worker_lock}, +}; use super::{CliModule, CliModuleArgs}; +mod ipc; + +pub enum ExitCode { + Success = 0, + ExitCodeUnwrapError = 100, +} + pub fn new() -> CliModule { #[allow(clippy::needless_update)] CliModule { requires_paths: true, requires_config: true, enable_logs: true, - log_mode: super::LogMode::Write, + log_mode: super::LogMode::CleanAndAppend, subcommand: "daemon".to_string(), entry: daemon_main, ..Default::default() @@ -41,16 +56,18 @@ pub fn new() -> CliModule { const VERSION: &str = env!("CARGO_PKG_VERSION"); -fn daemon_main(args: CliModuleArgs) { +fn daemon_main(args: CliModuleArgs) -> i32 { let paths = args.paths.expect("missing paths in daemon main"); // Make sure only one instance of the daemon is running let lock_file = acquire_daemon_lock(&paths.runtime); if lock_file.is_none() { error!("daemon is already running!"); - std::process::exit(1); + return 1; } + // TODO: we might need to check preconditions: accessibility on macOS, presence of binaries on Linux, etc + info!("espanso version: {}", VERSION); // TODO: print os system and version? (with os_info crate) @@ -59,13 +76,74 @@ fn daemon_main(args: CliModuleArgs) { terminate_worker_if_already_running(&paths.runtime, worker_ipc); + let (exit_notify, exit_signal) = unbounded::(); + // TODO: register signals to terminate the worker if the daemon terminates + spawn_worker(&paths, exit_notify.clone()); + + ipc::initialize_and_spawn(&paths.runtime, exit_notify) + .expect("unable to initialize ipc server for daemon"); + + // TODO: start file watcher thread + + let mut exit_code: i32 = ExitCode::Success as i32; + + loop { + select! { + recv(exit_signal) -> code => { + match code { + Ok(code) => { + exit_code = code + }, + Err(err) => { + error!("received error when unwrapping exit_code: {}", err); + exit_code = ExitCode::ExitCodeUnwrapError as i32; + }, + } + break; + }, + } + } + + exit_code +} + +fn terminate_worker_if_already_running(runtime_dir: &Path, worker_ipc: impl IPCClient) { + let lock_file = acquire_worker_lock(&runtime_dir); + if lock_file.is_some() { + return; + } + + warn!("a worker process is already running, sending termination signal..."); + if let Err(err) = worker_ipc.send(IPCEvent::Exit) { + error!( + "unable to send termination signal to worker process: {}", + err + ); + } + + let now = Instant::now(); + while now.elapsed() < std::time::Duration::from_secs(3) { + let lock_file = acquire_worker_lock(runtime_dir); + if lock_file.is_some() { + return; + } + + std::thread::sleep(std::time::Duration::from_millis(200)); + } + + panic!( + "could not terminate worker process, please kill it manually, otherwise espanso won't start" + ) +} + +fn spawn_worker(paths: &Paths, exit_notify: Sender) { + info!("spawning the worker process..."); + let espanso_exe_path = std::env::current_exe().expect("unable to obtain espanso executable location"); - info!("spawning the worker process..."); - let mut command = Command::new(&espanso_exe_path.to_string_lossy().to_string()); command.args(&["worker"]); command.env( @@ -81,44 +159,35 @@ fn daemon_main(args: CliModuleArgs) { paths.runtime.to_string_lossy().to_string(), ); - // On windows, we need to spawn the process as "Detached" - #[cfg(target_os = "windows")] - { - use std::os::windows::process::CommandExt; - command.creation_flags(0x08000008); // Detached process without window - } + // TODO: investigate if this is needed here, especially when invoking a form + // // On windows, we need to spawn the process as "Detached" + // #[cfg(target_os = "windows")] + // { + // use std::os::windows::process::CommandExt; + // //command.creation_flags(0x08000008); // CREATE_NO_WINDOW + DETACHED_PROCESS + // } - command.spawn().expect("unable to spawn worker process"); + let mut child = command.spawn().expect("unable to spawn worker process"); - // TODO: start IPC server - - // TODO: start file watcher thread - - loop { - std::thread::sleep(std::time::Duration::from_millis(1000)); - } + // Create a monitor thread that will exit with the same non-zero code if + // the worker thread exits + std::thread::Builder::new() + .name("worker-status-monitor".to_string()) + .spawn(move || { + let result = child.wait(); + if let Ok(status) = result { + if let Some(code) = status.code() { + if code != 0 { + error!( + "worker process exited with non-zero code: {}, exiting", + code + ); + exit_notify + .send(code) + .expect("unable to forward worker exit code"); + } + } + } + }) + .expect("Unable to spawn worker monitor thread"); } - -fn terminate_worker_if_already_running(runtime_dir: &Path, worker_ipc: impl IPCClient) { - let lock_file = acquire_worker_lock(&runtime_dir); - if lock_file.is_some() { - return; - } - - warn!("a worker process is already running, sending termination signal..."); - if let Err(err) = worker_ipc.send(IPCEvent::Exit) { - error!("unable to send termination signal to worker process: {}", err); - } - - let now = Instant::now(); - while now.elapsed() < std::time::Duration::from_secs(3) { - let lock_file = acquire_worker_lock(runtime_dir); - if lock_file.is_some() { - return; - } - - std::thread::sleep(std::time::Duration::from_millis(200)); - } - - panic!("could not terminate worker process, please kill it manually, otherwise espanso won't start") -} \ No newline at end of file diff --git a/espanso/src/cli/log.rs b/espanso/src/cli/log.rs index ab0bd28..ea47489 100644 --- a/espanso/src/cli/log.rs +++ b/espanso/src/cli/log.rs @@ -31,13 +31,13 @@ pub fn new() -> CliModule { } } -fn log_main(args: CliModuleArgs) { +fn log_main(args: CliModuleArgs) -> i32 { let paths = args.paths.expect("missing paths argument"); let log_file = paths.runtime.join(crate::LOG_FILE_NAME); if !log_file.exists() { eprintln!("No log file found."); - std::process::exit(2); + return 2; } let log_file = File::open(log_file); @@ -50,6 +50,8 @@ fn log_main(args: CliModuleArgs) { } } else { eprintln!("Error reading log file"); - std::process::exit(1); + return 1; } + + 0 } diff --git a/espanso/src/cli/mod.rs b/espanso/src/cli/mod.rs index c1da747..7cd1633 100644 --- a/espanso/src/cli/mod.rs +++ b/espanso/src/cli/mod.rs @@ -32,7 +32,7 @@ pub struct CliModule { pub requires_paths: bool, pub requires_config: bool, pub subcommand: String, - pub entry: fn(CliModuleArgs), + pub entry: fn(CliModuleArgs)->i32, } impl Default for CliModule { @@ -43,7 +43,7 @@ impl Default for CliModule { requires_paths: false, requires_config: false, subcommand: "".to_string(), - entry: |_| {}, + entry: |_| {0}, } } } diff --git a/espanso/src/cli/path.rs b/espanso/src/cli/path.rs index e5a276b..e80717e 100644 --- a/espanso/src/cli/path.rs +++ b/espanso/src/cli/path.rs @@ -29,7 +29,7 @@ pub fn new() -> CliModule { } } -fn path_main(args: CliModuleArgs) { +fn path_main(args: CliModuleArgs) -> i32 { let paths = args.paths.expect("missing paths argument"); let cli_args = args.cli_args.expect("missing cli_args argument"); @@ -56,4 +56,6 @@ fn path_main(args: CliModuleArgs) { println!("Packages: {}", paths.packages.to_string_lossy()); println!("Runtime: {}", paths.runtime.to_string_lossy()); } + + 0 } diff --git a/espanso/src/cli/worker/mod.rs b/espanso/src/cli/worker/mod.rs index 840005b..7d445d2 100644 --- a/espanso/src/cli/worker/mod.rs +++ b/espanso/src/cli/worker/mod.rs @@ -37,21 +37,21 @@ pub fn new() -> CliModule { requires_paths: true, requires_config: true, enable_logs: true, - log_mode: super::LogMode::Append, + log_mode: super::LogMode::AppendOnly, subcommand: "worker".to_string(), entry: worker_main, ..Default::default() } } -fn worker_main(args: CliModuleArgs) { +fn worker_main(args: CliModuleArgs) -> i32 { let paths = args.paths.expect("missing paths in worker main"); // Avoid running multiple worker instances let lock_file = acquire_worker_lock(&paths.runtime); if lock_file.is_none() { error!("worker is already running!"); - std::process::exit(1); + return 1; } let config_store = args @@ -101,4 +101,6 @@ fn worker_main(args: CliModuleArgs) { })); info!("exiting worker process..."); + + 0 } diff --git a/espanso/src/main.rs b/espanso/src/main.rs index 2d8c2f4..0e3ff4b 100644 --- a/espanso/src/main.rs +++ b/espanso/src/main.rs @@ -18,7 +18,7 @@ */ // This is needed to avoid showing a console window when starting espanso on Windows -// TODO #![windows_subsystem = "windows"] +#![windows_subsystem = "windows"] #[macro_use] extern crate lazy_static; @@ -56,7 +56,7 @@ lazy_static! { } fn main() { - // TODO: attach console + util::attach_console(); let install_subcommand = SubCommand::with_name("install") .about("Install a package. Equivalent to 'espanso package install'") @@ -325,7 +325,9 @@ fn main() { cli_args.cli_args = Some(args.clone()); } - (handler.entry)(cli_args) + let exit_code = (handler.entry)(cli_args); + + std::process::exit(exit_code); } else { clap_instance .print_long_help() diff --git a/espanso/src/util.rs b/espanso/src/util.rs index 99a9251..d5bea46 100644 --- a/espanso/src/util.rs +++ b/espanso/src/util.rs @@ -30,4 +30,16 @@ pub fn set_command_flags(command: &mut Command) { #[cfg(not(target_os = "windows"))] pub fn set_command_flags(_: &mut Command) { // NOOP on Linux and macOS +} + +#[cfg(target_os = "windows")] +pub fn attach_console() { + // When using the windows subsystem we loose the terminal output. + // Therefore we try to attach to the current console if available. + unsafe { winapi::um::wincon::AttachConsole(0xFFFFFFFF) }; +} + +#[cfg(not(target_os = "windows"))] +pub fn attach_console() { + // Not necessary on Linux and macOS } \ No newline at end of file diff --git a/packager/win/espanso.cmd b/packager/win/espanso.cmd new file mode 100644 index 0000000..83de66f --- /dev/null +++ b/packager/win/espanso.cmd @@ -0,0 +1 @@ +@"%~dp0espansow.exe" %* \ No newline at end of file