Fix the service install hang (pipe deadlock) + stop it from ever freezing the app

Ed uninstalled then reinstalled the service from the app and the app "kind of crashed":
its audio froze while the connection stayed alive. The logs told the whole story - the
app logged "install requested" then never "install finished"; the elevated helper
(child of the app) was still running 40+ minutes later, and the app sat blocked on it,
UI thread frozen, so streaming died but the background heartbeat kept ticking.

Root cause - a classic pipe deadlock in the elevated installer. GrantUsersWriteToBin ran
`icacls /T` over the service bin (100+ files) and read STDERR to end, THEN stdout. icacls
floods stdout far past the ~4 KB pipe buffer, so it blocked writing stdout while we blocked
reading stderr - forever. That hung DoInstall, which hung the app waiting on it.

Fixes (root cause + defence in depth, so a stuck helper can never freeze the app again):
- RunProcessCaptured: one safe process runner that drains stdout AND stderr concurrently
  (async), bounded by a timeout, and kills the child (whole tree) if it overruns. RunSc,
  RunScCapture and GrantUsersWriteToBin all go through it now. This kills the deadlock.
- RunElevated now waits with a 120s cap and returns ElevatedTimedOut instead of blocking
  forever.
- ServiceAction runs the elevated helper OFF the UI thread and reports the result back, so
  even a slow/stuck helper can't stall the window or its audio. New "timed out" message.
- Program.cs Environment.Exit()s after a one-shot service verb, so a helper that finished
  its work can never linger (non-background thread) with the app waiting on it.

Test: "Elevated helper: no pipe deadlock on flooded output" - RunProcessCaptured against a
child that floods both pipes with ~260 KB (a big dir listing + a failing dir); it must
return promptly with the full output. The pre-fix order would have hung. Gate 46/46.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Ednunp
2026-07-17 23:56:26 +01:00
co-authored by Claude Opus 4.8
parent 5fc1b5bd75
commit aa5707f8e1
4 changed files with 97 additions and 53 deletions
+15
View File
@@ -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";
}
/// <summary>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.</summary>
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