diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 307ebde0..e189a20a 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -103,26 +103,62 @@ amendment. Immediate workaround (confirmed live): drag-resize the windowed client — resize events rebuild the swapchain (#387) and the retail UI rescales from its 800x600 authored canvas. -## #406 — Launcher records a crashed client as `exited{code:0,reason:"graceful"}` +## #406 — CLOSED: Launcher records a crashed client as `exited{code:0,reason:"graceful"}` -**Status:** OPEN (Campaign CC gate round 1, 2026-08-16) +**Status:** DONE (this commit, 2026-08-16) **Severity:** MEDIUM (diagnosis-misleading, not data-loss) Found while diagnosing #405: the client process died with exit code `0xE0434352` (.NET unhandled exception, stack on stderr), but the launcher's session status stream recorded `{"e":"exited","code":0, -"reason":"graceful"}` — the exact opposite of what happened. Running the -identical binary + session config from a console shows the true nonzero -exit code, so the corruption is in the launcher's session-orchestrator -exit observation (wrong process handle/exit-code read, or a default that -masks the real code), not in the client. §LA1 explicitly promises -`exited{code,reason}` carries the real termination; a launcher that -reports "graceful" for a crash sends any future gate/automation -diagnosis in the wrong direction (it did exactly that this round until -the console repro). Investigate the launcher-side session orchestrator's -exit capture; a test should pin a nonzero-exit child producing -`exited{code:,reason:"crashed"|"failed"}` per the LA contract's -vocabulary. +"reason":"graceful"}` — the exact opposite of what happened. + +Root cause was NOT in the launcher's process supervision (its own +OS-level exit-code read was always correct) — it was in the CLIENT's own +self-report. `GameWindow.Dispose()` (`src/AcDream.App/Rendering/GameWindow.cs`) +runs unconditionally via `Program.cs`'s `using var window = new +GameWindow(...)` even when invoked mid-unwind of an exception that +escaped `Run()`'s Silk.NET frame loop — the resource-shutdown transaction +itself can converge cleanly (nothing it tears down touches the crash), +so `CompleteShutdown` had no way to tell "normal `Run()` return" from "an +exception is propagating through me right now" and always wrote the +hardcoded `exited{code:0,reason:"graceful"}`. Fixed by latching +`_runFailure` in `Run()`'s existing `catch (Exception failure)` block +(right before the `throw;` that already existed for the +`_constructionCleanup.RetainFrom(failure)` ledger) and consulting it from +a new `ReportExited` method that is now the ONE call site for the +terminal status write: `exited{code:1,reason:"crashed"}` when a crash was +observed, `exited{code:0,reason:"graceful"}` on a real graceful +Dispose(), `exited{code:1,reason:"shutdown-incomplete"}` unchanged for a +non-crash teardown failure. `"crashed"` is a new value for the already- +free-text `reason` field (§LA1's `exited{code,reason}` vocabulary pins +the EVENT name, not an enum of `reason` strings — `StatusEventParser` +already round-trips any string there) so no wire-contract amendment was +needed. Pinned as a source-shape test (`GameWindowCrashStatusTests`) since +`GameWindow` cannot be constructed without a live GPU/window. + +Sibling gap fixed in the same commit: the launcher previously discarded +the child's stdout/stderr entirely, which is why diagnosing this exact +crash required a manual console re-run. Added +`BoundedProcessOutputCapture` (`src/AcDream.Launcher.Core/Launching/`) — +a 2 MiB-capped, additive-only sink mirroring `SessionStatusWriter`'s +open-append-flush-close-per-write posture (a long-lived write handle is +NOT actually concurrently readable on Windows even with +`FileShare.Read` — confirmed by isolated repro) — wired into BOTH +`SystemChildProcess` (`ProcessStartInfo.RedirectStandardError` + +`ErrorDataReceived`; used on Linux for every child and on Windows for +graphical/non-console children, i.e. exactly this bug's own App/GUI +scenario) and `WindowsSystemChildProcess` (a real native pipe via a new +`CreateChildOutputPipe`, mirroring the existing stdin pipe in the +opposite direction, drained on a background pump thread; used on Windows +for console-capable children, i.e. Headless). The capture path is opt-in +via a new `LauncherProcessSpec.StderrLogPath` (null = behave exactly as +before) threaded through `SessionConfigComposer` → `client.err.log` +beside `status.jsonl` in the per-session directory → +`LauncherExecutableSet.CreatePlaySpec`/`CreateProbeSpec` → +`LauncherOrchestrator`. Real end-to-end tests +(`LauncherProcessSupervisorTests`) spawn an actual child via both code +paths and assert the captured file. ## #405 — CLOSED: chargen/summary preview leases missing Transfer killed every retail-UI window load diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 4a0bc6aa..50cc1d79 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -137,6 +137,14 @@ public sealed class GameWindow : _constructionCleanup = new(); private readonly AcDream.App.World.WorldEnvironmentController _worldEnvironment; private readonly GameWindowLifetime _lifetime = new(); + // fix #406: set by Run()'s own catch the instant an exception escapes + // the Silk.NET frame loop, BEFORE it is rethrown and unwinds through + // Program.cs's `using var window = ...` (which calls Dispose() — + // therefore CompleteShutdown() — while that exception is still in + // flight). CompleteShutdown consults this so a crash is never reported + // as the hardcoded "exited{code:0,reason:graceful}" the resource + // teardown transaction's own convergence would otherwise imply. + private Exception? _runFailure; private readonly DisplayFramePacingController _displayFramePacing; private readonly RuntimeSettingsController _runtimeSettings; @@ -820,6 +828,10 @@ public sealed class GameWindow : catch (Exception failure) { _constructionCleanup.RetainFrom(failure); + // fix #406: latch BEFORE rethrowing — Dispose() (and therefore + // CompleteShutdown) can run mid-unwind of this exact exception, + // via Program.cs's `using var window = ...`. + _runFailure = failure; throw; } } @@ -1698,7 +1710,7 @@ public sealed class GameWindow : // OnClosing() native-window-close-request pass) represents the // process actually being done. if (releaseNativeWindow) - _statusWriter.Exited(_options.SessionId ?? "app", 0, "graceful"); + ReportExited(report); return; } @@ -1715,12 +1727,41 @@ public sealed class GameWindow : Console.Error.WriteLine($"[shutdown] {report.Error}"); if (releaseNativeWindow) + ReportExited(report); + } + + /// + /// Writes the ONE terminal "exited" status event for this session + /// (fix #406). A resource-shutdown transaction can converge cleanly + /// ('s own + /// says nothing about this) even though this call + /// is running mid-unwind of an exception that escaped + /// 's frame loop and is about to terminate the process + /// via the CLR's unhandled-exception path — + /// is the one signal that actually distinguishes those two cases. + /// Before this fix every such crash wrote the exact same + /// "exited{code:0,reason:graceful}" as a real graceful shutdown, + /// sending any launcher-side diagnosis in the wrong direction (#406). + /// + private void ReportExited(GameWindowLifetimeReport report) + { + string sessionId = _options.SessionId ?? "app"; + if (_runFailure is not null) { - _statusWriter.Exited( - _options.SessionId ?? "app", - 1, - "shutdown-incomplete"); + // The real OS-level exit code (e.g. 0xE0434352 on Windows for + // an unhandled .NET exception) is produced by the runtime AFTER + // this method returns and the exception keeps propagating — it + // cannot be predicted from here. "crashed" is the truthful, + // platform-independent classification; the launcher's own + // process supervisor observes the real OS exit code separately. + _statusWriter.Exited(sessionId, 1, "crashed"); + return; } + + if (report.Status == GameWindowLifetimeStatus.Complete) + _statusWriter.Exited(sessionId, 0, "graceful"); + else + _statusWriter.Exited(sessionId, 1, "shutdown-incomplete"); } private GameWindowShutdownRoots CaptureShutdownRoots() => new( diff --git a/src/AcDream.Launcher.Core/Launching/BoundedProcessOutputCapture.cs b/src/AcDream.Launcher.Core/Launching/BoundedProcessOutputCapture.cs new file mode 100644 index 00000000..0803a711 --- /dev/null +++ b/src/AcDream.Launcher.Core/Launching/BoundedProcessOutputCapture.cs @@ -0,0 +1,238 @@ +using System.Text; + +namespace AcDream.Launcher.Core.Launching; + +/// +/// Captures a supervised child's stderr into a per-session file, bounded +/// so a log-spamming (or endlessly crash-looping) child can never fill the +/// disk (fix #406 sibling gap). Before this class existed the launcher +/// discarded a child's stdout/stderr entirely — including the unhandled- +/// exception stack trace a crash writes there — so diagnosing exactly the +/// #406 crash required re-running the identical binary + session config +/// from a console by hand. This is purely additive diagnostics: it does +/// not touch the pinned status.jsonl event vocabulary (Campaign LA +/// plan §LA1) at all. +/// +/// +/// Every write opens the file fresh (), +/// writes its chunk, flushes, and closes — mirroring +/// 's "no long- +/// lived file handle" posture exactly, and for the SAME reason: a +/// long-lived write handle only opened with +/// is NOT actually concurrently readable in practice — Windows' sharing +/// check is bidirectional, and a plain File.ReadAllText-style +/// reader (which itself only requests , not +/// ) fails with a sharing violation +/// against ANY still-open handle that holds write access, regardless of +/// what share flags that writer declared. Opening fresh per write avoids +/// the problem entirely: there is never a handle open except for the +/// duration of one small, synchronous write. +/// +/// +/// +/// Every write is defensively guarded the same way +/// guards its own +/// I/O: a recoverable failure latches this sink into a permanent no-op +/// rather than throwing back into the caller's read-and-forward loop. The +/// launcher's job is to supervise the child, not to go down because a +/// local diagnostics file could not be written. +/// +/// +/// +/// Callers are responsible for continuing to drain the child's stderr +/// stream/pipe even after this sink stops accepting bytes (cap reached or +/// latched off) — this class only bounds what lands on disk, never how +/// much the caller may read. A caller that stopped draining on a full +/// sink could leave the child blocked writing to a full OS pipe buffer. +/// +/// +public sealed class BoundedProcessOutputCapture : IDisposable +{ + /// 2 MiB is generous for the lifecycle/shutdown diagnostics + /// and a crash stack trace this exists to capture, while still being a + /// firm, small bound against a pathological child that spams stderr + /// for an entire long-running headless-bot session. + public const long DefaultMaxBytes = 2 * 1024 * 1024; + + private static readonly byte[] Newline = "\n"u8.ToArray(); + + private readonly string _path; + private readonly long _maxBytes; + private readonly object _gate = new(); + private bool _directoryEnsured; + private long _written; + private bool _capped; + private bool _latchedOff; + private bool _disposed; + + public BoundedProcessOutputCapture(string path, long maxBytes = DefaultMaxBytes) + { + ArgumentException.ThrowIfNullOrWhiteSpace(path); + if (maxBytes <= 0) + { + throw new ArgumentOutOfRangeException( + nameof(maxBytes), + "The bounded capture size must be positive."); + } + + _path = Path.GetFullPath(path); + _maxBytes = maxBytes; + } + + /// True once no further byte will ever be written — either + /// the size cap was reached (a truncation marker was appended) or a + /// local I/O failure latched this sink off. Exposed for tests; a + /// caller never needs to check this before calling + /// — it is always safe to call. + public bool IsDone + { + get + { + lock (_gate) + { + return _capped || _latchedOff || _disposed; + } + } + } + + /// Appends one line of already-decoded text (e.g. one + /// Process.ErrorDataReceived line) followed by a newline. Never + /// throws. A line (the sentinel .NET's + /// ErrorDataReceived raises once when the stream closes) is a + /// silent no-op. + public void AppendLine(string? line) + { + if (line is null) + { + return; + } + + lock (_gate) + { + AppendLocked(Encoding.UTF8.GetBytes(line)); + AppendLocked(Newline); + } + } + + /// Appends a raw decoded chunk (no implied line boundary). + /// Never throws. + public void Append(ReadOnlySpan data) + { + if (data.IsEmpty) + { + return; + } + + lock (_gate) + { + AppendLocked(data); + } + } + + private void AppendLocked(ReadOnlySpan data) + { + if (data.IsEmpty || _disposed || _latchedOff || _capped) + { + return; + } + + try + { + long remaining = _maxBytes - _written; + if (remaining <= 0) + { + CapLocked(); + return; + } + + int toWrite = data.Length > remaining + ? checked((int)remaining) + : data.Length; + WriteChunkLocked(data[..toWrite]); + _written += toWrite; + + if (toWrite < data.Length) + { + CapLocked(); + } + } + catch (Exception error) when (IsRecoverableIoFailure(error)) + { + _latchedOff = true; + } + } + + /// Opens the file fresh, writes one chunk, flushes, and + /// closes — see the class doc for why this never keeps a long-lived + /// handle. Exceptions propagate to the caller's own guard. + private void WriteChunkLocked(ReadOnlySpan chunk) + { + EnsureDirectoryLocked(); + using FileStream stream = new( + _path, + FileMode.Append, + FileAccess.Write, + FileShare.Read); + stream.Write(chunk); + stream.Flush(); + } + + private void EnsureDirectoryLocked() + { + if (_directoryEnsured) + { + return; + } + + string? directory = Path.GetDirectoryName(_path); + if (!string.IsNullOrEmpty(directory)) + { + Directory.CreateDirectory(directory); + } + + _directoryEnsured = true; + } + + /// Writes the one-time truncation marker — every later + /// / becomes a cheap + /// no-op via . + private void CapLocked() + { + if (_capped) + { + return; + } + + _capped = true; + try + { + byte[] marker = Encoding.UTF8.GetBytes( + $"\n[acdream-launcher] client.err.log truncated at {_maxBytes} bytes\n"); + WriteChunkLocked(marker); + } + catch (Exception error) when (IsRecoverableIoFailure(error)) + { + // The marker itself is best-effort — the cap already took + // effect via _capped regardless of whether it could be written. + } + } + + private static bool IsRecoverableIoFailure(Exception error) => + error is IOException + or UnauthorizedAccessException + or NotSupportedException + or System.Security.SecurityException + or DirectoryNotFoundException; + + /// No open handle to release — see the class doc. Marks this + /// sink permanently done so any late-arriving chunk from a caller's + /// still-draining pump is a silent no-op instead of reopening the + /// file after the caller considers capture finished. + public void Dispose() + { + lock (_gate) + { + _disposed = true; + } + } +} diff --git a/src/AcDream.Launcher.Core/Launching/ILauncherChildProcess.cs b/src/AcDream.Launcher.Core/Launching/ILauncherChildProcess.cs index b8816ab7..b3f9a3ce 100644 --- a/src/AcDream.Launcher.Core/Launching/ILauncherChildProcess.cs +++ b/src/AcDream.Launcher.Core/Launching/ILauncherChildProcess.cs @@ -88,7 +88,9 @@ internal sealed partial class SystemChildProcess : ILauncherChildProcess private readonly Process _process; private readonly bool _supportsConsoleGracefulStop; + private readonly BoundedProcessOutputCapture? _stderrCapture; private bool _raisingEnabled; + private bool _errorReadingEnabled; internal SystemChildProcess(LauncherProcessSpec spec) { @@ -102,6 +104,17 @@ internal sealed partial class SystemChildProcess : ILauncherChildProcess UseShellExecute = false, }; + // fix #406 sibling gap: capture stderr (crash stack traces land + // there) into a bounded per-session file instead of discarding it. + // Purely additive — RedirectStandardOutput/CreateNoWindow are left + // untouched, and a spec with no StderrLogPath behaves exactly as + // before. + if (!string.IsNullOrWhiteSpace(spec.StderrLogPath)) + { + startInfo.RedirectStandardError = true; + _stderrCapture = new BoundedProcessOutputCapture(spec.StderrLogPath); + } + foreach (string argument in spec.Arguments) { startInfo.ArgumentList.Add(argument); @@ -128,7 +141,17 @@ internal sealed partial class SystemChildProcess : ILauncherChildProcess _process.EnableRaisingEvents = true; _process.Exited += OnExited; _raisingEnabled = true; + if (_stderrCapture is not null) + { + _process.ErrorDataReceived += OnErrorDataReceived; + _errorReadingEnabled = true; + } + _process.Start(); + if (_errorReadingEnabled) + { + _process.BeginErrorReadLine(); + } } public bool TryRequestGracefulStop() @@ -169,9 +192,18 @@ internal sealed partial class SystemChildProcess : ILauncherChildProcess _process.Exited -= OnExited; } + if (_errorReadingEnabled) + { + _process.ErrorDataReceived -= OnErrorDataReceived; + } + _process.Dispose(); + _stderrCapture?.Dispose(); } private void OnExited(object? sender, EventArgs e) => Exited?.Invoke(this, EventArgs.Empty); + + private void OnErrorDataReceived(object? sender, DataReceivedEventArgs e) => + _stderrCapture?.AppendLine(e.Data); } diff --git a/src/AcDream.Launcher.Core/Launching/LauncherProcessSpec.cs b/src/AcDream.Launcher.Core/Launching/LauncherProcessSpec.cs index 599a932d..9cdac21b 100644 --- a/src/AcDream.Launcher.Core/Launching/LauncherProcessSpec.cs +++ b/src/AcDream.Launcher.Core/Launching/LauncherProcessSpec.cs @@ -11,9 +11,14 @@ namespace AcDream.Launcher.Core.Launching; /// so Windows starts them /// as isolated process-group leaders for targeted CTRL_BREAK_EVENT and /// Linux sends SIGINT; graphical specs leave it false and use WM_CLOSE. +/// is an optional, purely-additive +/// diagnostics sink (fix #406 sibling gap): when set, the launcher +/// captures the child's stderr into a bounded file at that path instead +/// of discarding it; when null, behavior is exactly as before. /// public sealed record LauncherProcessSpec( string ExecutablePath, IReadOnlyList Arguments, string? WorkingDirectory = null, - bool SupportsConsoleGracefulStop = true); + bool SupportsConsoleGracefulStop = true, + string? StderrLogPath = null); diff --git a/src/AcDream.Launcher.Core/Launching/SessionConfigComposer.cs b/src/AcDream.Launcher.Core/Launching/SessionConfigComposer.cs index 1fceee5c..7d9eb594 100644 --- a/src/AcDream.Launcher.Core/Launching/SessionConfigComposer.cs +++ b/src/AcDream.Launcher.Core/Launching/SessionConfigComposer.cs @@ -5,12 +5,17 @@ using AcDream.Platform; namespace AcDream.Launcher.Core.Launching; -/// The composed session-config document plus the two per-launch -/// paths derived from the session id, per Campaign LA spec §6. +/// The composed session-config document plus the per-launch +/// paths derived from the session id, per Campaign LA spec §6. +/// is launcher-internal (fix #406 sibling +/// gap) — it never appears in the written session.json, only in +/// the the launcher spawns the +/// child with. public sealed record ComposedSessionConfig( string SessionId, string ConfigFilePath, string StatusFilePath, + string StderrLogPath, SessionConfigDocument Document); /// @@ -110,7 +115,8 @@ public static class SessionConfigComposer ArgumentNullException.ThrowIfNull(paths); ArgumentException.ThrowIfNullOrWhiteSpace(sessionId); - (string configFilePath, string statusFilePath) = BuildSessionPaths(paths, sessionId); + (string configFilePath, string statusFilePath, string stderrLogPath) = + BuildSessionPaths(paths, sessionId); SessionCharacterSelector? selector = character.LaunchMode == LaunchMode.GuiSelect ? null @@ -159,6 +165,7 @@ public static class SessionConfigComposer sessionId, configFilePath, statusFilePath, + stderrLogPath, document); } @@ -184,7 +191,8 @@ public static class SessionConfigComposer ArgumentNullException.ThrowIfNull(paths); ArgumentException.ThrowIfNullOrWhiteSpace(sessionId); - (string configFilePath, string statusFilePath) = BuildSessionPaths(paths, sessionId); + (string configFilePath, string statusFilePath, string stderrLogPath) = + BuildSessionPaths(paths, sessionId); var descriptor = new SessionDescriptor { @@ -222,6 +230,7 @@ public static class SessionConfigComposer sessionId, configFilePath, statusFilePath, + stderrLogPath, document); } @@ -282,9 +291,10 @@ public static class SessionConfigComposer public static string Serialize(SessionConfigDocument document) => JsonSerializer.Serialize(document, SerializerOptions); - private static (string ConfigFilePath, string StatusFilePath) BuildSessionPaths( - ApplicationPathSet paths, - string sessionId) + private static (string ConfigFilePath, string StatusFilePath, string StderrLogPath) + BuildSessionPaths( + ApplicationPathSet paths, + string sessionId) { string sessionDirectory = Path.Combine( paths.CacheDirectory, @@ -294,7 +304,10 @@ public static class SessionConfigComposer return ( Path.Combine(sessionDirectory, "session.json"), - Path.Combine(sessionDirectory, "status.jsonl")); + Path.Combine(sessionDirectory, "status.jsonl"), + // fix #406 sibling gap: lives beside status.jsonl in the same + // per-session directory. + Path.Combine(sessionDirectory, "client.err.log")); } private static SessionCharacterSelector BuildSelector(CharacterProfile character) diff --git a/src/AcDream.Launcher.Core/Launching/WindowsSystemChildProcess.cs b/src/AcDream.Launcher.Core/Launching/WindowsSystemChildProcess.cs index 00138c30..335f58cb 100644 --- a/src/AcDream.Launcher.Core/Launching/WindowsSystemChildProcess.cs +++ b/src/AcDream.Launcher.Core/Launching/WindowsSystemChildProcess.cs @@ -20,6 +20,10 @@ internal sealed class WindowsSystemChildProcess : ILauncherChildProcess private TextWriter? _standardInput; private int _processGroupId; private bool _raisingEnabled; + // fix #406 sibling gap: null unless _spec.StderrLogPath was set. + private BoundedProcessOutputCapture? _stderrCapture; + private FileStream? _stderrReadStream; + private Thread? _stderrPumpThread; internal WindowsSystemChildProcess( LauncherProcessSpec spec, @@ -54,6 +58,30 @@ internal sealed class WindowsSystemChildProcess : ILauncherChildProcess _raisingEnabled = true; _standardInput = started.TakeStandardInput(); _processGroupId = started.ProcessId; + if (!string.IsNullOrWhiteSpace(_spec.StderrLogPath)) + { + // fix #406 sibling gap: drain the real stderr pipe + // WindowsProcessNative.StartCore created for this spec into + // a bounded file, BEFORE resuming the suspended child below + // — the pump is already running by the time the child can + // write anything. + SafeFileHandle stderrRead = started.TakeStandardErrorRead() + ?? throw new InvalidOperationException( + "The launcher child stderr pipe was not created."); + _stderrCapture = new BoundedProcessOutputCapture(_spec.StderrLogPath); + _stderrReadStream = new FileStream( + stderrRead, + FileAccess.Read, + 4096, + isAsync: false); + _stderrPumpThread = new Thread(PumpStderr) + { + IsBackground = true, + Name = "acdream-launcher-stderr-pump", + }; + _stderrPumpThread.Start(); + } + started.Resume(); } catch @@ -61,6 +89,10 @@ internal sealed class WindowsSystemChildProcess : ILauncherChildProcess started.Terminate(); _standardInput?.Dispose(); _standardInput = null; + _stderrReadStream?.Dispose(); + _stderrReadStream = null; + _stderrCapture?.Dispose(); + _stderrCapture = null; if (_process is not null) { if (_raisingEnabled) @@ -80,6 +112,43 @@ internal sealed class WindowsSystemChildProcess : ILauncherChildProcess } } + /// + /// Runs on a dedicated background thread for the lifetime of the + /// capture (fix #406 sibling gap): continuously drains the child's + /// stderr pipe into so the child's writes + /// never block on a full OS pipe buffer, even after the capture sink + /// itself has stopped accepting bytes (size cap reached, or a local + /// I/O failure latched it off — + /// never throws). Returns cleanly once the pipe's write end closes + /// (the child exited) or closes the read end. + /// + private void PumpStderr() + { + FileStream? stream = _stderrReadStream; + BoundedProcessOutputCapture? capture = _stderrCapture; + if (stream is null || capture is null) + { + return; + } + + byte[] buffer = new byte[4096]; + try + { + int read; + while ((read = stream.Read(buffer, 0, buffer.Length)) > 0) + { + capture.Append(buffer.AsSpan(0, read)); + } + } + catch (Exception error) + when (error is IOException or ObjectDisposedException) + { + // The pipe's write end closed (the child exited) or Dispose() + // released the read end concurrently — either way, this pump + // is simply done; never propagate onto this background thread. + } + } + public bool TryRequestGracefulStop() { try @@ -111,6 +180,21 @@ internal sealed class WindowsSystemChildProcess : ILauncherChildProcess { _standardInput?.Dispose(); _standardInput = null; + if (_stderrReadStream is not null) + { + // Closing the read end unblocks PumpStderr's pending Read() + // (ObjectDisposedException, caught there). The bounded Join + // lets that last in-flight chunk land in the capture file + // before it is disposed below, without letting a wedged pump + // thread ever hang this Dispose() call. + _stderrReadStream.Dispose(); + _stderrReadStream = null; + _stderrPumpThread?.Join(TimeSpan.FromSeconds(2)); + _stderrPumpThread = null; + } + + _stderrCapture?.Dispose(); + _stderrCapture = null; if (_process is not null) { if (_raisingEnabled) @@ -225,18 +309,21 @@ internal sealed class WindowsProcessStartResult : IDisposable private readonly SafeKernelHandle _processHandle; private readonly SafeKernelHandle _threadHandle; private SafeFileHandle? _standardInput; + private SafeFileHandle? _stderrRead; private bool _resumed; internal WindowsProcessStartResult( int processId, SafeKernelHandle processHandle, SafeKernelHandle threadHandle, - SafeFileHandle standardInput) + SafeFileHandle standardInput, + SafeFileHandle? stderrRead = null) { ProcessId = processId; _processHandle = processHandle; _threadHandle = threadHandle; _standardInput = standardInput; + _stderrRead = stderrRead; } internal int ProcessId { get; } @@ -263,6 +350,22 @@ internal sealed class WindowsProcessStartResult : IDisposable } } + /// + /// Transfers ownership of the parent-side stderr pipe read handle (fix + /// #406 sibling gap) — non-null only when + /// was set, in which case + /// created a real pipe for the child's stderr instead of the usual + /// duplicate-or-NUL handle. Returns null if capture was not requested, + /// or if this handle was already claimed. The caller owns disposal + /// after this call. + /// + internal SafeFileHandle? TakeStandardErrorRead() + { + SafeFileHandle? handle = _stderrRead; + _stderrRead = null; + return handle; + } + internal void Resume() { if (WindowsProcessNative.ResumeThread(_threadHandle) == uint.MaxValue) @@ -290,6 +393,7 @@ internal sealed class WindowsProcessStartResult : IDisposable } _standardInput?.Dispose(); + _stderrRead?.Dispose(); _threadHandle.Dispose(); _processHandle.Dispose(); } @@ -361,13 +465,20 @@ internal static class WindowsProcessNative private static WindowsProcessStartResult StartCore(LauncherProcessSpec spec) { SafeFileHandle? parentInput = null; + // fix #406 sibling gap: when the spec requests stderr capture, the + // child's stderr handle is a real pipe (this parent-side read end) + // instead of the usual duplicate-or-NUL handle below. + SafeFileHandle? parentStderrRead = null; + SafeHandle? childError = null; try { using SafeFileHandle childInput = CreateChildInputPipe( out SafeFileHandle createdParentInput); parentInput = createdParentInput; using SafeKernelHandle childOutput = DuplicateOrOpenNull(StdOutputHandle); - using SafeKernelHandle childError = DuplicateOrOpenNull(StdErrorHandle); + childError = string.IsNullOrWhiteSpace(spec.StderrLogPath) + ? DuplicateOrOpenNull(StdErrorHandle) + : CreateChildOutputPipe(out parentStderrRead); using var attributes = new ProcessThreadAttributeList( childInput.DangerousGetHandle(), childOutput.DangerousGetHandle(), @@ -420,8 +531,10 @@ internal static class WindowsProcessNative checked((int)information.ProcessId), processHandle, threadHandle, - parentInput); + parentInput, + parentStderrRead); parentInput = null; + parentStderrRead = null; return result; } catch @@ -435,6 +548,8 @@ internal static class WindowsProcessNative finally { parentInput?.Dispose(); + parentStderrRead?.Dispose(); + childError?.Dispose(); } } @@ -525,6 +640,45 @@ internal static class WindowsProcessNative return child; } + /// + /// Mirror of with the roles + /// reversed (fix #406 sibling gap): the CHILD gets the pipe's WRITE + /// end (its stderr handle, inheritable across + /// ), the PARENT keeps the READ end + /// (inherit flag cleared, exactly like 's + /// counterpart on the stdin pipe) so the launcher can drain the + /// child's stderr into a bounded file. + /// + private static SafeFileHandle CreateChildOutputPipe(out SafeFileHandle parentRead) + { + var security = new SecurityAttributes + { + Length = Marshal.SizeOf(), + InheritHandle = true, + }; + if (!CreatePipe(out IntPtr read, out IntPtr write, ref security, 0)) + { + throw new Win32Exception(Marshal.GetLastWin32Error(), + "The launcher child stderr pipe could not be created."); + } + + var child = new SafeFileHandle(write, ownsHandle: true); + parentRead = new SafeFileHandle(read, ownsHandle: true); + if (!SetHandleInformation( + parentRead, + HandleFlagInherit, + 0)) + { + int error = Marshal.GetLastWin32Error(); + child.Dispose(); + parentRead.Dispose(); + throw new Win32Exception(error, + "The launcher child stderr pipe could not be isolated."); + } + + return child; + } + private static SafeKernelHandle DuplicateOrOpenNull(int standardHandle) { IntPtr source = GetStdHandle(standardHandle); diff --git a/src/AcDream.Launcher.Core/Orchestration/LauncherExecutableSet.cs b/src/AcDream.Launcher.Core/Orchestration/LauncherExecutableSet.cs index 5a98b795..4add2816 100644 --- a/src/AcDream.Launcher.Core/Orchestration/LauncherExecutableSet.cs +++ b/src/AcDream.Launcher.Core/Orchestration/LauncherExecutableSet.cs @@ -91,7 +91,8 @@ public sealed class LauncherExecutableSet public LauncherProcessSpec CreatePlaySpec( LaunchMode mode, - string configFilePath) + string configFilePath, + string? stderrLogPath = null) { ArgumentException.ThrowIfNullOrWhiteSpace(configFilePath); ExecutablePaths paths = RequireAvailable(mode); @@ -100,22 +101,27 @@ public sealed class LauncherExecutableSet ? new LauncherProcessSpec( paths.HeadlessHostPath, ["--config", configFilePath], - paths.WorkingDirectory) + paths.WorkingDirectory, + StderrLogPath: stderrLogPath) : new LauncherProcessSpec( paths.GraphicalHostPath, ["--session-config", configFilePath], paths.WorkingDirectory, - SupportsConsoleGracefulStop: false); + SupportsConsoleGracefulStop: false, + StderrLogPath: stderrLogPath); } - public LauncherProcessSpec CreateProbeSpec(string configFilePath) + public LauncherProcessSpec CreateProbeSpec( + string configFilePath, + string? stderrLogPath = null) { ArgumentException.ThrowIfNullOrWhiteSpace(configFilePath); ExecutablePaths paths = RequireAvailable(LaunchMode.Headless); return new LauncherProcessSpec( paths.HeadlessHostPath, ["--config", configFilePath], - paths.WorkingDirectory); + paths.WorkingDirectory, + StderrLogPath: stderrLogPath); } public static LauncherExecutableSet FromDirectory(string directory) diff --git a/src/AcDream.Launcher.Core/Orchestration/LauncherOrchestrator.cs b/src/AcDream.Launcher.Core/Orchestration/LauncherOrchestrator.cs index a734fcaf..9bb48560 100644 --- a/src/AcDream.Launcher.Core/Orchestration/LauncherOrchestrator.cs +++ b/src/AcDream.Launcher.Core/Orchestration/LauncherOrchestrator.cs @@ -683,10 +683,13 @@ public sealed class LauncherOrchestrator : ILauncherOrchestrator request.Cancellation.Token.ThrowIfCancellationRequested(); LauncherProcessSpec processSpec = request.IsProbe - ? _executables.CreateProbeSpec(composed.ConfigFilePath) + ? _executables.CreateProbeSpec( + composed.ConfigFilePath, + composed.StderrLogPath) : _executables.CreatePlaySpec( request.Activity.LaunchMode!.Value, - composed.ConfigFilePath); + composed.ConfigFilePath, + composed.StderrLogPath); supervisor.Start(processSpec, password); hostStarted = true; diff --git a/tests/AcDream.App.Tests/Rendering/GameWindowCrashStatusTests.cs b/tests/AcDream.App.Tests/Rendering/GameWindowCrashStatusTests.cs new file mode 100644 index 00000000..779aa6d8 --- /dev/null +++ b/tests/AcDream.App.Tests/Rendering/GameWindowCrashStatusTests.cs @@ -0,0 +1,139 @@ +namespace AcDream.App.Tests.Rendering; + +/// +/// Fix #406: before this change, GameWindow.CompleteShutdown wrote +/// a hardcoded exited{code:0,reason:"graceful"} status event +/// whenever the resource-shutdown transaction converged — even when +/// Dispose() (and therefore CompleteShutdown) ran mid-unwind +/// of an exception that escaped Run()'s Silk.NET frame loop and was +/// about to crash the process via the CLR's unhandled-exception path. +/// GameWindow cannot be constructed without a live GPU/window (see +/// the established pattern in GameWindowSlice8BoundaryTests), so +/// this pins the fix as a source-shape test exactly like that file does +/// for the surrounding shutdown machinery. +/// +public sealed class GameWindowCrashStatusTests +{ + [Fact] + public void Run_LatchesRunFailureBeforeRethrowingFromTheFrameLoopCatch() + { + string body = MethodBody( + "public void Run()", + "void IGameWindowPlatformPublication.PublishGraphics("); + string tryBlock = Slice(body, "try\n {\n _window.Run();", "}\n }"); + + AssertAppearsInOrder( + tryBlock, + "_window.Run();", + "catch (Exception failure)", + "_constructionCleanup.RetainFrom(failure);", + // The latch MUST happen before the rethrow: Dispose() (and + // therefore CompleteShutdown/ReportExited) can run mid-unwind + // of this exact exception, via Program.cs's + // `using var window = ...`. + "_runFailure = failure;", + "throw;"); + } + + [Fact] + public void ReportExited_ChecksRunFailureBeforeEitherGracefulOrShutdownIncompletePaths() + { + string source = GameWindowSource(); + string reportExited = Slice( + source, + "private void ReportExited(GameWindowLifetimeReport report)", + "\n }\n"); + + Assert.Contains( + "string sessionId = _options.SessionId ?? \"app\";", + reportExited, + StringComparison.Ordinal); + AssertAppearsInOrder( + reportExited, + "if (_runFailure is not null)", + "_statusWriter.Exited(sessionId, 1, \"crashed\");", + "return;", + "if (report.Status == GameWindowLifetimeStatus.Complete)", + "_statusWriter.Exited(sessionId, 0, \"graceful\");", + "_statusWriter.Exited(sessionId, 1, \"shutdown-incomplete\");"); + + // Every terminal-status write in CompleteShutdown funnels through + // this ONE method — a second, uncoordinated call site would be + // exactly how the pre-fix bug reappears. + Assert.Equal( + 2, + CountOccurrences(source, "ReportExited(report)")); + Assert.DoesNotContain( + "_statusWriter.Exited(_options.SessionId ?? \"app\", 0, \"graceful\")", + source, + StringComparison.Ordinal); + } + + [Fact] + public void RunFailureFieldExistsAndDefaultsToNull() + { + string source = GameWindowSource(); + + Assert.Contains( + "private Exception? _runFailure;", + source, + StringComparison.Ordinal); + } + + private static string MethodBody(string start, string end) => + Slice(GameWindowSource(), start, end); + + private static string Slice(string source, string start, string end) + { + int first = source.IndexOf(start, StringComparison.Ordinal); + int last = source.IndexOf(end, first + 1, StringComparison.Ordinal); + Assert.True(first >= 0, $"Missing source boundary: {start}"); + Assert.True(last > first, $"Missing source boundary: {end}"); + return source[first..last]; + } + + private static int CountOccurrences(string source, string value) + { + int count = 0; + int cursor = 0; + while ((cursor = source.IndexOf(value, cursor, StringComparison.Ordinal)) >= 0) + { + count++; + cursor += value.Length; + } + + return count; + } + + private static void AssertAppearsInOrder(string source, params string[] fragments) + { + int cursor = -1; + foreach (string fragment in fragments) + { + int next = source.IndexOf(fragment, cursor + 1, StringComparison.Ordinal); + Assert.True(next >= 0, $"Missing expected source fragment: {fragment}"); + Assert.True(next > cursor, $"Out-of-order source fragment: {fragment}"); + cursor = next; + } + } + + private static string GameWindowSource() => File.ReadAllText(Path.Combine( + FindRepoRoot(), + "src", + "AcDream.App", + "Rendering", + "GameWindow.cs")).Replace("\r\n", "\n", StringComparison.Ordinal); + + private static string FindRepoRoot() + { + DirectoryInfo? directory = new(AppContext.BaseDirectory); + while (directory is not null) + { + if (File.Exists(Path.Combine(directory.FullName, "AcDream.slnx"))) + return directory.FullName; + directory = directory.Parent; + } + + throw new DirectoryNotFoundException("Could not find AcDream.slnx."); + } +} diff --git a/tests/AcDream.Launcher.Core.Tests.Fixtures.ConsoleSignalChild/Program.cs b/tests/AcDream.Launcher.Core.Tests.Fixtures.ConsoleSignalChild/Program.cs index dc047cdf..092616a0 100644 --- a/tests/AcDream.Launcher.Core.Tests.Fixtures.ConsoleSignalChild/Program.cs +++ b/tests/AcDream.Launcher.Core.Tests.Fixtures.ConsoleSignalChild/Program.cs @@ -1,5 +1,34 @@ using System.Text.Json; +// fix #406 sibling gap: a "write-stderr" mode alongside the existing +// "wait-for-break" mode so the launcher's stderr-capture tests can drive a +// real child process (both WindowsSystemChildProcess's native pipe path +// and SystemChildProcess's ProcessStartInfo.RedirectStandardError path) +// without a second fixture project. Usage: +// write-stderr +// Writes followed by its 0-based index, one per line, to +// stderr times (flushing every line so a launcher-side pump +// observes them incrementally rather than all at once on process exit), +// then returns . +if (args.Length >= 1 && args[0] == "write-stderr") +{ + if (args.Length < 4 + || !int.TryParse(args[1], out int exitCode) + || !int.TryParse(args[2], out int lineCount)) + { + return 64; + } + + string lineText = args[3]; + for (int index = 0; index < lineCount; index++) + { + Console.Error.WriteLine($"{lineText}{index}"); + Console.Error.Flush(); + } + + return exitCode; +} + if (args.Length < 4 || args[0] != "wait-for-break" || string.IsNullOrWhiteSpace(args[1]) diff --git a/tests/AcDream.Launcher.Core.Tests/Launching/BoundedProcessOutputCaptureTests.cs b/tests/AcDream.Launcher.Core.Tests/Launching/BoundedProcessOutputCaptureTests.cs new file mode 100644 index 00000000..5ad94363 --- /dev/null +++ b/tests/AcDream.Launcher.Core.Tests/Launching/BoundedProcessOutputCaptureTests.cs @@ -0,0 +1,240 @@ +using System.Text; +using AcDream.Launcher.Core.Launching; + +namespace AcDream.Launcher.Core.Tests.Launching; + +/// Fix #406 sibling gap: the launcher previously discarded a +/// supervised child's stderr entirely, so diagnosing a crash (including +/// exactly the #406 crash) required re-running the identical binary by +/// hand. These tests cover in +/// isolation — the real-child-process end-to-end capture tests live in +/// LauncherProcessSupervisorTests alongside the existing real-process +/// coverage. +public sealed class BoundedProcessOutputCaptureTests +{ + [Fact] + public void AppendLineWritesEachLineWithATrailingNewline() + { + string path = TempPath(); + try + { + using var capture = new BoundedProcessOutputCapture(path); + + capture.AppendLine("first"); + capture.AppendLine("second"); + capture.Dispose(); + + Assert.Equal("first\nsecond\n", File.ReadAllText(path)); + } + finally + { + TryDelete(path); + } + } + + [Fact] + public void ANullLineFromTheEndOfStreamSentinelIsANoOp() + { + string path = TempPath(); + try + { + using var capture = new BoundedProcessOutputCapture(path); + + capture.AppendLine("kept"); + capture.AppendLine(null); + capture.Dispose(); + + Assert.Equal("kept\n", File.ReadAllText(path)); + } + finally + { + TryDelete(path); + } + } + + [Fact] + public void WritesBeyondTheCapAreDroppedAndAOneTimeTruncationMarkerIsAppended() + { + string path = TempPath(); + try + { + using var capture = new BoundedProcessOutputCapture(path, maxBytes: 16); + + capture.AppendLine("0123456789"); // 11 bytes incl. newline + capture.AppendLine("this line is dropped entirely"); + capture.AppendLine("so is this one"); + + Assert.True(capture.IsDone); + string written = File.ReadAllText(path); + Assert.StartsWith("0123456789\n", written, StringComparison.Ordinal); + Assert.Contains("truncated at 16 bytes", written, StringComparison.Ordinal); + // The cap is a hard ceiling: nothing past it EVER lands on disk, + // even the marker's own text does not push the file arbitrarily + // far past the configured bound. + Assert.True( + written.Length < 200, + $"expected a small bounded file, got {written.Length} bytes"); + } + finally + { + TryDelete(path); + } + } + + [Fact] + public void ALogSpammingChildCannotGrowTheFileUnboundedly() + { + string path = TempPath(); + try + { + using var capture = new BoundedProcessOutputCapture( + path, + maxBytes: BoundedProcessOutputCapture.DefaultMaxBytes); + + // Far more than the 2 MiB default cap. + string spamLine = new('x', 4096); + for (int i = 0; i < 4096; i++) + { + capture.AppendLine(spamLine); + if (capture.IsDone) + { + break; + } + } + + Assert.True(capture.IsDone); + long fileLength = new FileInfo(path).Length; + Assert.True( + fileLength < BoundedProcessOutputCapture.DefaultMaxBytes + 256, + $"expected the file to stay near the {BoundedProcessOutputCapture.DefaultMaxBytes}-byte " + + $"cap, got {fileLength} bytes"); + } + finally + { + TryDelete(path); + } + } + + [Fact] + public void AppendCreatesTheSessionDirectoryOnFirstWrite() + { + string directory = Path.Combine( + Path.GetTempPath(), + "acdream-406-capture-" + Guid.NewGuid().ToString("N")); + string path = Path.Combine(directory, "client.err.log"); + Assert.False(Directory.Exists(directory)); + + try + { + using var capture = new BoundedProcessOutputCapture(path); + capture.AppendLine("hello"); + capture.Dispose(); + + Assert.True(File.Exists(path)); + } + finally + { + try + { + Directory.Delete(directory, recursive: true); + } + catch (IOException) + { + } + } + } + + [Fact] + public void RawByteAppendsAreConcatenatedWithoutAnImpliedLineBoundary() + { + string path = TempPath(); + try + { + using var capture = new BoundedProcessOutputCapture(path); + + capture.Append(Encoding.UTF8.GetBytes("abc")); + capture.Append(Encoding.UTF8.GetBytes("def")); + capture.Dispose(); + + Assert.Equal("abcdef", File.ReadAllText(path)); + } + finally + { + TryDelete(path); + } + } + + [Fact] + public void EmptyAppendsAreNoOps() + { + string path = TempPath(); + try + { + using var capture = new BoundedProcessOutputCapture(path); + + capture.Append(ReadOnlySpan.Empty); + capture.AppendLine(string.Empty); + capture.Dispose(); + + // An empty string line still gets its trailing newline — + // only a genuinely zero-length byte span (or a null line) is + // a true no-op. + Assert.Equal("\n", File.ReadAllText(path)); + } + finally + { + TryDelete(path); + } + } + + [Fact] + public void AppendAfterDisposeIsASilentNoOp() + { + string path = TempPath(); + try + { + var capture = new BoundedProcessOutputCapture(path); + capture.AppendLine("before"); + capture.Dispose(); + + capture.AppendLine("after — must not throw or reopen the file"); + + Assert.Equal("before\n", File.ReadAllText(path)); + } + finally + { + TryDelete(path); + } + } + + [Fact] + public void ConstructorRejectsANonPositiveMaxBytes() + { + Assert.Throws( + () => new BoundedProcessOutputCapture(TempPath(), maxBytes: 0)); + Assert.Throws( + () => new BoundedProcessOutputCapture(TempPath(), maxBytes: -1)); + } + + [Fact] + public void ConstructorRejectsANullOrBlankPath() + { + Assert.Throws(() => new BoundedProcessOutputCapture("")); + Assert.Throws(() => new BoundedProcessOutputCapture(" ")); + } + + private static string TempPath() => Path.Combine( + Path.GetTempPath(), + "acdream-406-capture-" + Guid.NewGuid().ToString("N") + ".log"); + + private static void TryDelete(string path) + { + try + { + File.Delete(path); + } + catch (IOException) + { + } + } +} diff --git a/tests/AcDream.Launcher.Core.Tests/Launching/LauncherProcessSupervisorTests.cs b/tests/AcDream.Launcher.Core.Tests/Launching/LauncherProcessSupervisorTests.cs index ca55ff60..837ed01b 100644 --- a/tests/AcDream.Launcher.Core.Tests/Launching/LauncherProcessSupervisorTests.cs +++ b/tests/AcDream.Launcher.Core.Tests/Launching/LauncherProcessSupervisorTests.cs @@ -543,6 +543,182 @@ public sealed class LauncherProcessSupervisorTests Assert.Equal(0, supervisor.ExitCode); } + [Fact] + public async Task RealChildStderrIsCapturedForTheProcessStartInfoPath() + { + // Fix #406 sibling gap: SystemChildProcess is used on Linux for + // EVERY child, and on Windows for graphical/non-console children — + // exactly #406's own App/GUI crash scenario + // (SupportsConsoleGracefulStop: false is the graphical shape; + // see LauncherExecutableSet.CreatePlaySpec). This proves the real + // child's stderr actually lands in the configured bounded file + // instead of being discarded, and that the real nonzero exit code + // is still observed independently of the capture. + string root = Path.Combine( + Path.GetTempPath(), + "acdream-406-stderr-psi", + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(root); + string stderrPath = Path.Combine(root, "client.err.log"); + + try + { + using var supervisor = new LauncherProcessSupervisor(); + var exited = new ManualResetEventSlim(false); + supervisor.StateChanged += (_, s) => + { + if (s == LauncherSessionState.Exited) + exited.Set(); + }; + + supervisor.Start( + new LauncherProcessSpec( + FindDotnetExecutable(), + [GetConsoleFixturePath(), "write-stderr", "7", "3", "line-"], + SupportsConsoleGracefulStop: false, + StderrLogPath: stderrPath), + password: null); + + Assert.True( + exited.Wait(TimeSpan.FromSeconds(30)), + "the write-stderr fixture did not exit within 30s"); + Assert.Equal(7, supervisor.ExitCode); + + string captured = await ReadFileEventuallyContainingAsync( + stderrPath, "line-2", TimeSpan.FromSeconds(5)); + Assert.Contains("line-0", captured, StringComparison.Ordinal); + Assert.Contains("line-1", captured, StringComparison.Ordinal); + Assert.Contains("line-2", captured, StringComparison.Ordinal); + } + finally + { + try + { + Directory.Delete(root, recursive: true); + } + catch (IOException) + { + } + } + } + + [Fact] + public async Task RealChildStderrIsCapturedForTheWindowsNativeConsolePath() + { + // Fix #406 sibling gap: on Windows, console-capable children + // (Headless) spawn through WindowsSystemChildProcess's native + // CreateProcessW path, a completely separate code path from + // SystemChildProcess above — WindowsProcessNative.StartCore + // creates a real pipe for stderr instead of the usual + // duplicate-or-NUL handle, and WindowsSystemChildProcess pumps it + // on a background thread. This proves that path end to end too. + if (!OperatingSystem.IsWindows()) + { + return; + } + + string root = Path.Combine( + Path.GetTempPath(), + "acdream-406-stderr-native", + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(root); + string stderrPath = Path.Combine(root, "client.err.log"); + + try + { + using var supervisor = new LauncherProcessSupervisor(); + var exited = new ManualResetEventSlim(false); + supervisor.StateChanged += (_, s) => + { + if (s == LauncherSessionState.Exited) + exited.Set(); + }; + + supervisor.Start( + new LauncherProcessSpec( + FindDotnetExecutable(), + [GetConsoleFixturePath(), "write-stderr", "9", "3", "native-line-"], + StderrLogPath: stderrPath), + password: null); + + Assert.True( + exited.Wait(TimeSpan.FromSeconds(30)), + "the write-stderr fixture did not exit within 30s"); + Assert.Equal(9, supervisor.ExitCode); + + string captured = await ReadFileEventuallyContainingAsync( + stderrPath, "native-line-2", TimeSpan.FromSeconds(5)); + Assert.Contains("native-line-0", captured, StringComparison.Ordinal); + Assert.Contains("native-line-1", captured, StringComparison.Ordinal); + Assert.Contains("native-line-2", captured, StringComparison.Ordinal); + } + finally + { + try + { + Directory.Delete(root, recursive: true); + } + catch (IOException) + { + } + } + } + + [Fact] + public void ANullStderrLogPathBehavesExactlyAsBeforeForBothChildProcessKinds() + { + // The additive-diagnostics contract: a spec with no StderrLogPath + // must not change behavior at all (fix #406 sibling gap review + // guard against a regression that always redirects stderr). + string dotnet = FindDotnetExecutable(); + using var supervisor = new LauncherProcessSupervisor(); + var exited = new ManualResetEventSlim(false); + supervisor.StateChanged += (_, s) => + { + if (s == LauncherSessionState.Exited) + exited.Set(); + }; + + supervisor.Start(new LauncherProcessSpec(dotnet, ["--version"]), null); + + Assert.True(exited.Wait(TimeSpan.FromSeconds(30))); + Assert.Equal(0, supervisor.ExitCode); + } + + private static async Task ReadFileEventuallyContainingAsync( + string path, + string expectedFragment, + TimeSpan timeout) + { + DateTime deadline = DateTime.UtcNow + timeout; + string last = string.Empty; + while (DateTime.UtcNow < deadline) + { + if (File.Exists(path)) + { + try + { + last = await File.ReadAllTextAsync(path); + if (last.Contains(expectedFragment, StringComparison.Ordinal)) + { + return last; + } + } + catch (IOException) + { + // The pump/writer may hold the file open for a + // moment — retry within the deadline. + } + } + + await Task.Delay(20); + } + + throw new TimeoutException( + $"'{path}' never contained '{expectedFragment}' within {timeout}. " + + $"Last observed content: {last}"); + } + private static LauncherProcessSpec Spec() => new("fake-host", ["--session-config", "session.json"]);