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. ///