refactor(windows): deduplicate after simplify review

- Drop the duplicate WINDOWS_SERVICE_NAME constant; call sites use the
  single source of truth at windows_service::SERVICE_NAME.
- windows_service_exe_path and service_config_path now compose from
  crate::data_dir() instead of re-parsing %PROGRAMDATA% locally.
- Factor the 6× sc.exe invocation boilerplate into a run_sc helper.
- Replace the 200ms try_recv polling loop in the service dispatcher
  with a recv_timeout wait — cuts shutdown latency and idle CPU.
- stop_service_scm/delete_service_scm now log warnings instead of
  silently swallowing failures, so unexpected errors are visible.

src/system_dns.rs

@@ -729,20 +729,24 @@ fn install_windows() -> Result<(), String> {
     Ok(())
 }
 
-#[cfg(windows)]
-const WINDOWS_SERVICE_NAME: &str = "Numa";
-
 /// Stable install location for the service binary. SCM keeps a handle to
 /// this path; the user's Downloads folder (where `current_exe()` points at
 /// install time) is not durable.
 #[cfg(windows)]
 fn windows_service_exe_path() -> std::path::PathBuf {
-    std::path::PathBuf::from(
-        std::env::var("PROGRAMDATA").unwrap_or_else(|_| "C:\\ProgramData".into()),
-    )
-    .join("numa")
-    .join("bin")
-    .join("numa.exe")
+    crate::data_dir().join("bin").join("numa.exe")
+}
+
+/// Run `sc.exe` with the given args and return its merged stdout/stderr on
+/// failure. `sc` emits errors on stdout (not stderr) on Windows, so the
+/// caller reads stdout to format a useful error.
+#[cfg(windows)]
+fn run_sc(args: &[&str]) -> Result<std::process::Output, String> {
+    let out = std::process::Command::new("sc")
+        .args(args)
+        .output()
+        .map_err(|e| format!("failed to run sc {}: {}", args.first().unwrap_or(&""), e))?;
+    Ok(out)
 }
 
 /// Copy the currently-running binary to the service install location. SCM
@@ -782,13 +786,13 @@ fn remove_service_binary() {
 #[cfg(windows)]
 fn register_service_scm(exe: &std::path::Path) -> Result<(), String> {
     let bin_path = format!("\"{}\" --service", exe.display());
+    let name = crate::windows_service::SERVICE_NAME;
 
     // sc.exe uses a leading space as its `name= value` delimiter; the space
     // after `=` is mandatory.
-    let create = std::process::Command::new("sc")
-        .args([
+    let create = run_sc(&[
         "create",
-            WINDOWS_SERVICE_NAME,
+        name,
         "binPath=",
         &bin_path,
         "DisplayName=",
@@ -797,9 +801,7 @@ fn register_service_scm(exe: &std::path::Path) -> Result<(), String> {
         "auto",
         "obj=",
         "LocalSystem",
-        ])
-        .output()
-        .map_err(|e| format!("failed to run sc create: {}", e))?;
+    ])?;
     if !create.status.success() {
         let out = String::from_utf8_lossy(&create.stdout);
         // "service already exists" is 1073 — treat as idempotent success.
@@ -808,30 +810,23 @@ fn register_service_scm(exe: &std::path::Path) -> Result<(), String> {
         }
     }
 
-    let _ = std::process::Command::new("sc")
-        .args([
+    let _ = run_sc(&[
         "description",
-            WINDOWS_SERVICE_NAME,
+        name,
         "Self-sovereign DNS resolver (ad blocking, DoH/DoT, local zones).",
-        ])
-        .status();
+    ]);
 
     // Restart on crash: 5s, 5s, 10s; reset failure counter after 60s.
-    let _ = std::process::Command::new("sc")
-        .args([
+    let _ = run_sc(&[
         "failure",
-            WINDOWS_SERVICE_NAME,
+        name,
         "reset=",
         "60",
         "actions=",
         "restart/5000/restart/5000/restart/10000",
-        ])
-        .status();
+    ]);
 
-    eprintln!(
-        "  Registered service '{}' (boot-time).",
-        WINDOWS_SERVICE_NAME
-    );
+    eprintln!("  Registered service '{}' (boot-time).", name);
     Ok(())
 }
 
@@ -840,10 +835,7 @@ fn register_service_scm(exe: &std::path::Path) -> Result<(), String> {
 /// return the underlying error string rather than masking it.
 #[cfg(windows)]
 fn start_service_scm() -> Result<(), String> {
-    let out = std::process::Command::new("sc")
-        .args(["start", WINDOWS_SERVICE_NAME])
-        .output()
-        .map_err(|e| format!("failed to run sc start: {}", e))?;
+    let out = run_sc(&["start", crate::windows_service::SERVICE_NAME])?;
     if !out.status.success() {
         let text = String::from_utf8_lossy(&out.stdout);
         if text.contains("1056") {
@@ -854,20 +846,22 @@ fn start_service_scm() -> Result<(), String> {
     Ok(())
 }
 
-/// Stop the service. Returns Ok if already stopped — idempotent.
+/// Stop the service. Idempotent — already-stopped or missing service logs
+/// a warning but doesn't error, since both callers (install re-run,
+/// uninstall) want best-effort cleanup rather than hard failure.
 #[cfg(windows)]
 fn stop_service_scm() {
-    let _ = std::process::Command::new("sc")
-        .args(["stop", WINDOWS_SERVICE_NAME])
-        .status();
+    if let Err(e) = run_sc(&["stop", crate::windows_service::SERVICE_NAME]) {
+        log::warn!("sc stop failed: {}", e);
+    }
 }
 
-/// Remove the service from SCM. Safe if already absent.
+/// Remove the service from SCM. Idempotent — see `stop_service_scm`.
 #[cfg(windows)]
 fn delete_service_scm() {
-    let _ = std::process::Command::new("sc")
-        .args(["delete", WINDOWS_SERVICE_NAME])
-        .status();
+    if let Err(e) = run_sc(&["delete", crate::windows_service::SERVICE_NAME]) {
+        log::warn!("sc delete failed: {}", e);
+    }
 }
 
 #[cfg(windows)]

src/windows_service.rs

@@ -62,7 +62,7 @@ fn run_service() -> windows_service::Result<()> {
     // once the SCM tells us to stop — we can't block the dispatcher thread
     // forever without preventing graceful shutdown.
     let config_path = service_config_path();
-    let (runtime_stop_tx, runtime_stop_rx) = mpsc::channel::<()>();
+    let (server_done_tx, server_done_rx) = mpsc::channel::<()>();
 
     let server_thread = std::thread::spawn(move || {
         let runtime = match tokio::runtime::Builder::new_multi_thread()
@@ -72,28 +72,25 @@ fn run_service() -> windows_service::Result<()> {
             Ok(rt) => rt,
             Err(e) => {
                 log::error!("failed to build tokio runtime: {}", e);
-                let _ = runtime_stop_tx.send(());
+                let _ = server_done_tx.send(());
                 return;
             }
         };
 
-        // block_on returns when serve::run's UDP loop errors out OR when the
-        // runtime is dropped from another thread. Either signals exit.
         if let Err(e) = runtime.block_on(crate::serve::run(config_path)) {
             log::error!("numa serve exited with error: {}", e);
         }
-        let _ = runtime_stop_tx.send(());
+        let _ = server_done_tx.send(());
     });
 
     // Wait for either SCM stop or server termination.
     loop {
-        if shutdown_rx.try_recv().is_ok() {
+        if shutdown_rx.recv_timeout(Duration::from_millis(500)).is_ok() {
             break;
         }
-        if runtime_stop_rx.try_recv().is_ok() {
+        if server_done_rx.try_recv().is_ok() {
             break;
         }
-        std::thread::sleep(Duration::from_millis(200));
     }
 
     // The server's tokio runtime runs detached inside server_thread. Abandon
@@ -124,9 +121,10 @@ pub fn run_as_service() -> windows_service::Result<()> {
 
 /// Path to the config file used when running under SCM. SCM launches the
 /// service with SYSTEM's working directory (usually `C:\Windows\System32`),
-/// so a relative `numa.toml` lookup won't find anything meaningful — use an
-/// absolute path under `%PROGRAMDATA%` instead.
+/// so a relative `numa.toml` lookup won't find anything meaningful.
 fn service_config_path() -> String {
-    let base = std::env::var("PROGRAMDATA").unwrap_or_else(|_| "C:\\ProgramData".into());
-    format!("{}\\numa\\numa.toml", base)
+    crate::data_dir()
+        .join("numa.toml")
+        .to_string_lossy()
+        .into_owned()
 }