From dd0f0e30b2285d408285ac88d1dcd39df9e7cf6e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 21:27:17 +0000 Subject: [PATCH] Bound the stderr drain when an inhibitor helper exits at startup [patch] ChildProcessHold.Start waited for the helper's redirected streams with an untimed WaitForExit after the helper had exited. A background child that inherited the helper's stderr kept the pipe open for as long as it ran, so Start (inside the blocker's and controller's locks) hung with it. Wait for stderr's end of input for at most one second instead, then report whatever stderr arrived by then. Fixes ktsu-dev/NoSleep#43 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW5nyEDqgqjU6AtLLLi --- NoSleep.Test/ChildProcessHoldTests.cs | 23 +++++++++++++++++++ NoSleep/Platforms/ChildProcessHold.cs | 32 ++++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/NoSleep.Test/ChildProcessHoldTests.cs b/NoSleep.Test/ChildProcessHoldTests.cs index b78df33..834a125 100644 --- a/NoSleep.Test/ChildProcessHoldTests.cs +++ b/NoSleep.Test/ChildProcessHoldTests.cs @@ -106,6 +106,29 @@ public void Start_WhenTheHelperFloodsStandardError_KeepsOnlyTheLastOfItInTheMess Assert.IsLessThan(4096 + 200, exception.Message.Length, "The message should stay bounded"); } + [TestMethod] + public void Start_WhenTheHelperExitsLeavingAChildOnItsStandardError_ThrowsWithoutWaitingForTheChild() + { + if (!IsUnix) + { + Assert.Inconclusive("The helper paths used by this test only exist on Unix."); + return; + } + + // The background sleep inherits the helper's standard error, so the pipe stays open for 20 s after the + // helper itself has exited (#43). + const string Script = "echo the-real-reason >&2; sleep 20 & exit 1"; + + Stopwatch stopwatch = Stopwatch.StartNew(); + SleepBlockException exception = Assert.ThrowsExactly( + () => ChildProcessHold.Start(ImmediateExitHelper, ["-c", Script])); + stopwatch.Stop(); + + StringAssert.Contains(exception.Message, "exited immediately with code 1", StringComparison.Ordinal); + StringAssert.Contains(exception.Message, "the-real-reason", StringComparison.Ordinal); + Assert.IsLessThan(TimeSpan.FromSeconds(5), stopwatch.Elapsed, "Start should not wait for the helper's background child"); + } + [TestMethod] public void Start_WithAMissingHelper_Throws() { diff --git a/NoSleep/Platforms/ChildProcessHold.cs b/NoSleep/Platforms/ChildProcessHold.cs index 21afaab..b6b700d 100644 --- a/NoSleep/Platforms/ChildProcessHold.cs +++ b/NoSleep/Platforms/ChildProcessHold.cs @@ -9,6 +9,7 @@ namespace ktsu.NoSleep.Platforms; using System.Globalization; using System.IO; using System.Threading; +using System.Threading.Tasks; /// /// A sleep inhibitor that exists for as long as a helper process stays alive. @@ -27,6 +28,16 @@ public sealed class ChildProcessHold : IDisposable private static readonly TimeSpan StartupGrace = TimeSpan.FromMilliseconds(250); private static readonly TimeSpan GracefulExitTimeout = TimeSpan.FromSeconds(2); + /// + /// How long to wait, once a helper has exited, for the rest of its standard error to arrive. + /// + /// + /// The pipe reaches end of input only when every process holding its write end has closed it. A helper + /// that started a background child before exiting leaves that child holding the pipe, so waiting for end + /// of input without a deadline would wait for as long as the child runs. + /// + private static readonly TimeSpan ExitDrainTimeout = TimeSpan.FromSeconds(1); + private readonly Process process; private bool disposed; @@ -96,9 +107,10 @@ public static ChildProcessHold Start(string fileName, IEnumerable argume if (process.WaitForExit(GetMilliseconds(StartupGrace))) { // The timed wait returns as soon as the process ends, possibly before its last lines of standard - // error have been read. The untimed wait also waits for both streams to reach end of input, which - // they do promptly now that the helper is gone. - process.WaitForExit(); + // error have been read. Wait a bounded time for the stream to reach end of input, rather than with + // the untimed WaitForExit: a background child the helper left behind can hold the pipe open for as + // long as it runs, and this wait happens inside the blocker's and the controller's locks. + _ = startupErrors.WaitForEndOfInput(ExitDrainTimeout); int exitCode = process.ExitCode; string errorOutput = startupErrors.Stop(); process.Dispose(); @@ -180,6 +192,7 @@ private sealed class StartupErrorOutput private readonly Lock gate = new(); private readonly LinkedList lines = new(); + private readonly TaskCompletionSource endOfInput = new(TaskCreationOptions.RunContinuationsAsynchronously); private int length; private bool stopped; @@ -189,6 +202,12 @@ private sealed class StartupErrorOutput /// The line, or at end of input. public void Add(string? line) { + if (line is null) + { + _ = endOfInput.TrySetResult(); + return; + } + if (string.IsNullOrWhiteSpace(line)) { return; @@ -220,6 +239,13 @@ public void Add(string? line) } } + /// + /// Waits for standard error to reach end of input, up to a deadline. + /// + /// The longest time to wait. + /// when end of input arrived; when the deadline passed first. + public bool WaitForEndOfInput(TimeSpan timeout) => endOfInput.Task.Wait(timeout); + /// /// Stops recording and returns what was recorded. ///