From e1d8bd6a914fb4528f873f8650ff9317e9e14831 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Sun, 1 Sep 2024 22:57:22 -0700 Subject: [PATCH 01/36] Add windows service functionality #344 --- Cargo.lock | 146 +++++++--- pueue/Cargo.toml | 3 + pueue/src/bin/pueued.rs | 26 +- pueue/src/daemon/cli.rs | 19 ++ pueue/src/daemon/mod.rs | 2 + pueue/src/daemon/service.rs | 515 ++++++++++++++++++++++++++++++++++++ 6 files changed, 678 insertions(+), 33 deletions(-) create mode 100644 pueue/src/daemon/service.rs diff --git a/Cargo.lock b/Cargo.lock index b6f13667..3153875d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -291,7 +291,7 @@ dependencies = [ "num-traits", "serde", "wasm-bindgen", - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -777,7 +777,7 @@ dependencies = [ "iana-time-zone-haiku", "js-sys", "wasm-bindgen", - "windows-core", + "windows-core 0.52.0", ] [[package]] @@ -864,7 +864,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c2a198fb6b0eada2a8df47933734e6d35d350665a33a3593d7164fa52c75c19" dependencies = [ "cfg-if", - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -1108,7 +1108,7 @@ dependencies = [ "libc", "redox_syscall 0.5.1", "smallvec", - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -1288,6 +1288,7 @@ dependencies = [ "handlebars", "interim", "log", + "once_cell", "pest", "pest_derive", "pretty_assertions", @@ -1308,6 +1309,8 @@ dependencies = [ "test-log", "tokio", "whoami", + "windows", + "windows-service", ] [[package]] @@ -2193,6 +2196,12 @@ dependencies = [ "web-sys", ] +[[package]] +name = "widestring" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7219d36b6eac893fa81e84ebe06485e7dcbb616177469b142df14f1f4deb1311" + [[package]] name = "winapi" version = "0.3.9" @@ -2224,13 +2233,88 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd04d41d93c4992d421894c18c8b43496aa748dd4c081bac0dc93eb0489272b6" +dependencies = [ + "windows-core 0.58.0", + "windows-targets 0.52.6", +] + [[package]] name = "windows-core" version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9" dependencies = [ - "windows-targets 0.52.5", + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-core" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ba6d44ec8c2591c134257ce647b7ea6b20335bf6379a27dac5f1641fcf59f99" +dependencies = [ + "windows-implement", + "windows-interface", + "windows-result", + "windows-strings", + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-implement" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2bbd5b46c938e506ecbce286b6628a02171d56153ba733b6c741fc627ec9579b" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "windows-interface" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "053c4c462dc91d3b1504c6fe5a726dd15e216ba718e84a0e46a88fbe5ded3515" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "windows-result" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d1043d8214f791817bab27572aaa8af63732e11bf84aa21a45a78d6c317ae0e" +dependencies = [ + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-service" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d24d6bcc7f734a4091ecf8d7a64c5f7d7066f45585c1861eba06449909609c8a" +dependencies = [ + "bitflags 2.5.0", + "widestring", + "windows-sys 0.52.0", +] + +[[package]] +name = "windows-strings" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cd9b125c486025df0eabcb585e62173c6c9eddcec5d117d3b6e8c30e2ee4d10" +dependencies = [ + "windows-result", + "windows-targets 0.52.6", ] [[package]] @@ -2248,7 +2332,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.5", + "windows-targets 0.52.6", ] [[package]] @@ -2268,18 +2352,18 @@ dependencies = [ [[package]] name = "windows-targets" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f0713a46559409d202e70e28227288446bf7841d3211583a4b53e3f6d96e7eb" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm 0.52.5", - "windows_aarch64_msvc 0.52.5", - "windows_i686_gnu 0.52.5", + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", "windows_i686_gnullvm", - "windows_i686_msvc 0.52.5", - "windows_x86_64_gnu 0.52.5", - "windows_x86_64_gnullvm 0.52.5", - "windows_x86_64_msvc 0.52.5", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] [[package]] @@ -2290,9 +2374,9 @@ checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" [[package]] name = "windows_aarch64_gnullvm" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7088eed71e8b8dda258ecc8bac5fb1153c5cffaf2578fc8ff5d61e23578d3263" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" [[package]] name = "windows_aarch64_msvc" @@ -2302,9 +2386,9 @@ checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" [[package]] name = "windows_aarch64_msvc" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9985fd1504e250c615ca5f281c3f7a6da76213ebd5ccc9561496568a2752afb6" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" [[package]] name = "windows_i686_gnu" @@ -2314,15 +2398,15 @@ checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" [[package]] name = "windows_i686_gnu" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "88ba073cf16d5372720ec942a8ccbf61626074c6d4dd2e745299726ce8b89670" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" [[package]] name = "windows_i686_gnullvm" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87f4261229030a858f36b459e748ae97545d6f1ec60e5e0d6a3d32e0dc232ee9" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" [[package]] name = "windows_i686_msvc" @@ -2332,9 +2416,9 @@ checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" [[package]] name = "windows_i686_msvc" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "db3c2bf3d13d5b658be73463284eaf12830ac9a26a90c717b7f771dfe97487bf" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" [[package]] name = "windows_x86_64_gnu" @@ -2344,9 +2428,9 @@ checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" [[package]] name = "windows_x86_64_gnu" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4e4246f76bdeff09eb48875a0fd3e2af6aada79d409d33011886d3e1581517d9" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" [[package]] name = "windows_x86_64_gnullvm" @@ -2356,9 +2440,9 @@ checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" [[package]] name = "windows_x86_64_gnullvm" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "852298e482cd67c356ddd9570386e2862b5673c85bd5f88df9ab6802b334c596" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" [[package]] name = "windows_x86_64_msvc" @@ -2368,9 +2452,9 @@ checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" [[package]] name = "windows_x86_64_msvc" -version = "0.52.5" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bec47e5bfd1bff0eeaf6d8b485cc1074891a197ab4225d504cb7a1ab88b02bf0" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "yansi" diff --git a/pueue/Cargo.toml b/pueue/Cargo.toml index e0ab31bf..985baa55 100644 --- a/pueue/Cargo.toml +++ b/pueue/Cargo.toml @@ -39,6 +39,7 @@ strum = { workspace = true } strum_macros = { workspace = true } tempfile = "3" tokio = { workspace = true } +once_cell = "1.19.0" [dev-dependencies] anyhow = { workspace = true } @@ -62,6 +63,8 @@ test-log = "0.2" crossterm = { version = "0.27", default-features = false } [target.'cfg(windows)'.dependencies] crossterm = { version = "0.27", default-features = false, features = ["windows"] } +windows-service = "0.7.0" +windows = { version = "0.58.0", features = ["Win32_System_RemoteDesktop", "Win32_Security", "Win32_System_Threading", "Win32_System_SystemServices", "Win32_System_Environment"] } # Test specific dev-dependencies [target.'cfg(any(target_os = "linux", target_os = "freebsd"))'.dependencies] diff --git a/pueue/src/bin/pueued.rs b/pueue/src/bin/pueued.rs index 6ba11eb7..8c3f0408 100644 --- a/pueue/src/bin/pueued.rs +++ b/pueue/src/bin/pueued.rs @@ -5,8 +5,9 @@ use clap::Parser; use log::warn; use simplelog::{Config, ConfigBuilder, LevelFilter, SimpleLogger, TermLogger, TerminalMode}; -use pueue::daemon::cli::CliArguments; -use pueue::daemon::run; +#[cfg(target_os = "windows")] +use pueue::daemon::service; +use pueue::daemon::{cli::CliArguments, run}; #[tokio::main(flavor = "multi_thread", worker_threads = 4)] async fn main() -> Result<()> { @@ -47,6 +48,27 @@ async fn main() -> Result<()> { SimpleLogger::init(level, logger_config).unwrap(); } + #[cfg(target_os = "windows")] + { + if opt.service { + // start service + service::start_service(opt.config.clone(), opt.profile.clone())?; + return Ok(()); + } + + if opt.install { + service::install_service(opt.config.clone(), opt.profile.clone())?; + println!("Successfully installed `pueued` Windows service"); + return Ok(()); + } + + if opt.uninstall { + service::uninstall_service()?; + println!("Successfully uninstalled `pueued` Windows service"); + return Ok(()); + } + } + run(opt.config, opt.profile, false).await } diff --git a/pueue/src/daemon/cli.rs b/pueue/src/daemon/cli.rs index 5a46a5eb..94d97360 100644 --- a/pueue/src/daemon/cli.rs +++ b/pueue/src/daemon/cli.rs @@ -24,4 +24,23 @@ pub struct CliArguments { /// The name of the profile that should be loaded from your config file. #[arg(short, long)] pub profile: Option, + + /// Install as a Windows service. + /// Once installed, you must not move the binary, otherwise the Windows + /// service will not be able to find it. If you wish to move the binary, + /// first uninstall the service, then install the service again. + #[cfg(target_os = "windows")] + #[arg(long, conflicts_with_all = ["daemonize", "uninstall"])] + pub install: bool, + + /// Uninstall the Windows service. + #[cfg(target_os = "windows")] + #[arg(long, conflicts_with_all = ["daemonize", "install"])] + pub uninstall: bool, + + /// Start the Windows service. This command is internal and should never + /// be used. As a user, to manage the service, use the Windows Service Manager. + #[cfg(target_os = "windows")] + #[arg(long, conflicts_with_all = ["daemonize", "install", "uninstall"])] + pub service: bool, } diff --git a/pueue/src/daemon/mod.rs b/pueue/src/daemon/mod.rs index 3fd39575..c226f9ca 100644 --- a/pueue/src/daemon/mod.rs +++ b/pueue/src/daemon/mod.rs @@ -23,6 +23,8 @@ pub mod cli; mod network; mod pid; mod process_handler; +#[cfg(target_os = "windows")] +pub mod service; /// Contains re-usable helper functions, that operate on the pueue-lib state. pub mod state_helper; pub mod task_handler; diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs new file mode 100644 index 00000000..32733e5a --- /dev/null +++ b/pueue/src/daemon/service.rs @@ -0,0 +1,515 @@ +use std::{ + env, + ffi::{c_void, OsString}, + iter, + path::PathBuf, + process, ptr, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, Mutex, + }, + thread, + time::Duration, +}; + +use anyhow::{anyhow, bail, Result}; +use log::error; +use once_cell::sync::OnceCell; +use windows::{ + core::{PCWSTR, PWSTR}, + Win32::{ + Foundation::{CloseHandle, HANDLE, LUID}, + Security::{ + AdjustTokenPrivileges, DuplicateTokenEx, LookupPrivilegeValueW, SecurityIdentification, + TokenPrimary, SECURITY_ATTRIBUTES, SE_PRIVILEGE_ENABLED, SE_PRIVILEGE_REMOVED, + SE_TCB_NAME, TOKEN_ACCESS_MASK, TOKEN_ADJUST_PRIVILEGES, TOKEN_PRIVILEGES, + }, + System::{ + Environment::{CreateEnvironmentBlock, DestroyEnvironmentBlock}, + RemoteDesktop::{WTSGetActiveConsoleSessionId, WTSQueryUserToken}, + SystemServices::MAXIMUM_ALLOWED, + Threading::{ + CreateProcessAsUserW, OpenProcess, OpenProcessToken, TerminateProcess, + WaitForSingleObject, CREATE_NO_WINDOW, CREATE_UNICODE_ENVIRONMENT, INFINITE, + PROCESS_INFORMATION, PROCESS_QUERY_INFORMATION, STARTUPINFOW, + }, + }, + }, +}; +use windows_service::{ + define_windows_service, + service::{ + ServiceAccess, ServiceControl, ServiceControlAccept, ServiceErrorControl, ServiceExitCode, + ServiceInfo, ServiceStartType, ServiceState, ServiceStatus, ServiceType, + SessionChangeReason, + }, + service_control_handler::{self, ServiceControlHandlerResult}, + service_dispatcher, + service_manager::{ServiceManager, ServiceManagerAccess}, +}; + +#[derive(Clone)] +struct Config { + config_path: Option, + profile: Option, +} + +const SERVICE_NAME: &str = "pueued"; +const SERVICE_TYPE: ServiceType = ServiceType::OWN_PROCESS; +static CONFIG: OnceCell = OnceCell::new(); + +define_windows_service!(ffi_service_main, service_main); + +pub fn start_service(config_path: Option, profile: Option) -> Result<()> { + CONFIG + .set(Config { + config_path, + profile, + }) + .map_err(|_| anyhow!("static CONFIG set failed"))?; + + service_dispatcher::start(SERVICE_NAME, ffi_service_main)?; + Ok(()) +} + +pub fn install_service(config_path: Option, profile: Option) -> Result<()> { + let manager_access = ServiceManagerAccess::CONNECT | ServiceManagerAccess::CREATE_SERVICE; + let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)?; + + let service_binary_path = std::env::current_exe()?; + + let mut args = vec!["--service".into()]; + if let Some(config_path) = config_path { + args.push("--config".into()); + args.push(config_path.into_os_string()); + } + if let Some(profile) = profile { + args.push("--profile".into()); + args.push(profile.into()); + } + + let service_info = ServiceInfo { + name: SERVICE_NAME.into(), + display_name: SERVICE_NAME.into(), + service_type: SERVICE_TYPE, + start_type: ServiceStartType::AutoStart, + error_control: ServiceErrorControl::Normal, + executable_path: service_binary_path, + launch_arguments: args, + dependencies: vec![], + account_name: None, // run as System + account_password: None, + }; + + let service = service_manager.create_service(&service_info, ServiceAccess::CHANGE_CONFIG)?; + service.set_description("pueued daemon is a task management tool for sequential and parallel execution of long-running tasks.")?; + + Ok(()) +} + +pub fn uninstall_service() -> Result<()> { + let manager_access = ServiceManagerAccess::CONNECT; + let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)?; + + let service_access = ServiceAccess::QUERY_STATUS | ServiceAccess::STOP | ServiceAccess::DELETE; + let service = service_manager.open_service(SERVICE_NAME, service_access)?; + + // The service will be marked for deletion as long as this function call succeeds. + // However, it will not be deleted from the database until it is stopped and all open handles to it are closed. + service.delete()?; + + // Our handle to it is not closed yet. So we can still query it. + if service.query_status()?.current_state != ServiceState::Stopped { + // If the service cannot be stopped, it will be deleted when the system restarts. + service.stop()?; + } + + Ok(()) +} + +fn service_main(_: Vec) { + if let Err(e) = run_service() { + error!("Failed to start service: {e}"); + } +} + +fn run_service() -> Result<()> { + let spawner = Arc::new(Spawner::new()); + + let shutdown = Arc::new(AtomicBool::default()); + + let event_handler = { + let spawner = spawner.clone(); + let shutdown = shutdown.clone(); + + move |control_event| -> ServiceControlHandlerResult { + match control_event { + ServiceControl::Stop => { + spawner.stop(); + shutdown.store(true, Ordering::Relaxed); + + ServiceControlHandlerResult::NoError + } + + ServiceControl::SessionChange(param) => { + match param.reason { + SessionChangeReason::SessionLogon + | SessionChangeReason::RemoteConnect + | SessionChangeReason::SessionUnlock => { + if !spawner.running() { + if let Err(e) = spawner.start(Some(param.notification.session_id)) { + error!("failed to spawn: {e}"); + } + } + } + + SessionChangeReason::SessionLogoff => { + spawner.stop(); + } + + _ => (), + } + + ServiceControlHandlerResult::NoError + } + + // All services must accept Interrogate even if it's a no-op. + ServiceControl::Interrogate => ServiceControlHandlerResult::NoError, + + _ => ServiceControlHandlerResult::NotImplemented, + } + } + }; + + // Register system service event handler + let status_handle = service_control_handler::register(SERVICE_NAME, event_handler)?; + + let set_status = move |state: ServiceState, controls: ServiceControlAccept| -> Result<()> { + status_handle.set_service_status(ServiceStatus { + service_type: SERVICE_TYPE, + current_state: state, + controls_accepted: controls, + exit_code: ServiceExitCode::Win32(0), + checkpoint: 0, + wait_hint: Duration::default(), + process_id: None, + })?; + + Ok(()) + }; + + set_status(ServiceState::StartPending, ServiceControlAccept::STOP)?; + + // make sure we have privileges + if let Err(e) = set_privilege(SE_TCB_NAME, true) { + set_status(ServiceState::Stopped, ServiceControlAccept::empty())?; + bail!("failed to set privileges: {e}"); + } + + if let Err(e) = spawner.start(None) { + //set_status(ServiceState::Stopped, ServiceControlAccept::empty())?; + bail!("failed to spawn: {e}"); + } + + set_status( + ServiceState::Running, + ServiceControlAccept::STOP | ServiceControlAccept::SESSION_CHANGE, + )?; + + while !shutdown.load(Ordering::Relaxed) { + spawner.wait(); + } + + // ensure we kill the daemon after + spawner.stop(); + + set_status(ServiceState::Stopped, ServiceControlAccept::empty())?; + + Ok(()) +} + +fn set_privilege(name: PCWSTR, state: bool) -> Result<()> { + let handle: OwnedHandle = + unsafe { OpenProcess(PROCESS_QUERY_INFORMATION, false, process::id())?.into() }; + + let mut token: OwnedHandle = OwnedHandle::default(); + unsafe { + OpenProcessToken(handle.0, TOKEN_ADJUST_PRIVILEGES, &mut token.0)?; + } + + let mut luid = LUID::default(); + + unsafe { + LookupPrivilegeValueW(PCWSTR::null(), name, &mut luid)?; + } + + let mut tp = TOKEN_PRIVILEGES { + PrivilegeCount: 1, + ..Default::default() + }; + + tp.Privileges[0].Luid = luid; + + let attributes = if state { + SE_PRIVILEGE_ENABLED + } else { + SE_PRIVILEGE_REMOVED + }; + + if state { + tp.Privileges[0].Attributes = attributes; + } else { + tp.Privileges[0].Attributes = attributes; + } + + unsafe { + AdjustTokenPrivileges(token.0, false, Some(&tp), 0, None, None)?; + } + + Ok(()) +} + +fn get_current_session() -> Option { + let session = unsafe { WTSGetActiveConsoleSessionId() }; + + match session { + 0xFFFFFFFF => None, + session => Some(session), + } +} + +fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Result { + let mut query_token: OwnedHandle = OwnedHandle::default(); + unsafe { + WTSQueryUserToken(session_id, &mut query_token.0)?; + } + + let sa = SECURITY_ATTRIBUTES { + bInheritHandle: true.into(), + ..Default::default() + }; + + let mut token = OwnedHandle::default(); + + unsafe { + DuplicateTokenEx( + query_token.0, + TOKEN_ACCESS_MASK(MAXIMUM_ALLOWED), + Some(&sa), + SecurityIdentification, + TokenPrimary, + &mut token.0, + )?; + }; + + let t = cb(token)?; + + Ok(t) +} + +struct OwnedHandle(HANDLE); + +unsafe impl Send for OwnedHandle {} +unsafe impl Sync for OwnedHandle {} + +impl OwnedHandle { + fn is_valid(&self) -> bool { + !self.0.is_invalid() + } +} + +impl Default for OwnedHandle { + fn default() -> Self { + Self(HANDLE::default()) + } +} + +impl From for OwnedHandle { + fn from(value: HANDLE) -> Self { + Self(value) + } +} + +impl Drop for OwnedHandle { + fn drop(&mut self) { + if !self.0.is_invalid() { + _ = unsafe { CloseHandle(self.0) }; + } + } +} + +struct Child(OwnedHandle); + +impl Child { + fn new() -> Self { + Self(OwnedHandle::default()) + } + + fn kill(&mut self) -> Result<()> { + if self.0.is_valid() { + unsafe { TerminateProcess(self.0 .0, 0)? }; + self.0 = OwnedHandle::default(); + } + + Ok(()) + } +} + +impl Default for Child { + fn default() -> Self { + Self::new() + } +} + +impl Drop for Child { + fn drop(&mut self) { + _ = self.kill(); + } +} + +struct EnvBlock(*mut c_void); + +impl EnvBlock { + fn new(token: HANDLE) -> Result { + let mut env = ptr::null_mut(); + unsafe { + CreateEnvironmentBlock(&mut env, token, false)?; + } + + Ok(Self(env)) + } +} + +impl Drop for EnvBlock { + fn drop(&mut self) { + _ = unsafe { DestroyEnvironmentBlock(self.0) }; + } +} + +struct Spawner { + running: Arc, + child: Arc>, +} + +impl Spawner { + fn new() -> Self { + Self { + running: Arc::default(), + child: Arc::default(), + } + } + + fn running(&self) -> bool { + self.running.load(Ordering::Relaxed) + } + + fn stop(&self) { + let mut child = self.child.lock().unwrap(); + if child.kill().is_ok() { + self.running.store(false, Ordering::Relaxed); + } + } + + fn wait(&self) { + let mut handle = { self.child.lock().unwrap().0 .0 }; + + if handle.is_invalid() { + while !self.running.load(Ordering::Relaxed) { + thread::park(); + } + + handle = self.child.lock().unwrap().0 .0; + } + + unsafe { + WaitForSingleObject(handle, INFINITE); + } + } + + fn start(&self, session: Option) -> Result<()> { + let Some(session) = session.or_else(|| get_current_session()) else { + bail!("get_current_session failed"); + }; + + let running = self.running.clone(); + let child = self.child.clone(); + let curr_thread = thread::current(); + _ = thread::spawn(move || { + let res = run_as(session, move |token| { + let mut arguments = Vec::new(); + + let config = CONFIG + .get() + .ok_or_else(|| anyhow!("failed to get CONFIG"))? + .clone(); + + if let Some(config) = &config.config_path { + arguments.push("--config".to_string()); + arguments.push(format!(r#""{}""#, config.to_string_lossy().into_owned())); + } + + if let Some(profile) = &config.profile { + arguments.push("--profile".to_string()); + arguments.push(format!(r#""{profile}""#)); + } + + let arguments = arguments.join(" "); + + // Try to get the path to the current binary, since it may not be in the $PATH. + // If we cannot detect it (for some unknown reason), fallback to the raw `pueued` binary name. + let current_exe = env::current_exe()?.to_string_lossy().to_string(); + + let mut command = format!(r#""{current_exe}" {arguments}"#) + .encode_utf16() + .chain(iter::once(0)) + .collect::>(); + + command.reserve(1024 - command.len()); + + let env_block = EnvBlock::new(token.0)?; + + let mut process_info = PROCESS_INFORMATION::default(); + unsafe { + CreateProcessAsUserW( + token.0, + None, + PWSTR(command.as_mut_ptr()), + None, + None, + false, + CREATE_UNICODE_ENVIRONMENT | CREATE_NO_WINDOW, + Some(env_block.0), + None, + &STARTUPINFOW::default(), + &mut process_info, + )?; + } + + { + let mut lock = child.lock().unwrap(); + *lock = Child(process_info.hProcess.into()); + running.store(true, Ordering::Relaxed); + curr_thread.unpark(); + } + + unsafe { + WaitForSingleObject(process_info.hProcess, INFINITE); + } + + { + let mut lock = child.lock().unwrap(); + _ = lock.kill(); + running.store(false, Ordering::Relaxed); + } + + Ok(()) + }); + + if let Err(e) = res { + error!("spawner failed: {e}"); + } + }); + + Ok(()) + } +} From ec20db26f159c6796248f506c576855c8f80c5ba Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Sun, 1 Sep 2024 23:26:28 -0700 Subject: [PATCH 02/36] Bug fixes and better formatted code --- pueue/src/daemon/service.rs | 45 ++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 32733e5a..1d992755 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -8,7 +8,7 @@ use std::{ atomic::{AtomicBool, Ordering}, Arc, Mutex, }, - thread, + thread::{self, Thread}, time::Duration, }; @@ -21,8 +21,8 @@ use windows::{ Foundation::{CloseHandle, HANDLE, LUID}, Security::{ AdjustTokenPrivileges, DuplicateTokenEx, LookupPrivilegeValueW, SecurityIdentification, - TokenPrimary, SECURITY_ATTRIBUTES, SE_PRIVILEGE_ENABLED, SE_PRIVILEGE_REMOVED, - SE_TCB_NAME, TOKEN_ACCESS_MASK, TOKEN_ADJUST_PRIVILEGES, TOKEN_PRIVILEGES, + TokenPrimary, SE_PRIVILEGE_ENABLED, SE_PRIVILEGE_REMOVED, SE_TCB_NAME, + TOKEN_ACCESS_MASK, TOKEN_ADJUST_PRIVILEGES, TOKEN_PRIVILEGES, }, System::{ Environment::{CreateEnvironmentBlock, DestroyEnvironmentBlock}, @@ -135,7 +135,6 @@ fn service_main(_: Vec) { fn run_service() -> Result<()> { let spawner = Arc::new(Spawner::new()); - let shutdown = Arc::new(AtomicBool::default()); let event_handler = { @@ -198,7 +197,7 @@ fn run_service() -> Result<()> { Ok(()) }; - set_status(ServiceState::StartPending, ServiceControlAccept::STOP)?; + set_status(ServiceState::StartPending, ServiceControlAccept::empty())?; // make sure we have privileges if let Err(e) = set_privilege(SE_TCB_NAME, true) { @@ -206,9 +205,10 @@ fn run_service() -> Result<()> { bail!("failed to set privileges: {e}"); } + // if it fails here, we probably launched before user logged in? + // for that reason we only log, but do not bail and stop the service if let Err(e) = spawner.start(None) { - //set_status(ServiceState::Stopped, ServiceControlAccept::empty())?; - bail!("failed to spawn: {e}"); + error!("failed to spawn: {e}"); } set_status( @@ -220,9 +220,6 @@ fn run_service() -> Result<()> { spawner.wait(); } - // ensure we kill the daemon after - spawner.stop(); - set_status(ServiceState::Stopped, ServiceControlAccept::empty())?; Ok(()) @@ -256,11 +253,7 @@ fn set_privilege(name: PCWSTR, state: bool) -> Result<()> { SE_PRIVILEGE_REMOVED }; - if state { - tp.Privileges[0].Attributes = attributes; - } else { - tp.Privileges[0].Attributes = attributes; - } + tp.Privileges[0].Attributes = attributes; unsafe { AdjustTokenPrivileges(token.0, false, Some(&tp), 0, None, None)?; @@ -284,23 +277,18 @@ fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Resu WTSQueryUserToken(session_id, &mut query_token.0)?; } - let sa = SECURITY_ATTRIBUTES { - bInheritHandle: true.into(), - ..Default::default() - }; - let mut token = OwnedHandle::default(); unsafe { DuplicateTokenEx( query_token.0, TOKEN_ACCESS_MASK(MAXIMUM_ALLOWED), - Some(&sa), + None, SecurityIdentification, TokenPrimary, &mut token.0, )?; - }; + } let t = cb(token)?; @@ -347,7 +335,10 @@ impl Child { fn kill(&mut self) -> Result<()> { if self.0.is_valid() { - unsafe { TerminateProcess(self.0 .0, 0)? }; + unsafe { + TerminateProcess(self.0 .0, 0)?; + } + self.0 = OwnedHandle::default(); } @@ -389,6 +380,7 @@ impl Drop for EnvBlock { struct Spawner { running: Arc, child: Arc>, + main: Arc, } impl Spawner { @@ -396,6 +388,7 @@ impl Spawner { Self { running: Arc::default(), child: Arc::default(), + main: Arc::new(thread::current()), } } @@ -411,7 +404,7 @@ impl Spawner { } fn wait(&self) { - let mut handle = { self.child.lock().unwrap().0 .0 }; + let mut handle = self.child.lock().unwrap().0 .0; if handle.is_invalid() { while !self.running.load(Ordering::Relaxed) { @@ -433,7 +426,7 @@ impl Spawner { let running = self.running.clone(); let child = self.child.clone(); - let curr_thread = thread::current(); + let main_thread = self.main.clone(); _ = thread::spawn(move || { let res = run_as(session, move |token| { let mut arguments = Vec::new(); @@ -489,7 +482,7 @@ impl Spawner { let mut lock = child.lock().unwrap(); *lock = Child(process_info.hProcess.into()); running.store(true, Ordering::Relaxed); - curr_thread.unpark(); + main_thread.unpark(); } unsafe { From c547af5a9e6d541e5487e510f2628407ef8a3dc0 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 06:15:15 -0700 Subject: [PATCH 03/36] Add unpark() after kill() in stop(), and make sure shutdown condition always gets seen --- pueue/src/daemon/service.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 1d992755..c4cf26de 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -144,8 +144,8 @@ fn run_service() -> Result<()> { move |control_event| -> ServiceControlHandlerResult { match control_event { ServiceControl::Stop => { - spawner.stop(); shutdown.store(true, Ordering::Relaxed); + spawner.stop(); ServiceControlHandlerResult::NoError } @@ -396,10 +396,17 @@ impl Spawner { self.running.load(Ordering::Relaxed) } + // note: if you need any `while` loop to exit by checking condition, + // make _sure_ you put this stop() _after_ you change the `while` condition to false + // otherwise it will not be observable fn stop(&self) { let mut child = self.child.lock().unwrap(); if child.kill().is_ok() { self.running.store(false, Ordering::Relaxed); + // even if thread got stuck in park(), this ensures it will test the + // `while` condition at least once more. as long as `while` conditions have + // been changed _before_ the call to stop(), it will exit the wait() + self.main.unpark(); } } From cbd1d8561ef9d53690267797719140cfd168baff Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 06:29:08 -0700 Subject: [PATCH 04/36] Sort deps --- pueue/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pueue/Cargo.toml b/pueue/Cargo.toml index 985baa55..2b3ee33a 100644 --- a/pueue/Cargo.toml +++ b/pueue/Cargo.toml @@ -26,6 +26,7 @@ ctrlc = { version = "3", features = ["termination"] } handlebars = { workspace = true } interim = { version = "0.1.2", features = ["chrono"] } log = { workspace = true } +once_cell = "1.19.0" pest = "2.7" pest_derive = "2.7" pueue-lib = { version = "0.26.1", path = "../pueue_lib" } @@ -39,7 +40,6 @@ strum = { workspace = true } strum_macros = { workspace = true } tempfile = "3" tokio = { workspace = true } -once_cell = "1.19.0" [dev-dependencies] anyhow = { workspace = true } From 0efaaf88e86cf9c6a57eeabede4a8bf8187bc72b Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 06:38:39 -0700 Subject: [PATCH 05/36] Only logon/logoff should be important --- pueue/src/daemon/service.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index c4cf26de..3414b35a 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -152,9 +152,7 @@ fn run_service() -> Result<()> { ServiceControl::SessionChange(param) => { match param.reason { - SessionChangeReason::SessionLogon - | SessionChangeReason::RemoteConnect - | SessionChangeReason::SessionUnlock => { + SessionChangeReason::SessionLogon => { if !spawner.running() { if let Err(e) = spawner.start(Some(param.notification.session_id)) { error!("failed to spawn: {e}"); From 26dc46d917a17561cfadd8e3320ff033561aa68c Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 06:46:06 -0700 Subject: [PATCH 06/36] Separate logon/logoff into separate arms --- pueue/src/daemon/service.rs | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 3414b35a..97c7663f 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -41,7 +41,7 @@ use windows_service::{ service::{ ServiceAccess, ServiceControl, ServiceControlAccept, ServiceErrorControl, ServiceExitCode, ServiceInfo, ServiceStartType, ServiceState, ServiceStatus, ServiceType, - SessionChangeReason, + SessionChangeParam, SessionChangeReason, SessionNotification, }, service_control_handler::{self, ServiceControlHandlerResult}, service_dispatcher, @@ -150,22 +150,26 @@ fn run_service() -> Result<()> { ServiceControlHandlerResult::NoError } - ServiceControl::SessionChange(param) => { - match param.reason { - SessionChangeReason::SessionLogon => { - if !spawner.running() { - if let Err(e) = spawner.start(Some(param.notification.session_id)) { - error!("failed to spawn: {e}"); - } - } + // Logon + ServiceControl::SessionChange(SessionChangeParam { + reason: SessionChangeReason::SessionLogon, + notification: SessionNotification { session_id, .. }, + }) => { + if !spawner.running() { + if let Err(e) = spawner.start(Some(session_id)) { + error!("failed to spawn: {e}"); } + } - SessionChangeReason::SessionLogoff => { - spawner.stop(); - } + ServiceControlHandlerResult::NoError + } - _ => (), - } + // Logoff + ServiceControl::SessionChange(SessionChangeParam { + reason: SessionChangeReason::SessionLogoff, + .. + }) => { + spawner.stop(); ServiceControlHandlerResult::NoError } @@ -412,7 +416,7 @@ impl Spawner { let mut handle = self.child.lock().unwrap().0 .0; if handle.is_invalid() { - while !self.running.load(Ordering::Relaxed) { + while !self.running() { thread::park(); } From 5c3ca8ef2ca94d933131d69107d224c049b451e5 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 08:11:16 -0700 Subject: [PATCH 07/36] Stop service if process unexpectedly exits --- pueue/src/daemon/service.rs | 39 ++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 97c7663f..878412b3 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -218,7 +218,7 @@ fn run_service() -> Result<()> { ServiceControlAccept::STOP | ServiceControlAccept::SESSION_CHANGE, )?; - while !shutdown.load(Ordering::Relaxed) { + while !shutdown.load(Ordering::Relaxed) && !spawner.dirty() { spawner.wait(); } @@ -383,6 +383,9 @@ struct Spawner { running: Arc, child: Arc>, main: Arc, + // whether the process has exited without our request + dirty: Arc, + request_stop: Arc, } impl Spawner { @@ -391,6 +394,8 @@ impl Spawner { running: Arc::default(), child: Arc::default(), main: Arc::new(thread::current()), + dirty: Arc::default(), + request_stop: Arc::default(), } } @@ -403,12 +408,20 @@ impl Spawner { // otherwise it will not be observable fn stop(&self) { let mut child = self.child.lock().unwrap(); - if child.kill().is_ok() { - self.running.store(false, Ordering::Relaxed); - // even if thread got stuck in park(), this ensures it will test the - // `while` condition at least once more. as long as `while` conditions have - // been changed _before_ the call to stop(), it will exit the wait() - self.main.unpark(); + self.request_stop.store(true, Ordering::Relaxed); + match child.kill() { + Ok(_) => { + self.running.store(false, Ordering::Relaxed); + // even if thread got stuck in park(), this ensures it will test the + // `while` condition at least once more. as long as `while` conditions have + // been changed _before_ the call to stop(), it will exit the wait() + self.main.unpark(); + } + + Err(e) => { + self.request_stop.store(false, Ordering::Relaxed); + error!("failed to stop(): {e}"); + } } } @@ -428,6 +441,11 @@ impl Spawner { } } + /// did the spawned process quit without our request? + fn dirty(&self) -> bool { + self.dirty.load(Ordering::Relaxed) + } + fn start(&self, session: Option) -> Result<()> { let Some(session) = session.or_else(|| get_current_session()) else { bail!("get_current_session failed"); @@ -436,6 +454,8 @@ impl Spawner { let running = self.running.clone(); let child = self.child.clone(); let main_thread = self.main.clone(); + let dirty = self.dirty.clone(); + let request_stop = self.request_stop.clone(); _ = thread::spawn(move || { let res = run_as(session, move |token| { let mut arguments = Vec::new(); @@ -504,6 +524,11 @@ impl Spawner { running.store(false, Ordering::Relaxed); } + // check if process exited on its own without our request + if !request_stop.swap(false, Ordering::Relaxed) { + dirty.store(true, Ordering::Relaxed); + } + Ok(()) }); From c3a961c92a4f0aa94472c15e7723a0f39bbf792f Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 08:24:21 -0700 Subject: [PATCH 08/36] Fix clippy suggestions --- pueue/src/daemon/service.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 878412b3..70ead529 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -297,6 +297,7 @@ fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Resu Ok(t) } +#[derive(Default)] struct OwnedHandle(HANDLE); unsafe impl Send for OwnedHandle {} @@ -308,12 +309,6 @@ impl OwnedHandle { } } -impl Default for OwnedHandle { - fn default() -> Self { - Self(HANDLE::default()) - } -} - impl From for OwnedHandle { fn from(value: HANDLE) -> Self { Self(value) @@ -447,7 +442,7 @@ impl Spawner { } fn start(&self, session: Option) -> Result<()> { - let Some(session) = session.or_else(|| get_current_session()) else { + let Some(session) = session.or_else(get_current_session) else { bail!("get_current_session failed"); }; From 7402a7e0347d2acbab3cb15fbbc9a58cf3f0559f Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 08:32:49 -0700 Subject: [PATCH 09/36] Added changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e502c0e9..021f4c28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ TLDR: The new task state representation is more verbose but significantly cleane - Ability to set the Unix socket permissions through the new `unix_socket_permissions` configuration option. [#544](https://github.com/Nukesor/pueue/pull/544) - Add `command` filter to `pueue status`. [#524](https://github.com/Nukesor/pueue/issues/524) [#560](https://github.com/Nukesor/pueue/pull/560) - Allow `pueue status` to order tasks by `enqueue_at`. [#554](https://github.com/Nukesor/pueue/issues/554) +- Added Windows service on Windows to allow a true daemon experience. [#344](https://github.com/Nukesor/pueue/issues/344) [#567](https://github.com/Nukesor/pueue/pull/567) ### Fixed From bd7762d098fa48ee9061765d20be7a886dc2f833 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 11:36:18 -0700 Subject: [PATCH 10/36] Quote profile and config args --- pueue/src/daemon/service.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 70ead529..5aa3c5fa 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -81,11 +81,11 @@ pub fn install_service(config_path: Option, profile: Option) -> let mut args = vec!["--service".into()]; if let Some(config_path) = config_path { args.push("--config".into()); - args.push(config_path.into_os_string()); + args.push(format!(r#""{}""#, config_path.to_string_lossy()).into()); } if let Some(profile) = profile { args.push("--profile".into()); - args.push(profile.into()); + args.push(format!(r#""{profile}""#).into()); } let service_info = ServiceInfo { @@ -414,7 +414,6 @@ impl Spawner { } Err(e) => { - self.request_stop.store(false, Ordering::Relaxed); error!("failed to stop(): {e}"); } } From a11498662c94f0063529dee5867b2a34842a6289 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 13:21:00 -0700 Subject: [PATCH 11/36] Fix remaining len calculation for number of elems in vec --- pueue/src/daemon/service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 5aa3c5fa..576a2d29 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -480,7 +480,7 @@ impl Spawner { .chain(iter::once(0)) .collect::>(); - command.reserve(1024 - command.len()); + command.reserve(1024 - (command.len() / 2)); let env_block = EnvBlock::new(token.0)?; From aceef4851dcca2c02f38acc05224404e6dbbabbc Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 2 Sep 2024 18:14:21 -0700 Subject: [PATCH 12/36] Now works with logoff/logon events --- pueue/src/daemon/service.rs | 64 ++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 576a2d29..b12e5e63 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -13,7 +13,7 @@ use std::{ }; use anyhow::{anyhow, bail, Result}; -use log::error; +use log::{debug, error}; use once_cell::sync::OnceCell; use windows::{ core::{PCWSTR, PWSTR}, @@ -29,9 +29,10 @@ use windows::{ RemoteDesktop::{WTSGetActiveConsoleSessionId, WTSQueryUserToken}, SystemServices::MAXIMUM_ALLOWED, Threading::{ - CreateProcessAsUserW, OpenProcess, OpenProcessToken, TerminateProcess, - WaitForSingleObject, CREATE_NO_WINDOW, CREATE_UNICODE_ENVIRONMENT, INFINITE, - PROCESS_INFORMATION, PROCESS_QUERY_INFORMATION, STARTUPINFOW, + CreateProcessAsUserW, GetExitCodeProcess, OpenProcess, OpenProcessToken, + TerminateProcess, WaitForSingleObject, CREATE_NO_WINDOW, + CREATE_UNICODE_ENVIRONMENT, INFINITE, PROCESS_INFORMATION, + PROCESS_QUERY_INFORMATION, STARTUPINFOW, }, }, }, @@ -144,6 +145,7 @@ fn run_service() -> Result<()> { move |control_event| -> ServiceControlHandlerResult { match control_event { ServiceControl::Stop => { + debug!("event stop"); shutdown.store(true, Ordering::Relaxed); spawner.stop(); @@ -153,10 +155,16 @@ fn run_service() -> Result<()> { // Logon ServiceControl::SessionChange(SessionChangeParam { reason: SessionChangeReason::SessionLogon, - notification: SessionNotification { session_id, .. }, + notification: + SessionNotification { + session_id: session, + .. + }, }) => { + debug!("event login"); if !spawner.running() { - if let Err(e) = spawner.start(Some(session_id)) { + debug!("event login: spawning"); + if let Err(e) = spawner.start(Some(session)) { error!("failed to spawn: {e}"); } } @@ -169,11 +177,14 @@ fn run_service() -> Result<()> { reason: SessionChangeReason::SessionLogoff, .. }) => { + debug!("event logoff"); spawner.stop(); ServiceControlHandlerResult::NoError } + ServiceControl::SessionChange(_) => ServiceControlHandlerResult::NoError, + // All services must accept Interrogate even if it's a no-op. ServiceControl::Interrogate => ServiceControlHandlerResult::NoError, @@ -219,9 +230,12 @@ fn run_service() -> Result<()> { )?; while !shutdown.load(Ordering::Relaxed) && !spawner.dirty() { + debug!("spawner wait()"); spawner.wait(); } + debug!("shutting down service"); + set_status(ServiceState::Stopped, ServiceControlAccept::empty())?; Ok(()) @@ -341,6 +355,10 @@ impl Child { Ok(()) } + + fn reset(&mut self) { + self.0 = OwnedHandle::default(); + } } impl Default for Child { @@ -403,9 +421,11 @@ impl Spawner { // otherwise it will not be observable fn stop(&self) { let mut child = self.child.lock().unwrap(); + self.request_stop.store(true, Ordering::Relaxed); match child.kill() { Ok(_) => { + debug!("stop() kill"); self.running.store(false, Ordering::Relaxed); // even if thread got stuck in park(), this ensures it will test the // `while` condition at least once more. as long as `while` conditions have @@ -414,6 +434,7 @@ impl Spawner { } Err(e) => { + self.running.store(false, Ordering::Relaxed); error!("failed to stop(): {e}"); } } @@ -423,13 +444,19 @@ impl Spawner { let mut handle = self.child.lock().unwrap().0 .0; if handle.is_invalid() { + debug!("wait() invalid handle"); while !self.running() { + debug!("wait() parking"); thread::park(); } + debug!("wait() unparking"); + handle = self.child.lock().unwrap().0 .0; } + debug!("wait() waitingforsingleobject"); + unsafe { WaitForSingleObject(handle, INFINITE); } @@ -451,6 +478,8 @@ impl Spawner { let dirty = self.dirty.clone(); let request_stop = self.request_stop.clone(); _ = thread::spawn(move || { + request_stop.store(false, Ordering::Relaxed); + let res = run_as(session, move |token| { let mut arguments = Vec::new(); @@ -512,17 +541,28 @@ impl Spawner { WaitForSingleObject(process_info.hProcess, INFINITE); } - { - let mut lock = child.lock().unwrap(); - _ = lock.kill(); - running.store(false, Ordering::Relaxed); - } + running.store(false, Ordering::Relaxed); // check if process exited on its own without our request if !request_stop.swap(false, Ordering::Relaxed) { - dirty.store(true, Ordering::Relaxed); + let mut code = 0u32; + unsafe { + GetExitCodeProcess(process_info.hProcess, &mut code)?; + } + + debug!("spawner code {code}"); + + // only stop in the case of an abnormal shutdown + const LOGOFF: u32 = 0x40010004; + if code != 0 && code != LOGOFF { + debug!("service storing dirty true"); + dirty.store(true, Ordering::Relaxed); + main_thread.unpark(); + } } + child.lock().unwrap().reset(); + Ok(()) }); From b594ffa913a39722aad12e09241d3415c79d7133 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Tue, 3 Sep 2024 07:56:59 -0700 Subject: [PATCH 13/36] Move service management to subcommands + add start and stop command --- pueue/src/bin/pueued.rs | 61 ++++++++++++++++++++++++++----------- pueue/src/daemon/cli.rs | 42 ++++++++++++++++--------- pueue/src/daemon/service.rs | 61 ++++++++++++++++++++++++++++++++----- 3 files changed, 124 insertions(+), 40 deletions(-) diff --git a/pueue/src/bin/pueued.rs b/pueue/src/bin/pueued.rs index 8c3f0408..c7448e6e 100644 --- a/pueue/src/bin/pueued.rs +++ b/pueue/src/bin/pueued.rs @@ -5,8 +5,6 @@ use clap::Parser; use log::warn; use simplelog::{Config, ConfigBuilder, LevelFilter, SimpleLogger, TermLogger, TerminalMode}; -#[cfg(target_os = "windows")] -use pueue::daemon::service; use pueue::daemon::{cli::CliArguments, run}; #[tokio::main(flavor = "multi_thread", worker_threads = 4)] @@ -15,6 +13,16 @@ async fn main() -> Result<()> { let opt = CliArguments::parse(); if opt.daemonize { + // Ordinarily this would be handled in clap, but they don't support conflicting args + // with subcommands + #[cfg(target_os = "windows")] + if opt.service.is_some() { + use clap::CommandFactory; + let mut cmd = CliArguments::command(); + cmd.print_help()?; + return Ok(()); + } + return fork_daemon(&opt); } @@ -50,22 +58,39 @@ async fn main() -> Result<()> { #[cfg(target_os = "windows")] { - if opt.service { - // start service - service::start_service(opt.config.clone(), opt.profile.clone())?; - return Ok(()); - } - - if opt.install { - service::install_service(opt.config.clone(), opt.profile.clone())?; - println!("Successfully installed `pueued` Windows service"); - return Ok(()); - } - - if opt.uninstall { - service::uninstall_service()?; - println!("Successfully uninstalled `pueued` Windows service"); - return Ok(()); + use pueue::daemon::cli::{ServiceSubcommand, ServiceSubcommandEntry}; + use pueue::daemon::service; + + if let Some(ServiceSubcommandEntry::Service(service)) = opt.service { + match service { + ServiceSubcommand::Run => { + // start service + service::run_service(opt.config.clone(), opt.profile.clone())?; + return Ok(()); + } + + ServiceSubcommand::Install => { + service::install_service(opt.config.clone(), opt.profile.clone())?; + println!("Successfully installed `pueued` Windows service"); + return Ok(()); + } + + ServiceSubcommand::Uninstall => { + service::uninstall_service()?; + println!("Successfully uninstalled `pueued` Windows service"); + return Ok(()); + } + + ServiceSubcommand::Start => { + service::start_service()?; + return Ok(()); + } + + ServiceSubcommand::Stop => { + service::stop_service()?; + return Ok(()); + } + } } } diff --git a/pueue/src/daemon/cli.rs b/pueue/src/daemon/cli.rs index 94d97360..bd1c7b66 100644 --- a/pueue/src/daemon/cli.rs +++ b/pueue/src/daemon/cli.rs @@ -1,5 +1,7 @@ use std::path::PathBuf; +#[cfg(target_os = "windows")] +use clap::Subcommand; use clap::{ArgAction, Parser, ValueHint}; #[derive(Parser, Debug)] @@ -25,22 +27,34 @@ pub struct CliArguments { #[arg(short, long)] pub profile: Option, + #[cfg(target_os = "windows")] + #[command(subcommand)] + pub service: Option, +} + +#[cfg(target_os = "windows")] +#[derive(Copy, Clone, Debug, Subcommand)] +pub enum ServiceSubcommandEntry { + /// Manage the Windows Service. + #[command(subcommand)] + Service(ServiceSubcommand), +} + +#[cfg(target_os = "windows")] +#[derive(Copy, Clone, Debug, Subcommand)] +pub enum ServiceSubcommand { + /// Run the Windows service. This command is internal and should never + /// be used. + Run, /// Install as a Windows service. /// Once installed, you must not move the binary, otherwise the Windows /// service will not be able to find it. If you wish to move the binary, /// first uninstall the service, then install the service again. - #[cfg(target_os = "windows")] - #[arg(long, conflicts_with_all = ["daemonize", "uninstall"])] - pub install: bool, - - /// Uninstall the Windows service. - #[cfg(target_os = "windows")] - #[arg(long, conflicts_with_all = ["daemonize", "install"])] - pub uninstall: bool, - - /// Start the Windows service. This command is internal and should never - /// be used. As a user, to manage the service, use the Windows Service Manager. - #[cfg(target_os = "windows")] - #[arg(long, conflicts_with_all = ["daemonize", "install", "uninstall"])] - pub service: bool, + Install, + /// Uninstall the service. + Uninstall, + /// Start the service. + Start, + /// Stop the service. + Stop, } diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index b12e5e63..1a83f723 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -61,7 +61,7 @@ static CONFIG: OnceCell = OnceCell::new(); define_windows_service!(ffi_service_main, service_main); -pub fn start_service(config_path: Option, profile: Option) -> Result<()> { +pub fn run_service(config_path: Option, profile: Option) -> Result<()> { CONFIG .set(Config { config_path, @@ -79,16 +79,19 @@ pub fn install_service(config_path: Option, profile: Option) -> let service_binary_path = std::env::current_exe()?; - let mut args = vec!["--service".into()]; + let mut args = vec![]; if let Some(config_path) = config_path { - args.push("--config".into()); - args.push(format!(r#""{}""#, config_path.to_string_lossy()).into()); + args.extend([ + "--config".into(), + format!(r#""{}""#, config_path.to_string_lossy()).into(), + ]); } if let Some(profile) = profile { - args.push("--profile".into()); - args.push(format!(r#""{profile}""#).into()); + args.extend(["--profile".into(), format!(r#""{profile}""#).into()]); } + args.extend(["service".into(), "run".into()]); + let service_info = ServiceInfo { name: SERVICE_NAME.into(), display_name: SERVICE_NAME.into(), @@ -128,13 +131,55 @@ pub fn uninstall_service() -> Result<()> { Ok(()) } +pub fn start_service() -> Result<()> { + let manager_access = ServiceManagerAccess::CONNECT; + let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)?; + + let service_access = ServiceAccess::QUERY_STATUS | ServiceAccess::START; + let service = service_manager.open_service(SERVICE_NAME, service_access)?; + + match service.query_status()?.current_state { + ServiceState::Stopped => { + service.start::(&[])?; + println!("Successfully started service"); + } + ServiceState::StartPending => println!("Service is already starting"), + ServiceState::Running => println!("Service is already running"), + + _ => (), + } + + Ok(()) +} + +pub fn stop_service() -> Result<()> { + let manager_access = ServiceManagerAccess::CONNECT; + let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)?; + + let service_access = ServiceAccess::QUERY_STATUS | ServiceAccess::STOP; + let service = service_manager.open_service(SERVICE_NAME, service_access)?; + + match service.query_status()?.current_state { + ServiceState::Stopped => println!("Successfully is already stopped"), + ServiceState::StartPending => println!("Service cannot stop because it is starting (please wait until it fully started to stop it)"), + ServiceState::Running => { + service.stop()?; + println!("Successfully stopped service"); + } + + _ => (), + } + + Ok(()) +} + fn service_main(_: Vec) { - if let Err(e) = run_service() { + if let Err(e) = service_event_loop() { error!("Failed to start service: {e}"); } } -fn run_service() -> Result<()> { +fn service_event_loop() -> Result<()> { let spawner = Arc::new(Spawner::new()); let shutdown = Arc::new(AtomicBool::default()); From ebdcc0744d0a5c1910764f671ce6ace7e4a25bac Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Tue, 3 Sep 2024 08:19:06 -0700 Subject: [PATCH 14/36] Service return error if failed to start service --- pueue/src/daemon/service.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 1a83f723..6af3abdc 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -211,6 +211,7 @@ fn service_event_loop() -> Result<()> { debug!("event login: spawning"); if let Err(e) = spawner.start(Some(session)) { error!("failed to spawn: {e}"); + return ServiceControlHandlerResult::Other(1); } } From 829e4b2e6fce6cff5397725cb3f342f4891bf752 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Tue, 3 Sep 2024 18:00:27 -0700 Subject: [PATCH 15/36] Simplify code by using channels instead of WaitForSingleObject in waiter --- pueue/src/daemon/service.rs | 40 +++++++++++++------------------------ 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 6af3abdc..e26ae3ea 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -6,9 +6,10 @@ use std::{ process, ptr, sync::{ atomic::{AtomicBool, Ordering}, + mpsc::{channel, Receiver, Sender}, Arc, Mutex, }, - thread::{self, Thread}, + thread, time::Duration, }; @@ -441,20 +442,25 @@ impl Drop for EnvBlock { struct Spawner { running: Arc, child: Arc>, - main: Arc, // whether the process has exited without our request dirty: Arc, request_stop: Arc, + park_tx: Arc>, + // we don't need mutation, but we do need Sync + park_rx: Mutex>, } impl Spawner { fn new() -> Self { + let (park_tx, park_rx) = channel(); + Self { running: Arc::default(), child: Arc::default(), - main: Arc::new(thread::current()), dirty: Arc::default(), request_stop: Arc::default(), + park_tx: Arc::new(park_tx), + park_rx: Mutex::new(park_rx), } } @@ -476,7 +482,7 @@ impl Spawner { // even if thread got stuck in park(), this ensures it will test the // `while` condition at least once more. as long as `while` conditions have // been changed _before_ the call to stop(), it will exit the wait() - self.main.unpark(); + _ = self.park_tx.send(()); } Err(e) => { @@ -487,25 +493,7 @@ impl Spawner { } fn wait(&self) { - let mut handle = self.child.lock().unwrap().0 .0; - - if handle.is_invalid() { - debug!("wait() invalid handle"); - while !self.running() { - debug!("wait() parking"); - thread::park(); - } - - debug!("wait() unparking"); - - handle = self.child.lock().unwrap().0 .0; - } - - debug!("wait() waitingforsingleobject"); - - unsafe { - WaitForSingleObject(handle, INFINITE); - } + _ = self.park_rx.lock().unwrap().recv(); } /// did the spawned process quit without our request? @@ -520,7 +508,7 @@ impl Spawner { let running = self.running.clone(); let child = self.child.clone(); - let main_thread = self.main.clone(); + let parker = self.park_tx.clone(); let dirty = self.dirty.clone(); let request_stop = self.request_stop.clone(); _ = thread::spawn(move || { @@ -580,7 +568,7 @@ impl Spawner { let mut lock = child.lock().unwrap(); *lock = Child(process_info.hProcess.into()); running.store(true, Ordering::Relaxed); - main_thread.unpark(); + _ = parker.send(()); } unsafe { @@ -603,7 +591,7 @@ impl Spawner { if code != 0 && code != LOGOFF { debug!("service storing dirty true"); dirty.store(true, Ordering::Relaxed); - main_thread.unpark(); + _ = parker.send(()); } } From 0a31056abad24fbd3be146b075f58a738761a186 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Tue, 3 Sep 2024 18:06:07 -0700 Subject: [PATCH 16/36] Unparking is no longer needed here thanks to channels --- pueue/src/daemon/service.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index e26ae3ea..fa6d41e4 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -568,7 +568,6 @@ impl Spawner { let mut lock = child.lock().unwrap(); *lock = Child(process_info.hProcess.into()); running.store(true, Ordering::Relaxed); - _ = parker.send(()); } unsafe { From e34c02250eb810c76cf67d7f9211f48d99addce0 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Tue, 3 Sep 2024 20:32:13 -0700 Subject: [PATCH 17/36] Sender does not need to be Arc --- pueue/src/daemon/service.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index fa6d41e4..d027cd18 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -445,7 +445,7 @@ struct Spawner { // whether the process has exited without our request dirty: Arc, request_stop: Arc, - park_tx: Arc>, + park_tx: Sender<()>, // we don't need mutation, but we do need Sync park_rx: Mutex>, } @@ -459,7 +459,7 @@ impl Spawner { child: Arc::default(), dirty: Arc::default(), request_stop: Arc::default(), - park_tx: Arc::new(park_tx), + park_tx, park_rx: Mutex::new(park_rx), } } From b0f3743ae043fc319d2b396af34e5290f716e90e Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Tue, 3 Sep 2024 23:09:03 -0700 Subject: [PATCH 18/36] Properly detach daemon on windows in `-d` mode --- pueue/src/bin/pueued.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pueue/src/bin/pueued.rs b/pueue/src/bin/pueued.rs index c7448e6e..e7a199ec 100644 --- a/pueue/src/bin/pueued.rs +++ b/pueue/src/bin/pueued.rs @@ -125,7 +125,17 @@ fn fork_daemon(opt: &CliArguments) -> Result<()> { "pueued".to_string() }; - Command::new(current_exe) + let mut command = Command::new(current_exe); + + #[cfg(target_os = "windows")] + { + use std::os::windows::process::CommandExt; + const CREATE_NO_WINDOW: u32 = 0x08000000; + + command.creation_flags(CREATE_NO_WINDOW); + } + + command .args(&arguments) .spawn() .expect("Failed to fork new process."); From ef464cdf0480cf39b42b78b524389138a5502c65 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 4 Sep 2024 14:15:42 -0700 Subject: [PATCH 19/36] Add -d fix to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 021f4c28..d62cff40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ TLDR: The new task state representation is more verbose but significantly cleane - Fixed delay after sending process related commands from client. [#548](https://github.com/Nukesor/pueue/pull/548) - Callback templating arguments were html escaped by accident. [#564](https://github.com/Nukesor/pueue/pull/564) - Print incompatible version warning info as a log message instead of plain stdout input, which broke json outputs [#562](https://github.com/Nukesor/pueue/issues/562). +- Fixed `-d` daemon mode on Windows. [#344](https://github.com/Nukesor/pueue/issues/344) ## \[3.4.1\] - 2024-06-04 From 7f234e99353b4085d5e7f3b987fd58253b152c21 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 4 Sep 2024 15:07:01 -0700 Subject: [PATCH 20/36] Add more comments --- pueue/src/bin/pueued.rs | 5 ++- pueue/src/daemon/service.rs | 71 ++++++++++++++++++++++++++----------- 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/pueue/src/bin/pueued.rs b/pueue/src/bin/pueued.rs index e7a199ec..67552569 100644 --- a/pueue/src/bin/pueued.rs +++ b/pueue/src/bin/pueued.rs @@ -131,7 +131,10 @@ fn fork_daemon(opt: &CliArguments) -> Result<()> { { use std::os::windows::process::CommandExt; const CREATE_NO_WINDOW: u32 = 0x08000000; - + // create_no_window causes all children to not show a visible console window + // but it also apparently has the effect of DETACH_PROCESS in that it's no longer tied to + // the parent process + // https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags#flags command.creation_flags(CREATE_NO_WINDOW); } diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index d027cd18..948c8b04 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -121,6 +121,7 @@ pub fn uninstall_service() -> Result<()> { // The service will be marked for deletion as long as this function call succeeds. // However, it will not be deleted from the database until it is stopped and all open handles to it are closed. + // If the service manager window is open, it will need to be closed before the service gets deleted. service.delete()?; // Our handle to it is not closed yet. So we can still query it. @@ -182,6 +183,7 @@ fn service_main(_: Vec) { fn service_event_loop() -> Result<()> { let spawner = Arc::new(Spawner::new()); + // a shutdown of the service was requested let shutdown = Arc::new(AtomicBool::default()); let event_handler = { @@ -190,8 +192,11 @@ fn service_event_loop() -> Result<()> { move |control_event| -> ServiceControlHandlerResult { match control_event { + // Stop ServiceControl::Stop => { debug!("event stop"); + // important, set the while loop's exit condition before calling stop(), otherwise + // the condition will not be observed. shutdown.store(true, Ordering::Relaxed); spawner.stop(); @@ -230,6 +235,7 @@ fn service_event_loop() -> Result<()> { ServiceControlHandlerResult::NoError } + // Other session change events we don't care about ServiceControl::SessionChange(_) => ServiceControlHandlerResult::NoError, // All services must accept Interrogate even if it's a no-op. @@ -259,7 +265,7 @@ fn service_event_loop() -> Result<()> { set_status(ServiceState::StartPending, ServiceControlAccept::empty())?; - // make sure we have privileges + // make sure we have privileges - this should always succeed if let Err(e) = set_privilege(SE_TCB_NAME, true) { set_status(ServiceState::Stopped, ServiceControlAccept::empty())?; bail!("failed to set privileges: {e}"); @@ -267,6 +273,7 @@ fn service_event_loop() -> Result<()> { // if it fails here, we probably launched before user logged in? // for that reason we only log, but do not bail and stop the service + // the event handler will start it when the user logs in if let Err(e) = spawner.start(None) { error!("failed to spawn: {e}"); } @@ -276,6 +283,8 @@ fn service_event_loop() -> Result<()> { ServiceControlAccept::STOP | ServiceControlAccept::SESSION_CHANGE, )?; + // while there's no shutdown request, and the spawner didn't exit unexpectedly, + // keep the service running while !shutdown.load(Ordering::Relaxed) && !spawner.dirty() { debug!("spawner wait()"); spawner.wait(); @@ -288,6 +297,8 @@ fn service_event_loop() -> Result<()> { Ok(()) } +/// set the specified process privilege to state +/// https://learn.microsoft.com/en-us/windows/win32/secauthz/privilege-constants fn set_privilege(name: PCWSTR, state: bool) -> Result<()> { let handle: OwnedHandle = unsafe { OpenProcess(PROCESS_QUERY_INFORMATION, false, process::id())?.into() }; @@ -325,6 +336,7 @@ fn set_privilege(name: PCWSTR, state: bool) -> Result<()> { Ok(()) } +/// get the current user session, only needed when we don't initially have a session id to go by fn get_current_session() -> Option { let session = unsafe { WTSGetActiveConsoleSessionId() }; @@ -334,6 +346,7 @@ fn get_current_session() -> Option { } } +/// run closure and supply the currently logged in user's token fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Result { let mut query_token: OwnedHandle = OwnedHandle::default(); unsafe { @@ -358,6 +371,7 @@ fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Resu Ok(t) } +/// newtype over handle which closes the HANDLE on drop #[derive(Default)] struct OwnedHandle(HANDLE); @@ -384,6 +398,7 @@ impl Drop for OwnedHandle { } } +/// a child process. tries to kill the process when dropped struct Child(OwnedHandle); impl Child { @@ -420,9 +435,12 @@ impl Drop for Child { } } +/// a users environment block +/// https://learn.microsoft.com/en-us/windows/win32/api/userenv/nf-userenv-createenvironmentblock struct EnvBlock(*mut c_void); impl EnvBlock { + /// get the environment block belonging to the supplied users token fn new(token: HANDLE) -> Result { let mut env = ptr::null_mut(); unsafe { @@ -439,38 +457,44 @@ impl Drop for EnvBlock { } } +/// manages the child daemon, by spawning / stopping it, or reporting abnormal exit, and allowing wait() struct Spawner { + // whether a child daemon is running running: Arc, + // holds the actual process of the running child daemon child: Arc>, // whether the process has exited without our request dirty: Arc, + // used to differentiate between requested stop() and if process is dirty ^ request_stop: Arc, - park_tx: Sender<()>, + // used for wait()ing until the child is done + wait_tx: Sender<()>, // we don't need mutation, but we do need Sync - park_rx: Mutex>, + wait_rx: Mutex>, } impl Spawner { fn new() -> Self { - let (park_tx, park_rx) = channel(); + let (wait_tx, wait_rx) = channel(); Self { running: Arc::default(), child: Arc::default(), dirty: Arc::default(), request_stop: Arc::default(), - park_tx, - park_rx: Mutex::new(park_rx), + wait_tx, + wait_rx: Mutex::new(wait_rx), } } + /// is the child daemon running? fn running(&self) -> bool { self.running.load(Ordering::Relaxed) } - // note: if you need any `while` loop to exit by checking condition, - // make _sure_ you put this stop() _after_ you change the `while` condition to false - // otherwise it will not be observable + /// note: if you need any `while` loop to exit by checking condition, + /// make _sure_ you put this stop() _after_ you change the `while` condition to false + /// otherwise it will not be observable fn stop(&self) { let mut child = self.child.lock().unwrap(); @@ -479,10 +503,10 @@ impl Spawner { Ok(_) => { debug!("stop() kill"); self.running.store(false, Ordering::Relaxed); - // even if thread got stuck in park(), this ensures it will test the - // `while` condition at least once more. as long as `while` conditions have - // been changed _before_ the call to stop(), it will exit the wait() - _ = self.park_tx.send(()); + // signal the wait() to exit so a `while` condition is checked at least once more. + // as long as `while` conditions have been changed _before_ the call to stop(), + // the changed condition will be observed + _ = self.wait_tx.send(()); } Err(e) => { @@ -492,8 +516,9 @@ impl Spawner { } } + /// wait for child process to exit fn wait(&self) { - _ = self.park_rx.lock().unwrap().recv(); + _ = self.wait_rx.lock().unwrap().recv(); } /// did the spawned process quit without our request? @@ -501,6 +526,7 @@ impl Spawner { self.dirty.load(Ordering::Relaxed) } + /// try to spawn a child daemon fn start(&self, session: Option) -> Result<()> { let Some(session) = session.or_else(get_current_session) else { bail!("get_current_session failed"); @@ -508,7 +534,7 @@ impl Spawner { let running = self.running.clone(); let child = self.child.clone(); - let parker = self.park_tx.clone(); + let waiter = self.wait_tx.clone(); let dirty = self.dirty.clone(); let request_stop = self.request_stop.clone(); _ = thread::spawn(move || { @@ -534,8 +560,7 @@ impl Spawner { let arguments = arguments.join(" "); - // Try to get the path to the current binary, since it may not be in the $PATH. - // If we cannot detect it (for some unknown reason), fallback to the raw `pueued` binary name. + // Try to get the path to the current binary let current_exe = env::current_exe()?.to_string_lossy().to_string(); let mut command = format!(r#""{current_exe}" {arguments}"#) @@ -543,6 +568,8 @@ impl Spawner { .chain(iter::once(0)) .collect::>(); + // lpcommandline may modify the the cmd vec. max valid size is 1024, so + // I think it best to ensure 1024 bytes of capacity total exist just in case command.reserve(1024 - (command.len() / 2)); let env_block = EnvBlock::new(token.0)?; @@ -556,6 +583,8 @@ impl Spawner { None, None, false, + // unicode is required if we pass env block + // create_no_window causes all child processes to not show a visible console window CREATE_UNICODE_ENVIRONMENT | CREATE_NO_WINDOW, Some(env_block.0), None, @@ -570,13 +599,14 @@ impl Spawner { running.store(true, Ordering::Relaxed); } + // wait until the process exits unsafe { WaitForSingleObject(process_info.hProcess, INFINITE); } running.store(false, Ordering::Relaxed); - // check if process exited on its own without our request + // check if process exited on its own without our explicit request if !request_stop.swap(false, Ordering::Relaxed) { let mut code = 0u32; unsafe { @@ -585,12 +615,13 @@ impl Spawner { debug!("spawner code {code}"); - // only stop in the case of an abnormal shutdown + // windows gives this exit code on the process in event of forced process shutdown + // this happens on logoff, so we treat this code as normal const LOGOFF: u32 = 0x40010004; if code != 0 && code != LOGOFF { debug!("service storing dirty true"); dirty.store(true, Ordering::Relaxed); - _ = parker.send(()); + _ = waiter.send(()); } } From f0254f2bceb469f27713fa73c1f6fc5ed4f638f1 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Mon, 9 Sep 2024 06:53:20 -0700 Subject: [PATCH 21/36] Bumped MSRV to 1.70 --- CHANGELOG.md | 1 + Cargo.lock | 1 - Cargo.toml | 2 +- pueue/Cargo.toml | 1 - pueue/src/daemon/service.rs | 5 ++--- 5 files changed, 4 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d62cff40..2d327eeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ TLDR: The new task state representation is more verbose but significantly cleane - **Breaking**: Remove the `--children` commandline flags, that have been deprecated and no longer serve any function since `v3.0.0`. - Send log output to `stderr` instead of `stdout` [#562](https://github.com/Nukesor/pueue/issues/562). - Change default log level from error to warning [#562](https://github.com/Nukesor/pueue/issues/562). +- Bumped MSRV to 1.70. ### Add diff --git a/Cargo.lock b/Cargo.lock index 3153875d..086128d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1288,7 +1288,6 @@ dependencies = [ "handlebars", "interim", "log", - "once_cell", "pest", "pest_derive", "pretty_assertions", diff --git a/Cargo.toml b/Cargo.toml index e316f69b..7de0ea0c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ homepage = "https://github.com/nukesor/pueue" repository = "https://github.com/nukesor/pueue" license = "MIT" edition = "2021" -rust-version = "1.67" +rust-version = "1.70" [workspace.dependencies] # Chrono version is hard pinned to a specific version. diff --git a/pueue/Cargo.toml b/pueue/Cargo.toml index 2b3ee33a..3a4305f7 100644 --- a/pueue/Cargo.toml +++ b/pueue/Cargo.toml @@ -26,7 +26,6 @@ ctrlc = { version = "3", features = ["termination"] } handlebars = { workspace = true } interim = { version = "0.1.2", features = ["chrono"] } log = { workspace = true } -once_cell = "1.19.0" pest = "2.7" pest_derive = "2.7" pueue-lib = { version = "0.26.1", path = "../pueue_lib" } diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 948c8b04..b383a42a 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -7,7 +7,7 @@ use std::{ sync::{ atomic::{AtomicBool, Ordering}, mpsc::{channel, Receiver, Sender}, - Arc, Mutex, + Arc, Mutex, OnceLock, }, thread, time::Duration, @@ -15,7 +15,6 @@ use std::{ use anyhow::{anyhow, bail, Result}; use log::{debug, error}; -use once_cell::sync::OnceCell; use windows::{ core::{PCWSTR, PWSTR}, Win32::{ @@ -58,7 +57,7 @@ struct Config { const SERVICE_NAME: &str = "pueued"; const SERVICE_TYPE: ServiceType = ServiceType::OWN_PROCESS; -static CONFIG: OnceCell = OnceCell::new(); +static CONFIG: OnceLock = OnceLock::new(); define_windows_service!(ffi_service_main, service_main); From 716bfca93ac0cb81b28ec880b9ace8892eed1c3c Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 18 Sep 2024 07:26:38 -0700 Subject: [PATCH 22/36] Add module docs, cleanup comments, and print specific daemon/subcommand conflict message. --- pueue/src/bin/pueued.rs | 9 ++-- pueue/src/daemon/service.rs | 98 ++++++++++++++++++++++++------------- 2 files changed, 68 insertions(+), 39 deletions(-) diff --git a/pueue/src/bin/pueued.rs b/pueue/src/bin/pueued.rs index 67552569..0387532a 100644 --- a/pueue/src/bin/pueued.rs +++ b/pueue/src/bin/pueued.rs @@ -13,13 +13,12 @@ async fn main() -> Result<()> { let opt = CliArguments::parse(); if opt.daemonize { - // Ordinarily this would be handled in clap, but they don't support conflicting args - // with subcommands + // Ordinarily this would be handled in clap, but they don't support conflicting specific args + // with subcommands. We can't turn this off globally because -c and -p are valid args when using + // subcommand to install the service #[cfg(target_os = "windows")] if opt.service.is_some() { - use clap::CommandFactory; - let mut cmd = CliArguments::command(); - cmd.print_help()?; + println!("daemonize flag cannot be used with service subcommand"); return Ok(()); } diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index b383a42a..825fdcc9 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -1,3 +1,33 @@ +//! How Windows services (and this file) work +//! +//! - This service runs as SYSTEM, and survives logoff and logon. +//! - This service launches the daemon as current user on login, and kills it on logoff +//! (actually it's a noop; Windows itself kills it - see below). +//! - All user processes are auto killed by Windows on user logoff. This is not a feature +//! of this service, it's just how Windows does things. +//! - You must install the service. After installed, the service entry maintains a cmdline +//! string with args to the pueued binary. Therefore, the binary must _not_ move while the +//! service is installed, otherwise it will not be able to function properly. It is best +//! not to rely on PATH for this, as it is finicky and a hassle for user setup. Absolute paths +//! are the way to go, and it is standard practice. +//! - To move the pueued binary: Uninstall the service, move the binary, and reinstall the service. +//! - When the service is installed, you can use pueued to start, stop, or uninstall the service. +//! You can also use the official service manager to start, stop, and restart the service. +//! - Services are automatically started/stopped by the system according to the setting the user +//! sets in the windows service manager. By default we install it as autostart, but the user +//! can set this to manual or even disabled. +//! - If you have the official service manager window open and you tell pueued to uninstall the +//! service, it will not disappear from the list until you close all service manager windows. +//! This is Windows specific behavior, and not a bug. (In Windows parlance, the service is pending +//! deletion, and all HANDLES to the service need to be closed). +//! - We do not support long running processes past when a user logs off; this would be +//! a massive security risk to allow anyone to run processes as SYSTEM. This account bypasses +//! even administrator in power! It is not something small to give permission to. +//! - Additionally, taking the above into account, as SYSTEM is its own account, the user config +//! would not apply to this account. You'd have to set up special configs for the SYSTEM account, +//! and I'm not even sure where the SYSTEM account's appdata is stored to begin with. +//! (not to mention, it would be a pain for the user to setup anyways) + use std::{ env, ffi::{c_void, OsString}, @@ -14,7 +44,7 @@ use std::{ }; use anyhow::{anyhow, bail, Result}; -use log::{debug, error}; +use log::{debug, error, info}; use windows::{ core::{PCWSTR, PWSTR}, Win32::{ @@ -161,7 +191,7 @@ pub fn stop_service() -> Result<()> { let service = service_manager.open_service(SERVICE_NAME, service_access)?; match service.query_status()?.current_state { - ServiceState::Stopped => println!("Successfully is already stopped"), + ServiceState::Stopped => println!("Service is already stopped"), ServiceState::StartPending => println!("Service cannot stop because it is starting (please wait until it fully started to stop it)"), ServiceState::Running => { service.stop()?; @@ -194,7 +224,7 @@ fn service_event_loop() -> Result<()> { // Stop ServiceControl::Stop => { debug!("event stop"); - // important, set the while loop's exit condition before calling stop(), otherwise + // Important! Set the while loop's exit condition before calling stop(), otherwise // the condition will not be observed. shutdown.store(true, Ordering::Relaxed); spawner.stop(); @@ -234,7 +264,7 @@ fn service_event_loop() -> Result<()> { ServiceControlHandlerResult::NoError } - // Other session change events we don't care about + // Other session change events we don't care about. ServiceControl::SessionChange(_) => ServiceControlHandlerResult::NoError, // All services must accept Interrogate even if it's a no-op. @@ -270,9 +300,9 @@ fn service_event_loop() -> Result<()> { bail!("failed to set privileges: {e}"); } - // if it fails here, we probably launched before user logged in? - // for that reason we only log, but do not bail and stop the service - // the event handler will start it when the user logs in + // If it fails here, we probably launched before user logged in? + // For that reason we only log, but do not bail and stop the service. + // The event handler will start it when the user logs in. if let Err(e) = spawner.start(None) { error!("failed to spawn: {e}"); } @@ -282,14 +312,14 @@ fn service_event_loop() -> Result<()> { ServiceControlAccept::STOP | ServiceControlAccept::SESSION_CHANGE, )?; - // while there's no shutdown request, and the spawner didn't exit unexpectedly, - // keep the service running + // While there's no shutdown request, and the spawner didn't exit unexpectedly, + // keep the service running. while !shutdown.load(Ordering::Relaxed) && !spawner.dirty() { debug!("spawner wait()"); spawner.wait(); } - debug!("shutting down service"); + info!("shutting down service"); set_status(ServiceState::Stopped, ServiceControlAccept::empty())?; @@ -335,7 +365,7 @@ fn set_privilege(name: PCWSTR, state: bool) -> Result<()> { Ok(()) } -/// get the current user session, only needed when we don't initially have a session id to go by +/// Get the current user session. Only needed when we don't initially have a session id to go by. fn get_current_session() -> Option { let session = unsafe { WTSGetActiveConsoleSessionId() }; @@ -345,7 +375,7 @@ fn get_current_session() -> Option { } } -/// run closure and supply the currently logged in user's token +/// Run closure and supply the currently logged in user's token. fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Result { let mut query_token: OwnedHandle = OwnedHandle::default(); unsafe { @@ -370,7 +400,7 @@ fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Resu Ok(t) } -/// newtype over handle which closes the HANDLE on drop +/// Newtype over handle which closes the HANDLE on drop. #[derive(Default)] struct OwnedHandle(HANDLE); @@ -397,7 +427,7 @@ impl Drop for OwnedHandle { } } -/// a child process. tries to kill the process when dropped +/// A child process. Tries to kill the process when dropped. struct Child(OwnedHandle); impl Child { @@ -434,7 +464,7 @@ impl Drop for Child { } } -/// a users environment block +/// A users' environment block. /// https://learn.microsoft.com/en-us/windows/win32/api/userenv/nf-userenv-createenvironmentblock struct EnvBlock(*mut c_void); @@ -456,19 +486,19 @@ impl Drop for EnvBlock { } } -/// manages the child daemon, by spawning / stopping it, or reporting abnormal exit, and allowing wait() +/// Manages the child daemon, by spawning / stopping it, or reporting abnormal exit, and allowing wait(). struct Spawner { - // whether a child daemon is running + // Whether a child daemon is running. running: Arc, - // holds the actual process of the running child daemon + // Holds the actual process of the running child daemon. child: Arc>, - // whether the process has exited without our request + // Whether the process has exited without our request. dirty: Arc, - // used to differentiate between requested stop() and if process is dirty ^ + // Used to differentiate between requested stop() and if process is dirty (see above). request_stop: Arc, - // used for wait()ing until the child is done + // Used for wait()ing until the child is done. wait_tx: Sender<()>, - // we don't need mutation, but we do need Sync + // We don't need mutation, but we do need Sync. wait_rx: Mutex>, } @@ -486,14 +516,14 @@ impl Spawner { } } - /// is the child daemon running? + /// Is the child daemon running? fn running(&self) -> bool { self.running.load(Ordering::Relaxed) } - /// note: if you need any `while` loop to exit by checking condition, + /// Note: if you need any `while` loop to exit by checking condition, /// make _sure_ you put this stop() _after_ you change the `while` condition to false - /// otherwise it will not be observable + /// otherwise it will not be observable. fn stop(&self) { let mut child = self.child.lock().unwrap(); @@ -502,9 +532,9 @@ impl Spawner { Ok(_) => { debug!("stop() kill"); self.running.store(false, Ordering::Relaxed); - // signal the wait() to exit so a `while` condition is checked at least once more. - // as long as `while` conditions have been changed _before_ the call to stop(), - // the changed condition will be observed + // Signal the wait() to exit so a `while` condition is checked at least once more. + // As long as `while` conditions have been changed _before_ the call to stop(), + // the changed condition will be observed. _ = self.wait_tx.send(()); } @@ -515,17 +545,17 @@ impl Spawner { } } - /// wait for child process to exit + /// Wait for child process to exit. fn wait(&self) { _ = self.wait_rx.lock().unwrap().recv(); } - /// did the spawned process quit without our request? + /// Did the spawned process quit without our request? fn dirty(&self) -> bool { self.dirty.load(Ordering::Relaxed) } - /// try to spawn a child daemon + /// Try to spawn a child daemon. fn start(&self, session: Option) -> Result<()> { let Some(session) = session.or_else(get_current_session) else { bail!("get_current_session failed"); @@ -598,7 +628,7 @@ impl Spawner { running.store(true, Ordering::Relaxed); } - // wait until the process exits + // Wait until the process exits. unsafe { WaitForSingleObject(process_info.hProcess, INFINITE); } @@ -614,8 +644,8 @@ impl Spawner { debug!("spawner code {code}"); - // windows gives this exit code on the process in event of forced process shutdown - // this happens on logoff, so we treat this code as normal + // Windows gives this exit code on the process in event of forced process shutdown. + // This happens on logoff, so we treat this code as normal. const LOGOFF: u32 = 0x40010004; if code != 0 && code != LOGOFF { debug!("service storing dirty true"); From 662a3307a800434d7a70c04d7aba349bce272e02 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 18 Sep 2024 07:31:55 -0700 Subject: [PATCH 23/36] Slight comment sentence fixes --- pueue/src/bin/pueued.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pueue/src/bin/pueued.rs b/pueue/src/bin/pueued.rs index 0387532a..d6ad5840 100644 --- a/pueue/src/bin/pueued.rs +++ b/pueue/src/bin/pueued.rs @@ -130,9 +130,9 @@ fn fork_daemon(opt: &CliArguments) -> Result<()> { { use std::os::windows::process::CommandExt; const CREATE_NO_WINDOW: u32 = 0x08000000; - // create_no_window causes all children to not show a visible console window + // CREATE_NO_WINDOW causes all children to not show a visible console window, // but it also apparently has the effect of DETACH_PROCESS in that it's no longer tied to - // the parent process + // the parent processs. // https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags#flags command.creation_flags(CREATE_NO_WINDOW); } From bd9cc34dfc4065620a1c8e038ebe4d4f54734a74 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 18 Sep 2024 07:35:37 -0700 Subject: [PATCH 24/36] Fix a few more comments --- pueue/src/daemon/service.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 825fdcc9..e1c4aa08 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -258,6 +258,9 @@ fn service_event_loop() -> Result<()> { reason: SessionChangeReason::SessionLogoff, .. }) => { + // Windows services kill all user processes on logoff. + // So this stopping is basically a noop, but I favor explicitness. + // See module-level docs for more details. debug!("event logoff"); spawner.stop(); @@ -294,7 +297,7 @@ fn service_event_loop() -> Result<()> { set_status(ServiceState::StartPending, ServiceControlAccept::empty())?; - // make sure we have privileges - this should always succeed + // Make sure we have privileges - this should always succeed if let Err(e) = set_privilege(SE_TCB_NAME, true) { set_status(ServiceState::Stopped, ServiceControlAccept::empty())?; bail!("failed to set privileges: {e}"); @@ -326,7 +329,7 @@ fn service_event_loop() -> Result<()> { Ok(()) } -/// set the specified process privilege to state +/// Set the specified process privilege to state. /// https://learn.microsoft.com/en-us/windows/win32/secauthz/privilege-constants fn set_privilege(name: PCWSTR, state: bool) -> Result<()> { let handle: OwnedHandle = @@ -597,8 +600,8 @@ impl Spawner { .chain(iter::once(0)) .collect::>(); - // lpcommandline may modify the the cmd vec. max valid size is 1024, so - // I think it best to ensure 1024 bytes of capacity total exist just in case + // lpcommandline may modify the the cmd vec. Max valid size is 1024, so + // I think it best to ensure 1024 bytes of capacity total exist just in case. command.reserve(1024 - (command.len() / 2)); let env_block = EnvBlock::new(token.0)?; @@ -635,7 +638,7 @@ impl Spawner { running.store(false, Ordering::Relaxed); - // check if process exited on its own without our explicit request + // Check if process exited on its own without our explicit request. if !request_stop.swap(false, Ordering::Relaxed) { let mut code = 0u32; unsafe { From 9d30ba558c191377e2e81e46097a42afccaf8f50 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 18 Sep 2024 07:53:56 -0700 Subject: [PATCH 25/36] Fixup comments + use lpApplicationName for simplification --- pueue/src/daemon/service.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index e1c4aa08..bc964edb 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -593,16 +593,28 @@ impl Spawner { let arguments = arguments.join(" "); // Try to get the path to the current binary - let current_exe = env::current_exe()?.to_string_lossy().to_string(); + let mut current_exe = env::current_exe()? + .to_string_lossy() + .to_string() + .encode_utf16() + .chain(iter::once(0)) + .collect::>(); - let mut command = format!(r#""{current_exe}" {arguments}"#) + let mut arguments = arguments .encode_utf16() .chain(iter::once(0)) .collect::>(); - // lpcommandline may modify the the cmd vec. Max valid size is 1024, so - // I think it best to ensure 1024 bytes of capacity total exist just in case. - command.reserve(1024 - (command.len() / 2)); + // CreateProcessAsUserW's lpcommandline arg may modify the the cmd vec, so we need to account for this. + // Does "modify" mean we need extra room in the string? Seems to according to the (below) docs, but how much? + // + // As per original docs (below), this potentially adds 1 extra character to the source. Do we need more than this? + // The system adds a null character to the command line string to separate the file name from the arguments. + // This divides the original string into two strings for internal processing. + // + // https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessasuserw + // https://devblogs.microsoft.com/oldnewthing/20090601-00/?p=18083 + arguments.reserve(10); let env_block = EnvBlock::new(token.0)?; @@ -610,13 +622,13 @@ impl Spawner { unsafe { CreateProcessAsUserW( token.0, - None, - PWSTR(command.as_mut_ptr()), + PWSTR(current_exe.as_mut_ptr()), + PWSTR(arguments.as_mut_ptr()), None, None, false, - // unicode is required if we pass env block - // create_no_window causes all child processes to not show a visible console window + // CREATE_UNICODE_ENVIRONMENT is required if we pass env block. + // CREATE_NO_WINDOW causes all child processes to not show a visible console window. CREATE_UNICODE_ENVIRONMENT | CREATE_NO_WINDOW, Some(env_block.0), None, From ac174ac59ab3ade94db63263b6d8f2f2a9e8edb2 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 18 Sep 2024 09:42:17 -0700 Subject: [PATCH 26/36] Add a little more to module level docs --- pueue/src/daemon/service.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index bc964edb..843b87e1 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -1,4 +1,4 @@ -//! How Windows services (and this file) work +//! How Windows services (and this service) work //! //! - This service runs as SYSTEM, and survives logoff and logon. //! - This service launches the daemon as current user on login, and kills it on logoff @@ -27,6 +27,8 @@ //! would not apply to this account. You'd have to set up special configs for the SYSTEM account, //! and I'm not even sure where the SYSTEM account's appdata is stored to begin with. //! (not to mention, it would be a pain for the user to setup anyways) +//! - Is the service failing to start up? It's probably a problem with the daemon. Run `pueued` +//! to see the actual error. use std::{ env, From 8c9bbe79e1f3c9ebe507d38fe7544c37b33e025a Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 18 Sep 2024 11:36:11 -0700 Subject: [PATCH 27/36] Add to help text --- pueue/src/daemon/cli.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pueue/src/daemon/cli.rs b/pueue/src/daemon/cli.rs index bd1c7b66..e89450d3 100644 --- a/pueue/src/daemon/cli.rs +++ b/pueue/src/daemon/cli.rs @@ -49,7 +49,8 @@ pub enum ServiceSubcommand { /// Install as a Windows service. /// Once installed, you must not move the binary, otherwise the Windows /// service will not be able to find it. If you wish to move the binary, - /// first uninstall the service, then install the service again. + /// first uninstall the service, move the binary, then install the service + /// again. Install, /// Uninstall the service. Uninstall, From 9a8a3476ad9baf229bb8529ecf445c55734e842c Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 18 Sep 2024 15:19:37 -0700 Subject: [PATCH 28/36] Add more clarification in docs --- pueue/src/bin/pueued.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pueue/src/bin/pueued.rs b/pueue/src/bin/pueued.rs index d6ad5840..43d3d8cb 100644 --- a/pueue/src/bin/pueued.rs +++ b/pueue/src/bin/pueued.rs @@ -131,9 +131,10 @@ fn fork_daemon(opt: &CliArguments) -> Result<()> { use std::os::windows::process::CommandExt; const CREATE_NO_WINDOW: u32 = 0x08000000; // CREATE_NO_WINDOW causes all children to not show a visible console window, - // but it also apparently has the effect of DETACH_PROCESS in that it's no longer tied to - // the parent processs. + // but it also apparently has the effect of starting a new process group. + // // https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags#flags + // https://stackoverflow.com/a/71364777/9423933 command.creation_flags(CREATE_NO_WINDOW); } From fd708690200a30e29682b1e74589a30a2fe6651d Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 18 Sep 2024 15:43:04 -0700 Subject: [PATCH 29/36] Shorten calling cb() --- pueue/src/daemon/service.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 843b87e1..8291b2a7 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -400,9 +400,7 @@ fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Resu )?; } - let t = cb(token)?; - - Ok(t) + cb(token) } /// Newtype over handle which closes the HANDLE on drop. From 54d4bfdaa0fdbf352c79c6682a5912dac0554e5b Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Wed, 18 Sep 2024 15:52:45 -0700 Subject: [PATCH 30/36] Small finishing touches - Turns out `arguments` does not need extra buffer space - We can use the Free trait for HANDLE drop - And made the dirty code checking more concise (added some comment to make it clear though) --- pueue/src/daemon/service.rs | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 8291b2a7..c336214e 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -48,9 +48,9 @@ use std::{ use anyhow::{anyhow, bail, Result}; use log::{debug, error, info}; use windows::{ - core::{PCWSTR, PWSTR}, + core::{Free, PCWSTR, PWSTR}, Win32::{ - Foundation::{CloseHandle, HANDLE, LUID}, + Foundation::{HANDLE, LUID}, Security::{ AdjustTokenPrivileges, DuplicateTokenEx, LookupPrivilegeValueW, SecurityIdentification, TokenPrimary, SE_PRIVILEGE_ENABLED, SE_PRIVILEGE_REMOVED, SE_TCB_NAME, @@ -424,8 +424,8 @@ impl From for OwnedHandle { impl Drop for OwnedHandle { fn drop(&mut self) { - if !self.0.is_invalid() { - _ = unsafe { CloseHandle(self.0) }; + unsafe { + self.0.free(); } } } @@ -605,17 +605,6 @@ impl Spawner { .chain(iter::once(0)) .collect::>(); - // CreateProcessAsUserW's lpcommandline arg may modify the the cmd vec, so we need to account for this. - // Does "modify" mean we need extra room in the string? Seems to according to the (below) docs, but how much? - // - // As per original docs (below), this potentially adds 1 extra character to the source. Do we need more than this? - // The system adds a null character to the command line string to separate the file name from the arguments. - // This divides the original string into two strings for internal processing. - // - // https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessasuserw - // https://devblogs.microsoft.com/oldnewthing/20090601-00/?p=18083 - arguments.reserve(10); - let env_block = EnvBlock::new(token.0)?; let mut process_info = PROCESS_INFORMATION::default(); @@ -659,10 +648,9 @@ impl Spawner { debug!("spawner code {code}"); - // Windows gives this exit code on the process in event of forced process shutdown. + // Windows gives exit code 0x40010004 when it did a forced process shutdown. // This happens on logoff, so we treat this code as normal. - const LOGOFF: u32 = 0x40010004; - if code != 0 && code != LOGOFF { + if code != 0x40010004 { debug!("service storing dirty true"); dirty.store(true, Ordering::Relaxed); _ = waiter.send(()); From 6d90c9640b02dc74ac5679400c366d0538c9c56b Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Thu, 19 Sep 2024 06:52:44 -0700 Subject: [PATCH 31/36] More fixups - Added more docs. - Reordered some code. - Remove noop stop() just to ensure no race condition can occur. --- pueue/src/daemon/service.rs | 108 +++++++++++++++++++++++++----------- 1 file changed, 77 insertions(+), 31 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index c336214e..da06af1f 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -87,24 +87,34 @@ struct Config { profile: Option, } +// The name of the installed service. const SERVICE_NAME: &str = "pueued"; +// The type of service. This one runs in its own dedicated process. const SERVICE_TYPE: ServiceType = ServiceType::OWN_PROCESS; +// This static lets us communicate Config over ffi callbacks. static CONFIG: OnceLock = OnceLock::new(); +// For how this works, please see docs @ +// https://docs.rs/windows-service/0.7.0/windows_service/#basics define_windows_service!(ffi_service_main, service_main); -pub fn run_service(config_path: Option, profile: Option) -> Result<()> { - CONFIG - .set(Config { - config_path, - profile, - }) - .map_err(|_| anyhow!("static CONFIG set failed"))?; - - service_dispatcher::start(SERVICE_NAME, ffi_service_main)?; - Ok(()) +/// The main service callback after `ffi_service_main`. +fn service_main(_: Vec) { + if let Err(e) = event_loop() { + error!("Failed to start service: {e}"); + } } +/// Installs the service. +/// +/// This must be run as admin. +/// +/// This passes the config and profile flags passed at the time of install to the service, e.g. +/// `pueued --config my-path --profile my_profile service install` +/// becomes -> +/// `C:\path\pueued.exe --config "my-path" --profile "my_profile" service run` +/// +/// This is set to run as SYSTEM user, and survives login/logoffs. pub fn install_service(config_path: Option, profile: Option) -> Result<()> { let manager_access = ServiceManagerAccess::CONNECT | ServiceManagerAccess::CREATE_SERVICE; let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)?; @@ -143,6 +153,9 @@ pub fn install_service(config_path: Option, profile: Option) -> Ok(()) } +/// Uninstall the service. +/// +/// This must be run as admin. pub fn uninstall_service() -> Result<()> { let manager_access = ServiceManagerAccess::CONNECT; let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)?; @@ -164,6 +177,9 @@ pub fn uninstall_service() -> Result<()> { Ok(()) } +/// Start the service. +/// +/// This can also be done from the windows service manager. pub fn start_service() -> Result<()> { let manager_access = ServiceManagerAccess::CONNECT; let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)?; @@ -185,6 +201,9 @@ pub fn start_service() -> Result<()> { Ok(()) } +/// Stop the service. +/// +/// This can also be done from the windows service manager. pub fn stop_service() -> Result<()> { let manager_access = ServiceManagerAccess::CONNECT; let service_manager = ServiceManager::local_computer(None::<&str>, manager_access)?; @@ -206,17 +225,30 @@ pub fn stop_service() -> Result<()> { Ok(()) } -fn service_main(_: Vec) { - if let Err(e) = service_event_loop() { - error!("Failed to start service: {e}"); - } +/// Begins running the pueued service. +/// +/// This calls `ffi_service_main` -> `service_main` -> `event_loop` +pub fn run_service(config_path: Option, profile: Option) -> Result<()> { + CONFIG + .set(Config { + config_path, + profile, + }) + .map_err(|_| anyhow!("static CONFIG set failed"))?; + + service_dispatcher::start(SERVICE_NAME, ffi_service_main)?; + Ok(()) } -fn service_event_loop() -> Result<()> { +/// This is the main event loop for the service. +/// +/// This gets called from `run_service` -> `ffi_service_main` -> `service_main` -> `event_loop` +fn event_loop() -> Result<()> { let spawner = Arc::new(Spawner::new()); - // a shutdown of the service was requested + // Whether a shutdown of the service was requested. let shutdown = Arc::new(AtomicBool::default()); + // The main event handler for the service. let event_handler = { let spawner = spawner.clone(); let shutdown = shutdown.clone(); @@ -260,11 +292,9 @@ fn service_event_loop() -> Result<()> { reason: SessionChangeReason::SessionLogoff, .. }) => { - // Windows services kill all user processes on logoff. - // So this stopping is basically a noop, but I favor explicitness. - // See module-level docs for more details. + // Windows kills all user processes on logoff. + // So we don't actually need to stop any running spawner. debug!("event logoff"); - spawner.stop(); ServiceControlHandlerResult::NoError } @@ -275,6 +305,7 @@ fn service_event_loop() -> Result<()> { // All services must accept Interrogate even if it's a no-op. ServiceControl::Interrogate => ServiceControlHandlerResult::NoError, + // Nothing else is implemented. _ => ServiceControlHandlerResult::NotImplemented, } } @@ -305,8 +336,11 @@ fn service_event_loop() -> Result<()> { bail!("failed to set privileges: {e}"); } - // If it fails here, we probably launched before user logged in? - // For that reason we only log, but do not bail and stop the service. + // This attempt is required in order to properly start pueued if the user starts/restarts + // the service manually, since no events would have been triggered from that. + // + // If it fails here, we probably launched before user logged in. + // For these reasons, we only log an error, but do not bail and stop the service. // The event handler will start it when the user logs in. if let Err(e) = spawner.start(None) { error!("failed to spawn: {e}"); @@ -375,14 +409,21 @@ fn get_current_session() -> Option { let session = unsafe { WTSGetActiveConsoleSessionId() }; match session { + // No session attached. + // https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-wtsgetactiveconsolesessionid#return-value 0xFFFFFFFF => None, + // Found a session! session => Some(session), } } /// Run closure and supply the currently logged in user's token. -fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Result { +fn run_as(session_id: u32, cb: impl FnOnce(HANDLE) -> Result) -> Result { let mut query_token: OwnedHandle = OwnedHandle::default(); + // Obtain the user's primary access token. Requires we are SYSTEM and have SE_TCB_NAME. + // + // Make sure to not leak this token anywhere, as it must remain secure. + // https://learn.microsoft.com/en-us/windows/win32/api/wtsapi32/nf-wtsapi32-wtsqueryusertoken unsafe { WTSQueryUserToken(session_id, &mut query_token.0)?; } @@ -400,7 +441,7 @@ fn run_as(session_id: u32, cb: impl FnOnce(OwnedHandle) -> Result) -> Resu )?; } - cb(token) + cb(token.0) } /// Newtype over handle which closes the HANDLE on drop. @@ -524,12 +565,15 @@ impl Spawner { self.running.load(Ordering::Relaxed) } - /// Note: if you need any `while` loop to exit by checking condition, + /// Stop the spawned daemon. + /// + /// Note: if you need any `while` loop to exit by checking a condition, /// make _sure_ you put this stop() _after_ you change the `while` condition to false - /// otherwise it will not be observable. + /// otherwise any condition change will not be observed. fn stop(&self) { let mut child = self.child.lock().unwrap(); + // Request a normal stop. This is not an abnormal process exit. self.request_stop.store(true, Ordering::Relaxed); match child.kill() { Ok(_) => { @@ -605,12 +649,12 @@ impl Spawner { .chain(iter::once(0)) .collect::>(); - let env_block = EnvBlock::new(token.0)?; + let env_block = EnvBlock::new(token)?; let mut process_info = PROCESS_INFORMATION::default(); unsafe { CreateProcessAsUserW( - token.0, + token, PWSTR(current_exe.as_mut_ptr()), PWSTR(arguments.as_mut_ptr()), None, @@ -626,12 +670,14 @@ impl Spawner { )?; } + // Store the child process. { let mut lock = child.lock().unwrap(); *lock = Child(process_info.hProcess.into()); - running.store(true, Ordering::Relaxed); } + running.store(true, Ordering::Relaxed); + // Wait until the process exits. unsafe { WaitForSingleObject(process_info.hProcess, INFINITE); @@ -639,7 +685,7 @@ impl Spawner { running.store(false, Ordering::Relaxed); - // Check if process exited on its own without our explicit request. + // Check if process exited on its own without our explicit request (`stop()` was not called). if !request_stop.swap(false, Ordering::Relaxed) { let mut code = 0u32; unsafe { @@ -649,7 +695,7 @@ impl Spawner { debug!("spawner code {code}"); // Windows gives exit code 0x40010004 when it did a forced process shutdown. - // This happens on logoff, so we treat this code as normal. + // This happens on logoff, so we ignore this code as it's not dirty. if code != 0x40010004 { debug!("service storing dirty true"); dirty.store(true, Ordering::Relaxed); From 77233f3c1905366eb05f53de4ab47fbb341f4130 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Thu, 19 Sep 2024 07:24:33 -0700 Subject: [PATCH 32/36] Even more docs --- pueue/src/daemon/service.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index da06af1f..e43a1a25 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -1,34 +1,34 @@ //! How Windows services (and this service) work //! -//! - This service runs as SYSTEM, and survives logoff and logon. -//! - This service launches the daemon as current user on login, and kills it on logoff -//! (actually it's a noop; Windows itself kills it - see below). -//! - All user processes are auto killed by Windows on user logoff. This is not a feature -//! of this service, it's just how Windows does things. +//! This service runs as SYSTEM, and survives logoff and logon. On startup/login, it launches +//! pueued as the current user. On logoff Windows kills all user processes (including the daemon). +//! +//! - All install/uninstallations of the service requires you are running as admin. //! - You must install the service. After installed, the service entry maintains a cmdline //! string with args to the pueued binary. Therefore, the binary must _not_ move while the //! service is installed, otherwise it will not be able to function properly. It is best //! not to rely on PATH for this, as it is finicky and a hassle for user setup. Absolute paths //! are the way to go, and it is standard practice. //! - To move the pueued binary: Uninstall the service, move the binary, and reinstall the service. -//! - When the service is installed, you can use pueued to start, stop, or uninstall the service. +//! - When the service is installed, you can use pueued cli to start, stop, or uninstall the service. //! You can also use the official service manager to start, stop, and restart the service. //! - Services are automatically started/stopped by the system according to the setting the user //! sets in the windows service manager. By default we install it as autostart, but the user //! can set this to manual or even disabled. //! - If you have the official service manager window open and you tell pueued to uninstall the //! service, it will not disappear from the list until you close all service manager windows. -//! This is Windows specific behavior, and not a bug. (In Windows parlance, the service is pending +//! This is not a bug. It's Windows specific behavior. (In Windows parlance, the service is pending //! deletion, and all HANDLES to the service need to be closed). -//! - We do not support long running processes past when a user logs off; this would be -//! a massive security risk to allow anyone to run processes as SYSTEM. This account bypasses -//! even administrator in power! It is not something small to give permission to. +//! - We do not support long running daemon past when a user logs off; this would be +//! a massive security risk to allow anyone to launch tasks as SYSTEM. This account bypasses +//! even administrator in power! //! - Additionally, taking the above into account, as SYSTEM is its own account, the user config -//! would not apply to this account. You'd have to set up special configs for the SYSTEM account, -//! and I'm not even sure where the SYSTEM account's appdata is stored to begin with. -//! (not to mention, it would be a pain for the user to setup anyways) -//! - Is the service failing to start up? It's probably a problem with the daemon. Run `pueued` -//! to see the actual error. +//! does not apply to this account. Unless there's an exception for config locations with this case, +//! you'd have to set up separate configs for the SYSTEM account in +//! `C:\Windows\system32\config\systemprofile\AppData`. (And even if there's an exception, what if +//! there's multiple users? Which user's config would be used?) This is very unintuitive. +//! - Is the service failing to start up? It's probably a problem with the daemon itself. Re-run `pueued` +//! by itself to see the actual startup error. use std::{ env, From 40508c9c8cc940770c6ae5d0bd795074fd018dfb Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Thu, 19 Sep 2024 07:35:38 -0700 Subject: [PATCH 33/36] Refactor signature of Spawner::start to be more robust to calling --- pueue/src/daemon/service.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index e43a1a25..d4df150b 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -278,7 +278,7 @@ fn event_loop() -> Result<()> { debug!("event login"); if !spawner.running() { debug!("event login: spawning"); - if let Err(e) = spawner.start(Some(session)) { + if let Err(e) = spawner.start(session) { error!("failed to spawn: {e}"); return ServiceControlHandlerResult::Other(1); } @@ -339,11 +339,16 @@ fn event_loop() -> Result<()> { // This attempt is required in order to properly start pueued if the user starts/restarts // the service manually, since no events would have been triggered from that. // - // If it fails here, we probably launched before user logged in. - // For these reasons, we only log an error, but do not bail and stop the service. + // If we can get the current user session on startup, then try to start the spawner. + // + // If we can't get the current session, that's OK. It just means there was no user logged in when + // the service started. + // // The event handler will start it when the user logs in. - if let Err(e) = spawner.start(None) { - error!("failed to spawn: {e}"); + if let Some(session) = get_current_session() { + if let Err(e) = spawner.start(session) { + error!("failed to spawn: {e}"); + } } set_status( @@ -603,11 +608,7 @@ impl Spawner { } /// Try to spawn a child daemon. - fn start(&self, session: Option) -> Result<()> { - let Some(session) = session.or_else(get_current_session) else { - bail!("get_current_session failed"); - }; - + fn start(&self, session: u32) -> Result<()> { let running = self.running.clone(); let child = self.child.clone(); let waiter = self.wait_tx.clone(); From e6d0d5053baa924a7683a88e0ceb1de56964b23a Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Thu, 19 Sep 2024 07:44:08 -0700 Subject: [PATCH 34/36] Eagerly drop some things cause we can. --- pueue/src/daemon/service.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index d4df150b..787ca402 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -446,6 +446,8 @@ fn run_as(session_id: u32, cb: impl FnOnce(HANDLE) -> Result) -> Result )?; } + drop(query_token); + cb(token.0) } @@ -671,6 +673,10 @@ impl Spawner { )?; } + // It is safe to drop this after calling CreateProcessAsUser. + // https://learn.microsoft.com/en-us/windows/win32/api/userenv/nf-userenv-createenvironmentblock#remarks + drop(env_block); + // Store the child process. { let mut lock = child.lock().unwrap(); From 05f180f2fa3aadcecb7904185eb814313734857b Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Thu, 19 Sep 2024 07:47:20 -0700 Subject: [PATCH 35/36] Add links to comment --- pueue/src/daemon/service.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pueue/src/daemon/service.rs b/pueue/src/daemon/service.rs index 787ca402..86cbcccd 100644 --- a/pueue/src/daemon/service.rs +++ b/pueue/src/daemon/service.rs @@ -664,7 +664,10 @@ impl Spawner { None, false, // CREATE_UNICODE_ENVIRONMENT is required if we pass env block. + // https://learn.microsoft.com/en-us/windows/win32/api/userenv/nf-userenv-createenvironmentblock#remarks + // // CREATE_NO_WINDOW causes all child processes to not show a visible console window. + // https://stackoverflow.com/a/71364777/9423933 CREATE_UNICODE_ENVIRONMENT | CREATE_NO_WINDOW, Some(env_block.0), None, From 5115c6818b421a02fa1aac03851340bb8b6e9f45 Mon Sep 17 00:00:00 2001 From: Cherry <13651622+MolotovCherry@users.noreply.github.com> Date: Sat, 21 Sep 2024 15:02:31 -0700 Subject: [PATCH 36/36] cargo update --- Cargo.lock | 141 ++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 112 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 77fbf73a..0f7f3cd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -232,15 +232,15 @@ checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "bytes" -version = "1.7.1" +version = "1.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8318a53db07bb3f8dca91a600466bdb3f2eaadeedfdbcf02e1accbad9271ba50" +checksum = "428d9aa8fbc0670b7b8d6030a7fadd0f86151cae55e4dbbece15f3780a3dfaf3" [[package]] name = "cc" -version = "1.1.20" +version = "1.1.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45bcde016d64c21da4be18b655631e5ab6d3107607e71a73a9f53eb48aae23fb" +checksum = "07b1695e2c7e8fc85310cde85aeaab7e3097f593c91d209d3f9df76c928100f0" dependencies = [ "shlex", ] @@ -294,9 +294,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.17" +version = "4.5.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e5a21b8495e732f1b3c364c9949b201ca7bae518c502c80256c96ad79eaf6ac" +checksum = "b0956a43b323ac1afaffc053ed5c4b7c1f1800bacd1683c353aabbb752515dd3" dependencies = [ "clap_builder", "clap_derive", @@ -304,9 +304,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.17" +version = "4.5.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8cf2dd12af7a047ad9d6da2b6b249759a22a7abc0f474c1dae1777afa4b21a73" +checksum = "4d72166dd41634086d5803a47eb71ae740e61d84709c36f3c34110173db3961b" dependencies = [ "anstream", "anstyle", @@ -316,9 +316,9 @@ dependencies = [ [[package]] name = "clap_complete" -version = "4.5.28" +version = "4.5.29" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b378c786d3bde9442d2c6dd7e6080b2a818db2b96e30d6e7f1b6d224eb617d3" +checksum = "8937760c3f4c60871870b8c3ee5f9b30771f792a7045c48bcbba999d7d6b3b8e" dependencies = [ "clap", ] @@ -335,9 +335,9 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.5.13" +version = "4.5.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "501d359d5f3dcaf6ecdeee48833ae73ec6e42723a1e52419c79abf9507eec0a0" +checksum = "4ac6a0c7b1a9e9a5186361f67dfa1b88213572f427fb9ab038efb2bd8c582dab" dependencies = [ "heck", "proc-macro2", @@ -758,7 +758,7 @@ dependencies = [ "iana-time-zone-haiku", "js-sys", "wasm-bindgen", - "windows-core", + "windows-core 0.52.0", ] [[package]] @@ -893,18 +893,18 @@ checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24" [[package]] name = "logos" -version = "0.14.1" +version = "0.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff1ceb190eb9bdeecdd8f1ad6a71d6d632a50905948771718741b5461fb01e13" +checksum = "1c6b6e02facda28ca5fb8dbe4b152496ba3b1bd5a4b40bb2b1b2d8ad74e0f39b" dependencies = [ "logos-derive", ] [[package]] name = "logos-codegen" -version = "0.14.1" +version = "0.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "90be66cb7bd40cb5cc2e9cfaf2d1133b04a3d93b72344267715010a466e0915a" +checksum = "b32eb6b5f26efacd015b000bfc562186472cd9b34bdba3f6b264e2a052676d10" dependencies = [ "beef", "fnv", @@ -917,9 +917,9 @@ dependencies = [ [[package]] name = "logos-derive" -version = "0.14.1" +version = "0.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45154231e8e96586b39494029e58f12f8ffcb5ecf80333a603a13aa205ea8cbd" +checksum = "3e5d0c5463c911ef55624739fc353238b4e310f0144be1f875dc42fec6bfd5ec" dependencies = [ "logos-codegen", ] @@ -1095,9 +1095,9 @@ dependencies = [ [[package]] name = "pest" -version = "2.7.12" +version = "2.7.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c73c26c01b8c87956cea613c907c9d6ecffd8d18a2a5908e5de0adfaa185cea" +checksum = "fdbef9d1d47087a895abd220ed25eb4ad973a5e26f6a4367b038c25e28dfc2d9" dependencies = [ "memchr", "thiserror", @@ -1106,9 +1106,9 @@ dependencies = [ [[package]] name = "pest_derive" -version = "2.7.12" +version = "2.7.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "664d22978e2815783adbdd2c588b455b1bd625299ce36b2a99881ac9627e6d8d" +checksum = "4d3a6e3394ec80feb3b6393c725571754c6188490265c61aaf260810d6b95aa0" dependencies = [ "pest", "pest_generator", @@ -1116,9 +1116,9 @@ dependencies = [ [[package]] name = "pest_generator" -version = "2.7.12" +version = "2.7.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2d5487022d5d33f4c30d91c22afa240ce2a644e87fe08caad974d4eab6badbe" +checksum = "94429506bde1ca69d1b5601962c73f4172ab4726571a59ea95931218cb0e930e" dependencies = [ "pest", "pest_meta", @@ -1129,9 +1129,9 @@ dependencies = [ [[package]] name = "pest_meta" -version = "2.7.12" +version = "2.7.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0091754bbd0ea592c4deb3a122ce8ecbb0753b738aa82bc055fcc2eccc8d8174" +checksum = "ac8a071862e93690b6e34e9a5fb8e33ff3734473ac0245b27232222c4906a33f" dependencies = [ "once_cell", "pest", @@ -1283,6 +1283,8 @@ dependencies = [ "test-log", "tokio", "whoami", + "windows", + "windows-service", ] [[package]] @@ -2030,9 +2032,9 @@ checksum = "f6ccf251212114b54433ec949fd6a7841275f9ada20dddd2f29e9ceea4501493" [[package]] name = "unicode-width" -version = "0.1.13" +version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0336d538f7abc86d282a4189614dfaa90810dfc2c6f6427eaf88e16311dd225d" +checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" [[package]] name = "unsafe-libyaml" @@ -2161,6 +2163,12 @@ dependencies = [ "web-sys", ] +[[package]] +name = "widestring" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7219d36b6eac893fa81e84ebe06485e7dcbb616177469b142df14f1f4deb1311" + [[package]] name = "winapi" version = "0.3.9" @@ -2192,6 +2200,16 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd04d41d93c4992d421894c18c8b43496aa748dd4c081bac0dc93eb0489272b6" +dependencies = [ + "windows-core 0.58.0", + "windows-targets 0.52.6", +] + [[package]] name = "windows-core" version = "0.52.0" @@ -2201,6 +2219,71 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "windows-core" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ba6d44ec8c2591c134257ce647b7ea6b20335bf6379a27dac5f1641fcf59f99" +dependencies = [ + "windows-implement", + "windows-interface", + "windows-result", + "windows-strings", + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-implement" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2bbd5b46c938e506ecbce286b6628a02171d56153ba733b6c741fc627ec9579b" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "windows-interface" +version = "0.58.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "053c4c462dc91d3b1504c6fe5a726dd15e216ba718e84a0e46a88fbe5ded3515" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "windows-result" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d1043d8214f791817bab27572aaa8af63732e11bf84aa21a45a78d6c317ae0e" +dependencies = [ + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-service" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d24d6bcc7f734a4091ecf8d7a64c5f7d7066f45585c1861eba06449909609c8a" +dependencies = [ + "bitflags", + "widestring", + "windows-sys 0.52.0", +] + +[[package]] +name = "windows-strings" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cd9b125c486025df0eabcb585e62173c6c9eddcec5d117d3b6e8c30e2ee4d10" +dependencies = [ + "windows-result", + "windows-targets 0.52.6", +] + [[package]] name = "windows-sys" version = "0.48.0"