diff --git a/src/RemSound.App/MainForm.cs b/src/RemSound.App/MainForm.cs index f953233..dd69e6f 100644 --- a/src/RemSound.App/MainForm.cs +++ b/src/RemSound.App/MainForm.cs @@ -2380,14 +2380,32 @@ public sealed class MainForm : Form } logFile.Event($"service: {label} requested (elevated)"); ServiceStore.AppendServiceEvent($"{label} requested (elevated)"); - var rc = ServiceControl.RunElevated(verb); - var outcome = rc == 0 ? "success" : rc == -1 ? "cancelled/declined" : "failed"; + // Run the elevated helper OFF the UI thread. A slow or stuck helper must never freeze the window or + // its audio — that was the install hang: the UI thread sat in WaitForExit while the pipe-deadlocked + // installer never returned, so the app locked up and streaming died. The UI stays live; we report + // the outcome (on the UI thread) when the helper comes back or the wait times out. + Task.Run(() => + { + var rc = ServiceControl.RunElevated(verb); + if (IsDisposed) return; + try { BeginInvoke(new Action(() => ReportServiceActionResult(label, rc))); } catch { /* window closing */ } + }); + } + + private void ReportServiceActionResult(string label, int rc) + { + var outcome = rc == 0 ? "success" + : rc == -1 ? "cancelled/declined" + : rc == ServiceControl.ElevatedTimedOut ? "timed out" + : "failed"; logFile.Event($"service: {label} finished with code {rc} ({outcome})"); ServiceStore.AppendServiceEvent($"{label} finished: code {rc} ({outcome})"); if (rc == 0) MessageBox.Show(this, $"Service {label} succeeded.", AppName, MessageBoxButtons.OK, MessageBoxIcon.Information); else if (rc == -1) MessageBox.Show(this, $"Service {label} was cancelled, or administrator rights were declined.", AppName, MessageBoxButtons.OK, MessageBoxIcon.Warning); + else if (rc == ServiceControl.ElevatedTimedOut) + MessageBox.Show(this, $"Service {label} is taking longer than expected and hasn't finished yet. It may still complete on its own — check the Service menu status in a moment.", AppName, MessageBoxButtons.OK, MessageBoxIcon.Warning); else MessageBox.Show(this, $"Service {label} failed (code {rc}).", AppName, MessageBoxButtons.OK, MessageBoxIcon.Warning); } diff --git a/src/RemSound.App/Program.cs b/src/RemSound.App/Program.cs index 80cbbfb..82fed46 100644 --- a/src/RemSound.App/Program.cs +++ b/src/RemSound.App/Program.cs @@ -83,8 +83,13 @@ internal static class Program // normal launch never touches that assembly. if (args.Length > 0 && IsServiceInvocation(args)) { - Environment.ExitCode = ServiceEntry.Dispatch(args); - return; + var serviceRc = ServiceEntry.Dispatch(args); + // Terminate decisively. These verbs run in an elevated helper the app is waiting on; if any + // library it touched left a non-background thread alive, a plain return would leave the process + // lingering and the app blocked on it (part of the install-hang, 2026-07-17). Environment.Exit + // guarantees the helper dies once its one-shot work is done. (--run-service returns here only + // after the SCM has stopped the service, so exiting then is correct too.) + Environment.Exit(serviceRc); } // --config-dir (test / portable isolation): redirect ALL user state - config, diff --git a/src/RemSound.App/SelfTest.cs b/src/RemSound.App/SelfTest.cs index 7bbbeeb..cfd8b96 100644 --- a/src/RemSound.App/SelfTest.cs +++ b/src/RemSound.App/SelfTest.cs @@ -64,6 +64,7 @@ internal static class SelfTest RunStep(results, "Lifecycle churn (modes, sources, pan/EQ, send/receive)", LifecycleChurn); RunStep(results, "Service app-yield token", ServiceInteractivePresence); RunStep(results, "Service sender parity (crypto + Opus frame)", ServiceSenderParity); + RunStep(results, "Elevated helper: no pipe deadlock on flooded output", ServiceProcessCaptureNoDeadlock); RunStep(results, "Default-output follower (service follows Windows default)", DefaultOutputFollower); RunStep(results, "Default follower exclusivity (locks out specific cards)", DefaultFollowerExclusivity); RunStep(results, "Service profile isolation (location + hidden from pickers)", ServiceProfileIsolation); @@ -944,6 +945,20 @@ internal static class SelfTest return "follower flagged + sentinel shared with the app; service resolves it to the live default render endpoint"; } + /// Reproduces the install-hang condition and proves it's fixed: a child that floods BOTH + /// stdout and stderr far past the ~4 KB pipe buffer (a big directory listing plus a failing dir). The + /// old "read stderr to end, then stdout" order deadlocked exactly here (icacls /T over the 100-file + /// service bin); RunProcessCaptured drains both pipes concurrently and must return promptly, in full. + private static string? ServiceProcessCaptureNoDeadlock() + { + var r = ServiceControl.RunProcessCaptured("cmd.exe", + "/c dir \"%SystemRoot%\\System32\" & dir \"%SystemRoot%\\__no_such_dir_remsound_test__\"", 20000); + Check(r.Started, "the test child process must launch"); + Check(r.Exited, "RunProcessCaptured must NOT hang on a child whose output overflows the pipe buffer"); + Check(r.StdOut.Length > 4096, $"the full flooded stdout must be captured, past the pipe buffer (got {r.StdOut.Length} bytes)"); + return $"drained {r.StdOut.Length} bytes stdout + {r.StdErr.Length} stderr concurrently, no deadlock"; + } + private static string? ServiceSenderParity() { // The profile deliberately carries the WRONG audio transport (raw PCM, broadcast frame, Standard diff --git a/src/RemSound.App/ServiceControl.cs b/src/RemSound.App/ServiceControl.cs index b0cd052..0c5a52f 100644 --- a/src/RemSound.App/ServiceControl.cs +++ b/src/RemSound.App/ServiceControl.cs @@ -62,8 +62,20 @@ public static class ServiceControl // ---- UI-side (unprivileged): re-launch self elevated to do the work -------------------------- - /// Re-launch this exe elevated with , wait, and return its exit code - /// (0 = success). Returns -1 if the user declined the UAC prompt or elevation failed. + /// Returned by when the elevated helper didn't finish within the + /// time limit (it hung, or the user left the UAC prompt sitting). Distinct from -1 (declined/failed). + public const int ElevatedTimedOut = -2; + + /// How long to wait for an elevated one-shot verb to finish. Generous: it covers the UAC + /// prompt, the file copy into the service bin, and the SCM calls. If it's exceeded, the helper is + /// assumed stuck and we stop waiting rather than block the caller forever. + private const int ElevatedTimeoutMs = 120000; + + /// Re-launch this exe elevated with , wait (bounded), and return its + /// exit code (0 = success). Returns -1 if the user declined the UAC prompt or elevation failed, or + /// if it didn't finish in time. NEVER waits forever — a stuck helper + /// must not be able to freeze the caller (that was the install-hang, 2026-07-17). Best called off the + /// UI thread so even the bounded wait can't stall the window or its audio. public static int RunElevated(string verb) { var exe = Environment.ProcessPath; @@ -80,7 +92,7 @@ public static class ServiceControl { using var p = Process.Start(psi); if (p is null) return -1; - p.WaitForExit(); + if (!p.WaitForExit(ElevatedTimeoutMs)) return ElevatedTimedOut; return p.ExitCode; } catch (Win32Exception) { return -1; } // user cancelled the UAC prompt @@ -144,28 +156,16 @@ public static class ServiceControl /// stopped service's binaries can be refreshed without administrator rights. Best-effort. private static void GrantUsersWriteToBin() { - try - { - // Grant the installing user (see InstallingUserSid) Modify. (OI)(CI) = inherit to files + - // subfolders; (M) = Modify. /T applies to the existing contents too (the bin was just - // populated), /C keeps going past any single-file error. Capture stderr so a real failure is - // logged rather than swallowed. - var psi = new ProcessStartInfo - { - FileName = "icacls.exe", - Arguments = $"\"{ServiceStore.BinDirectory}\" /grant \"*{InstallingUserSid()}:(OI)(CI)(M)\" /T /C", - UseShellExecute = false, - CreateNoWindow = true, - RedirectStandardOutput = true, - RedirectStandardError = true, - }; - using var p = Process.Start(psi); - var err = p?.StandardError.ReadToEnd(); - var outp = p?.StandardOutput.ReadToEnd(); - p?.WaitForExit(20000); - if (p is { ExitCode: not 0 }) ServiceStore.AppendServiceEvent($"install: icacls grant-write on bin returned {p.ExitCode}: {err}{outp}"); - } - catch (Exception ex) { ServiceStore.AppendServiceEvent($"install: grant-write on bin failed: {ex.GetType().Name}: {ex.Message}"); } + // Grant the installing user (see InstallingUserSid) Modify. (OI)(CI) = inherit to files + + // subfolders; (M) = Modify. /T applies to the existing contents too (the bin was just populated), + // /C keeps going past any single-file error. Runs through RunProcessCaptured, which drains both + // pipes concurrently — icacls /T over 100+ files emits far more than the pipe buffer holds, and the + // old read-stderr-then-stdout order deadlocked here (the install hang Ed hit, 2026-07-17). + var r = RunProcessCaptured("icacls.exe", + $"\"{ServiceStore.BinDirectory}\" /grant \"*{InstallingUserSid()}:(OI)(CI)(M)\" /T /C", 30000); + if (!r.Started) ServiceStore.AppendServiceEvent("install: grant-write on bin failed to launch icacls"); + else if (!r.Exited) ServiceStore.AppendServiceEvent("install: grant-write on bin timed out (icacls killed)"); + else if (r.ExitCode != 0) ServiceStore.AppendServiceEvent($"install: icacls grant-write on bin returned {r.ExitCode}: {r.StdErr}{r.StdOut}"); } /// Copies the program files from to , @@ -281,34 +281,30 @@ public static class ServiceControl private static int RunSc(string arguments) { - try - { - var psi = new ProcessStartInfo - { - FileName = "sc.exe", - Arguments = arguments, - UseShellExecute = false, - CreateNoWindow = true, - RedirectStandardOutput = true, - RedirectStandardError = true, - }; - using var p = Process.Start(psi); - if (p is null) return 2; - p.WaitForExit(20000); - return p.HasExited ? p.ExitCode : 3; - } - catch { return 4; } + var r = RunProcessCaptured("sc.exe", arguments, 20000); + return r.Started ? (r.Exited ? r.ExitCode : 3) : 4; } /// Runs sc.exe and returns its stdout (empty on failure). Used to read the service's security /// descriptor (sdshow) before amending it. - private static string RunScCapture(string arguments) + private static string RunScCapture(string arguments) => RunProcessCaptured("sc.exe", arguments, 20000).StdOut; + + internal readonly record struct ProcResult(bool Started, bool Exited, int ExitCode, string StdOut, string StdErr); + + /// Run a console tool and capture its output SAFELY. Both stdout and stderr are drained + /// CONCURRENTLY (async) while the process runs, then bounded by . This is + /// the fix for a real hang: reading one pipe to end before the other deadlocks whenever the child's + /// output exceeds the ~4 KB pipe buffer — e.g. icacls /T over the service bin's 100+ files + /// blocked writing stdout while we blocked reading stderr, hanging the elevated installer forever (and + /// with it the app that was waiting on it). On timeout the child is killed (whole tree) so nothing is + /// left stuck. Never throws. + internal static ProcResult RunProcessCaptured(string fileName, string arguments, int timeoutMs) { try { var psi = new ProcessStartInfo { - FileName = "sc.exe", + FileName = fileName, Arguments = arguments, UseShellExecute = false, CreateNoWindow = true, @@ -316,12 +312,22 @@ public static class ServiceControl RedirectStandardError = true, }; using var p = Process.Start(psi); - if (p is null) return ""; - var stdout = p.StandardOutput.ReadToEnd(); - p.WaitForExit(20000); - return stdout; + if (p is null) return new ProcResult(false, false, -1, "", ""); + var outTask = p.StandardOutput.ReadToEndAsync(); + var errTask = p.StandardError.ReadToEndAsync(); + if (!p.WaitForExit(timeoutMs)) + { + try { p.Kill(entireProcessTree: true); } catch { /* best-effort */ } + return new ProcResult(true, false, -1, Drain(outTask), Drain(errTask)); + } + return new ProcResult(true, true, p.ExitCode, Drain(outTask), Drain(errTask)); + } + catch { return new ProcResult(false, false, -1, "", ""); } + + static string Drain(Task t) + { + try { return t.Wait(2000) ? t.Result : ""; } catch { return ""; } } - catch { return ""; } } /// Builds the exact sc.exe "create" argument string for a given exe path. Pure and