diff --git a/GenHub/GenHub.Core/Constants/ProcessConstants.cs b/GenHub/GenHub.Core/Constants/ProcessConstants.cs index 1a438e8d5..9a182e664 100644 --- a/GenHub/GenHub.Core/Constants/ProcessConstants.cs +++ b/GenHub/GenHub.Core/Constants/ProcessConstants.cs @@ -59,10 +59,32 @@ public static class ProcessConstants // Process discovery and timing constants /// - /// Delay in milliseconds to wait before checking if a process has exited (launcher detection). + /// Minimum time in milliseconds a Windows launcher stub's child is given to appear, + /// measured from launch, before it is searched for. /// + /// + /// Formerly the fixed delay before the single exited-yet check. Exit detection now + /// waits on the process itself (see ), + /// which can observe a stub exiting well before 500 ms; this floor preserves the time + /// the fixed delay always gave the spawned game process to register. + /// public const int LauncherDetectionDelayMs = 500; + /// + /// Bounded window in milliseconds during which a just-started game process is watched + /// for an early exit before the launch is reported successful. + /// + /// + /// Sized from measurement rather than guessed. The native Zero Hour client aborting + /// initialisation in an empty workspace exits 1 after roughly 0.8–0.9 s once warm + /// (macOS, Apple Silicon), so three seconds is ~3x the observed abort, absorbing slow + /// disks and emulation. The very first run of a freshly copied binary can take 3–5 s + /// because macOS validates the new inode before execution; an abort that slow falls + /// outside the window and is reported through the process-exited event instead of the + /// launch result. + /// + public const int PostSpawnExitDetectionWindowMs = 3000; + /// /// Interval in milliseconds for process cleanup / reconciliation background task. /// diff --git a/GenHub/GenHub.Core/Constants/RetailArchiveConstants.cs b/GenHub/GenHub.Core/Constants/RetailArchiveConstants.cs index 0b9bbf11c..f768fec13 100644 --- a/GenHub/GenHub.Core/Constants/RetailArchiveConstants.cs +++ b/GenHub/GenHub.Core/Constants/RetailArchiveConstants.cs @@ -24,6 +24,29 @@ public static class RetailArchiveConstants /// Environment variable naming the Generals retail directory. public const string GeneralsInstallPathVariable = "CNC_GENERALS_INSTALLPATH"; + /// + /// Stderr line prefix the engine writes when an archive's identifier does not match. + /// The rest of the line is the archive path. + /// + /// + /// Fork-only, like : emitted by + /// StdBIGFileSystem on the bgfx fork and absent upstream on every platform, + /// including Win32BIGFileSystem. Both sentinels are therefore strictly + /// advisory — their absence means "this build does not emit one", never that the + /// launch was healthy. + /// + public const string ArchiveIdentifierMismatchStderrPrefix = "[ggc] archive identifier mismatch: "; + + /// + /// Stderr line prefix the engine writes when an archive cannot be mounted at all. + /// The rest of the line is the archive path. + /// + /// + /// See for why matching this is + /// advisory only. + /// + public const string ArchiveMountFailedStderrPrefix = "[ggc] ARCHIVE MOUNT FAILED, contents unavailable this run: "; + /// /// Search pattern for the archives the engine mounts from a retail root. /// diff --git a/GenHub/GenHub.Core/Models/Events/GameProcessExitedEventArgs.cs b/GenHub/GenHub.Core/Models/Events/GameProcessExitedEventArgs.cs index 53aebd717..edb4ffc01 100644 --- a/GenHub/GenHub.Core/Models/Events/GameProcessExitedEventArgs.cs +++ b/GenHub/GenHub.Core/Models/Events/GameProcessExitedEventArgs.cs @@ -1,3 +1,5 @@ +using GenHub.Core.Constants; + namespace GenHub.Core.Models.Events; /// @@ -19,4 +21,72 @@ public class GameProcessExitedEventArgs : EventArgs /// Gets the time when the process exited. /// public DateTime ExitTime { get; init; } = DateTime.UtcNow; + + /// + /// Gets the bounded tail of the process's captured standard error, when any was captured. + /// + /// + /// Populated only for processes whose stderr the process manager was capturing, i.e. + /// ones it started itself. An initialisation abort slow enough to escape the + /// post-spawn detection window surfaces here, so subscribers can record why a launch + /// that was reported as started actually failed. + /// + public string? StandardErrorTail { get; init; } + + /// + /// Gets the archives named by the engine's mount-failure stderr sentinels, if any. + /// + /// + /// Advisory: the sentinels are emitted only by the fork engine, so an empty list says + /// nothing about whether archives mounted. + /// + public IReadOnlyList UnmountableArchives { get; init; } = []; + + /// + /// Gets a value indicating whether this exit was requested through the process + /// manager's terminate path before the kill was attempted. + /// + /// + /// A killed process exits non-zero, which is otherwise the signature of a crash; + /// this flag is what lets consumers tell a deliberate stop apart from one. + /// + public bool TerminationRequested { get; init; } + + /// + /// Describes why this exit is a failure, or returns null for a clean or unknown exit. + /// + /// + /// The single source of the late-failure wording: the launch registry records it and + /// the UI surfaces it, so composing it here keeps the two from drifting apart. The + /// advisory mount sentinels, when present, name the archive; otherwise the stderr + /// tail stands in. Only the non-zero exit code decides that the exit counts as a + /// failure — quitting the game cleanly is not one. + /// + /// The failure description, or null when the exit is not a failure. + public string? DescribeFailure() + { + // A requested termination is never a failure, even though the kill produces a + // non-zero exit code. Trade-off, accepted deliberately: an engine that genuinely + // crashed moments before the user clicked Stop is suppressed too — a missed + // report of an already-dying process is preferred over false-alarming "exited + // unexpectedly" on every deliberate stop. + if (TerminationRequested) + { + return null; + } + + if (ExitCode is not int exitCode || exitCode == ProcessConstants.ExitCodeSuccess) + { + return null; + } + + if (UnmountableArchives.Count > 0) + { + return $"The game could not mount required archive(s): {string.Join(", ", UnmountableArchives)}. Process exited with code {exitCode} after launch."; + } + + return StandardErrorTail is null + ? $"Process exited with code {exitCode} after launch. No output was captured." + : $"Process exited with code {exitCode} after launch. {StandardErrorTail}"; + } } \ No newline at end of file diff --git a/GenHub/GenHub.Core/Models/GameProfile/GameLaunchInfo.cs b/GenHub/GenHub.Core/Models/GameProfile/GameLaunchInfo.cs index 2800efed2..27c353cab 100644 --- a/GenHub/GenHub.Core/Models/GameProfile/GameLaunchInfo.cs +++ b/GenHub/GenHub.Core/Models/GameProfile/GameLaunchInfo.cs @@ -23,6 +23,24 @@ public class GameLaunchInfo /// Gets or sets the termination timestamp. public DateTime? TerminatedAt { get; set; } + /// Gets or sets the process exit code, when it is known. + public int? ExitCode { get; set; } + + /// + /// Gets or sets why this launch is considered failed, when the process exited + /// abnormally after the launch had already been reported as started. + /// + /// + /// The late-failure channel: an initialisation abort slow enough to outlive the + /// post-spawn detection window cannot fail the start operation retroactively, so the + /// failure is recorded here instead. A clean exit leaves this null — quitting the + /// game is not a failed launch. + /// + public string? FailureReason { get; set; } + + /// Gets a value indicating whether this launch ended in failure. + public bool HasFailed => FailureReason != null; + /// Gets a value indicating whether the game is still running. public bool IsRunning => TerminatedAt == null; } \ No newline at end of file diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/NativeClientLaunchIntegrationTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/NativeClientLaunchIntegrationTests.cs index 209152f37..f5c4ea40e 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/NativeClientLaunchIntegrationTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/NativeClientLaunchIntegrationTests.cs @@ -64,7 +64,7 @@ public async Task RealNativeClient_LaunchesThroughGameProcessManager() try { - // StartProcessAsync only waits out the launcher-detection delay. Give the + // StartProcessAsync only waits out the post-spawn detection window. Give the // engine long enough to fail the way it fails for real: mounting archives and // initialising the renderer, both of which happen after the process exists. await Task.Delay(LaunchSettleTime); @@ -135,7 +135,7 @@ public async Task RealNativeClient_RequiresItsInstallDirectoryAsWorkingDirectory // The expected path: it dies during startup and the failure names the reason // rather than reporting a bare exit code. Assert.False(result.Success); - Assert.Contains("exited immediately", string.Join(" ", result.Errors), StringComparison.OrdinalIgnoreCase); + Assert.Contains("exited during startup", string.Join(" ", result.Errors), StringComparison.OrdinalIgnoreCase); } finally { diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/PostSpawnFailureDetectionTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/PostSpawnFailureDetectionTests.cs new file mode 100644 index 000000000..40df7d99d --- /dev/null +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/PostSpawnFailureDetectionTests.cs @@ -0,0 +1,379 @@ +using System; +using System.IO; +using System.Runtime.InteropServices; +using System.Threading.Tasks; +using GenHub.Core.Constants; +using GenHub.Core.Models.Events; +using GenHub.Core.Models.GameProfile; +using GenHub.Core.Models.Launching; +using GenHub.Features.GameProfiles.Infrastructure; +using GenHub.Features.Launching; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace GenHub.Tests.Core.Features.GameProfiles; + +/// +/// Verifies that an engine which spawns and then aborts during initialisation is reported +/// as a failed launch, not a successful one. +/// +/// The measured initialisation abort takes the native client roughly a second — beyond +/// the old fixed 500 ms sample, inside the exit-or-settle window. These tests use +/// synthetic scripts timed to land in exactly that gap, plus the fork engine's advisory +/// [ggc] mount-failure sentinels, which name the archive when present but whose +/// absence must change nothing. +/// +/// +public class PostSpawnFailureDetectionTests : IDisposable +{ + /// + /// When the late-exit scripts exit, in seconds: past the detection window, matching + /// the measured cold-start abort that motivates the late-failure channel. + /// + private const int PostWindowExitSeconds = 4; + + private readonly string _tempDir = Path.Combine( + Path.GetTempPath(), + $"genhub-postspawn-{Guid.NewGuid():N}"); + + private readonly GameProcessManager _processManager = new(NullLogger.Instance); + + /// + /// Initializes a new instance of the class. + /// + public PostSpawnFailureDetectionTests() => Directory.CreateDirectory(_tempDir); + + private static bool OnUnix => !RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + + /// + /// An abort after 700 ms — outside the old fixed window — that names its archives via + /// the sentinels must fail the launch with the archives named, not the raw tail. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task DelayedAbortWithMountSentinels_FailsNamingTheArchives() + { + if (!OnUnix) + { + return; + } + + var binary = await WriteScriptAsync( + "#!/bin/sh\n" + + "sleep 0.7\n" + + $"echo \"{RetailArchiveConstants.ArchiveIdentifierMismatchStderrPrefix}INIZH.big\" >&2\n" + + $"echo \"{RetailArchiveConstants.ArchiveMountFailedStderrPrefix}TexturesZH.big\" >&2\n" + + "exit 1\n"); + + var result = await _processManager.StartProcessAsync(new GameLaunchConfiguration + { + ExecutablePath = binary, + WorkingDirectory = _tempDir, + }); + + Assert.False(result.Success); + + var message = string.Join(" ", result.Errors); + Assert.Contains("could not mount", message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("INIZH.big", message); + Assert.Contains("TexturesZH.big", message); + } + + /// + /// The same delayed abort without a sentinel must still fail — the window, not the + /// sentinel, decides — and must surface the stderr tail unchanged. This is every + /// build except the fork, which never emits a sentinel at all. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task DelayedAbortWithoutSentinel_FailsWithTheStderrTail() + { + if (!OnUnix) + { + return; + } + + var binary = await WriteScriptAsync( + "#!/bin/sh\n" + + "sleep 0.7\n" + + "echo \"Technical difficulties during initialisation\" >&2\n" + + "exit 1\n"); + + var result = await _processManager.StartProcessAsync(new GameLaunchConfiguration + { + ExecutablePath = binary, + WorkingDirectory = _tempDir, + }); + + Assert.False(result.Success); + + var message = string.Join(" ", result.Errors); + Assert.Contains("Technical difficulties during initialisation", message); + Assert.DoesNotContain("could not mount", message, StringComparison.OrdinalIgnoreCase); + } + + /// + /// A fast abort without a sentinel keeps its existing behaviour: failure with the + /// exit code and the stderr tail. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task FastAbortWithoutSentinel_FailsWithTheStderrTail() + { + if (!OnUnix) + { + return; + } + + var binary = await WriteScriptAsync( + "#!/bin/sh\n" + + "echo \"missing data directory\" >&2\n" + + "exit 1\n"); + + var result = await _processManager.StartProcessAsync(new GameLaunchConfiguration + { + ExecutablePath = binary, + WorkingDirectory = _tempDir, + }); + + Assert.False(result.Success); + + var message = string.Join(" ", result.Errors); + Assert.Contains("1", message); + Assert.Contains("missing data directory", message); + } + + /// + /// A sentinel on stderr from a process that keeps running must not fail the launch: + /// the engine treats an unmountable archive as survivable, and the sentinel is + /// advisory in both directions. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task SentinelFromASurvivingProcess_DoesNotFailTheLaunch() + { + if (!OnUnix) + { + return; + } + + var binary = await WriteScriptAsync( + "#!/bin/sh\n" + + $"echo \"{RetailArchiveConstants.ArchiveMountFailedStderrPrefix}W3DZH.big\" >&2\n" + + "sleep 30\n"); + + var result = await _processManager.StartProcessAsync(new GameLaunchConfiguration + { + ExecutablePath = binary, + WorkingDirectory = _tempDir, + }); + + Assert.True(result.Success, $"Launch failed: {string.Join(" ", result.Errors)}"); + + if (result.Data is not null) + { + await _processManager.TerminateProcessAsync(result.Data.ProcessId); + } + } + + /// + /// A process that outlives the window is a successful launch, and remains manageable + /// afterwards: it can be found and terminated through the manager. The kill exits + /// the process non-zero, so the resulting exit event must carry the + /// requested-termination mark and classify as no failure — a deliberate stop must + /// never read as a crash. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task ProcessOutlivingTheWindow_LaunchesAndTerminatesWithoutFailureClassification() + { + if (!OnUnix) + { + return; + } + + var binary = await WriteScriptAsync("#!/bin/sh\nsleep 30\n"); + + var exited = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _processManager.ProcessExited += (_, e) => exited.TrySetResult(e); + + var result = await _processManager.StartProcessAsync(new GameLaunchConfiguration + { + ExecutablePath = binary, + WorkingDirectory = _tempDir, + }); + + Assert.True(result.Success, $"Launch failed: {string.Join(" ", result.Errors)}"); + Assert.NotNull(result.Data); + + var info = await _processManager.GetProcessInfoAsync(result.Data!.ProcessId); + Assert.True(info.Success); + + var terminated = await _processManager.TerminateProcessAsync(result.Data!.ProcessId); + Assert.True(terminated.Success); + + var completed = await Task.WhenAny(exited.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.True(completed == exited.Task, "The exit event for the terminated process never arrived."); + + var exitEvent = await exited.Task; + Assert.True(exitEvent.TerminationRequested); + Assert.Null(exitEvent.DescribeFailure()); + } + + /// + /// An abort landing after the window — the measured cold-start case — cannot fail + /// the start operation, so it must be recorded retroactively: the registry's launch + /// entry ends up failed, with the sentinel-named archives and the exit code. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task AbortAfterTheWindow_MarksTheRegisteredLaunchFailedNamingTheArchive() + { + if (!OnUnix) + { + return; + } + + var binary = await WriteScriptAsync( + "#!/bin/sh\n" + + $"sleep {PostWindowExitSeconds}\n" + + $"echo \"{RetailArchiveConstants.ArchiveMountFailedStderrPrefix}TexturesZH.big\" >&2\n" + + "exit 1\n"); + + var launch = await LaunchAndAwaitLateExitAsync(binary); + + Assert.True(launch.HasFailed); + Assert.Equal(1, launch.ExitCode); + Assert.False(launch.IsRunning); + Assert.Contains("could not mount", launch.FailureReason, StringComparison.OrdinalIgnoreCase); + Assert.Contains("TexturesZH.big", launch.FailureReason); + } + + /// + /// The same late abort without a sentinel must still be recorded as failed — the + /// exit code decides, the sentinel only improves the message — carrying the stderr + /// tail as the reason. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task AbortAfterTheWindowWithoutSentinel_MarksTheLaunchFailedWithTheTail() + { + if (!OnUnix) + { + return; + } + + var binary = await WriteScriptAsync( + "#!/bin/sh\n" + + $"sleep {PostWindowExitSeconds}\n" + + "echo \"renderer initialisation failed\" >&2\n" + + "exit 1\n"); + + var launch = await LaunchAndAwaitLateExitAsync(binary); + + Assert.True(launch.HasFailed); + Assert.Contains("renderer initialisation failed", launch.FailureReason); + Assert.DoesNotContain("could not mount", launch.FailureReason, StringComparison.OrdinalIgnoreCase); + } + + /// + /// A clean exit after the window is the user quitting, not a failed launch: the + /// entry terminates without a failure reason. + /// + /// A task representing the asynchronous test. + [Fact] + public async Task CleanExitAfterTheWindow_IsNotMarkedAsAFailure() + { + if (!OnUnix) + { + return; + } + + var binary = await WriteScriptAsync( + "#!/bin/sh\n" + + $"sleep {PostWindowExitSeconds}\n" + + "exit 0\n"); + + var launch = await LaunchAndAwaitLateExitAsync(binary); + + Assert.False(launch.HasFailed); + Assert.Null(launch.FailureReason); + Assert.Equal(0, launch.ExitCode); + Assert.False(launch.IsRunning); + } + + /// + /// Releases the temporary directory. + /// + public void Dispose() + { + GC.SuppressFinalize(this); + try + { + if (Directory.Exists(_tempDir)) + { + Directory.Delete(_tempDir, recursive: true); + } + } + catch (IOException) + { + // A leftover temp directory is not worth failing a test over. + } + } + + /// + /// Starts the script through the manager, registers the launch the way the launcher + /// does, waits for the post-window exit, and returns the registry's view of it. + /// + /// The script to launch. + /// The launch entry after the process exited. + private async Task LaunchAndAwaitLateExitAsync(string binary) + { + // Wired exactly as in production: the registry subscribes to the manager's exit + // event in its constructor, before this test's own completion probe. + var registry = new LaunchRegistry(NullLogger.Instance, null, _processManager); + + var exited = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _processManager.ProcessExited += (_, e) => exited.TrySetResult(e); + + var result = await _processManager.StartProcessAsync(new GameLaunchConfiguration + { + ExecutablePath = binary, + WorkingDirectory = _tempDir, + }); + + // The start contract is unchanged: a process that outlives the window launched. + Assert.True(result.Success, $"Launch failed: {string.Join(" ", result.Errors)}"); + Assert.NotNull(result.Data); + + var launchInfo = new GameLaunchInfo + { + LaunchId = Guid.NewGuid().ToString("N"), + ProfileId = "post-spawn-tests", + WorkspaceId = string.Empty, + ProcessInfo = result.Data!, + }; + await registry.RegisterLaunchAsync(launchInfo); + + var completed = await Task.WhenAny(exited.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.True(completed == exited.Task, "The process did not exit within the allotted time."); + + var launch = await registry.GetLaunchInfoAsync(launchInfo.LaunchId); + Assert.NotNull(launch); + return launch!; + } + + private async Task WriteScriptAsync(string content) + { + var binary = Path.Combine(_tempDir, NativeClientFixture.BinaryName); + await File.WriteAllTextAsync(binary, content); + if (!OperatingSystem.IsWindows()) + { + File.SetUnixFileMode( + binary, + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + + return binary; + } +} diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/ViewModels/GameProfileLauncherViewModelTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/ViewModels/GameProfileLauncherViewModelTests.cs index fea3b7161..85fbf669a 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/ViewModels/GameProfileLauncherViewModelTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/GameProfiles/ViewModels/GameProfileLauncherViewModelTests.cs @@ -11,6 +11,7 @@ using GenHub.Core.Interfaces.Shortcuts; using GenHub.Core.Interfaces.Steam; using GenHub.Core.Models.Enums; +using GenHub.Core.Models.Events; using GenHub.Core.Models.GameClients; using GenHub.Core.Models.GameInstallations; using GenHub.Core.Models.GameProfile; @@ -307,6 +308,122 @@ public void GenerateUniqueProfileName_CreatesUniqueName() Assert.Equal($"Test Profile {string.Format(ProfileConstants.CopyNameNumberedFormat, 3)}", uniqueName); } + /// + /// A process that dies after the launch was announced as running must not vanish + /// silently: the late failure surfaces through the same status, error, and + /// notification channel as a failed launch, naming the archive when known. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task ProcessExitedWithFailure_SurfacesTheFailureToTheUser() + { + var gameProcessManager = new Mock(); + var notificationService = new Mock(); + var vm = CreateViewModelWithMockDependencies(gameProcessManager, notificationService); + + // InitializeAsync is where the view model subscribes to ProcessExited. + await vm.InitializeAsync(); + + var profile = CreateProfileItem("Failing Profile"); + profile.ProcessId = 4242; + profile.IsProcessRunning = true; + vm.Profiles.Add(profile); + + gameProcessManager.Raise(m => m.ProcessExited += null, new GameProcessExitedEventArgs + { + ProcessId = 4242, + ExitCode = 1, + StandardErrorTail = "init abort", + UnmountableArchives = ["TexturesZH.big"], + }); + + Assert.False(profile.IsProcessRunning); + Assert.Equal(0, profile.ProcessId); + Assert.Contains("exited unexpectedly", vm.StatusMessage); + Assert.Contains("Failing Profile", vm.StatusMessage); + Assert.Contains("TexturesZH.big", vm.ErrorMessage); + notificationService.Verify( + n => n.ShowError( + "Game Exited Unexpectedly", + It.Is(s => s.Contains("TexturesZH.big") && s.Contains("Failing Profile")), + It.IsAny(), + It.IsAny()), + Times.Once); + } + + /// + /// A clean exit is the user quitting: the running state clears and nothing is + /// reported as an error. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task ProcessExitedCleanly_DoesNotReportAFailure() + { + var gameProcessManager = new Mock(); + var notificationService = new Mock(); + var vm = CreateViewModelWithMockDependencies(gameProcessManager, notificationService); + + // InitializeAsync is where the view model subscribes to ProcessExited. + await vm.InitializeAsync(); + + var profile = CreateProfileItem("Quitting Profile"); + profile.ProcessId = 4243; + profile.IsProcessRunning = true; + vm.Profiles.Add(profile); + + gameProcessManager.Raise(m => m.ProcessExited += null, new GameProcessExitedEventArgs + { + ProcessId = 4243, + ExitCode = 0, + }); + + Assert.False(profile.IsProcessRunning); + Assert.Equal(0, profile.ProcessId); + Assert.Equal(string.Empty, vm.ErrorMessage); + notificationService.Verify( + n => n.ShowError(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + /// + /// A stop the user asked for kills the process with a non-zero exit code; that must + /// not raise the "exited unexpectedly" alarm — the stop path's own status stands. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task ProcessExitedFromARequestedStop_DoesNotRaiseTheFailureAlarm() + { + var gameProcessManager = new Mock(); + var notificationService = new Mock(); + var vm = CreateViewModelWithMockDependencies(gameProcessManager, notificationService); + + // InitializeAsync is where the view model subscribes to ProcessExited. + await vm.InitializeAsync(); + + var profile = CreateProfileItem("Stopped Profile"); + profile.ProcessId = 4244; + profile.IsProcessRunning = true; + vm.Profiles.Add(profile); + + // The status a completed stop leaves behind; the exit event must not replace it. + vm.StatusMessage = "Stopped Profile stopped successfully"; + + gameProcessManager.Raise(m => m.ProcessExited += null, new GameProcessExitedEventArgs + { + ProcessId = 4244, + ExitCode = 137, + TerminationRequested = true, + }); + + Assert.False(profile.IsProcessRunning); + Assert.Equal(0, profile.ProcessId); + Assert.Equal("Stopped Profile stopped successfully", vm.StatusMessage); + Assert.Equal(string.Empty, vm.ErrorMessage); + notificationService.Verify( + n => n.ShowError(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + private static ProfileResourceService CreateProfileResourceService() { return new ProfileResourceService(NullLogger.Instance); @@ -341,9 +458,32 @@ private static SuperHackersProvider CreateSuperHackersProvider() /// /// A GameProfileLauncherViewModel instance for testing. private static GameProfileLauncherViewModel CreateViewModelWithMockDependencies() + { + return CreateViewModelWithMockDependencies( + new Mock(), + new Mock()); + } + + /// + /// Creates a GameProfileLauncherViewModel wired to the given process manager and + /// notification mocks, so tests can raise process events and observe notifications. + /// + /// The process manager mock the view model subscribes to. + /// The notification service mock to observe. + /// A GameProfileLauncherViewModel instance for testing. + private static GameProfileLauncherViewModel CreateViewModelWithMockDependencies( + Mock gameProcessManager, + Mock notificationService) { var gameProfileManager = new Mock(); + // InitializeAsync must complete cleanly: it is what subscribes the view model to + // ProcessExited, and a failed profile load would pollute the error state these + // tests assert on. + gameProfileManager + .Setup(x => x.GetAllProfilesAsync(It.IsAny())) + .ReturnsAsync(ProfileOperationResult>.CreateSuccess([])); + return new GameProfileLauncherViewModel( new Mock().Object, gameProfileManager.Object, @@ -362,15 +502,30 @@ private static GameProfileLauncherViewModel CreateViewModelWithMockDependencies( NullLogger.Instance), new Mock().Object, new Mock().Object, - new Mock().Object, + gameProcessManager.Object, new Mock().Object, new Mock().Object, new Mock().Object, CreateProfileResourceService(), new Mock().Object, - new Mock().Object, + notificationService.Object, new Mock().Object, new Mock().Object, NullLogger.Instance); } + + /// + /// Creates a profile item view model backed by a mocked profile. + /// + /// The profile name. + /// A profile item for the launcher's collection. + private static GameProfileItemViewModel CreateProfileItem(string name) + { + var profile = new Mock(); + profile.SetupGet(p => p.Name).Returns(name); + profile.SetupGet(p => p.Version).Returns("1.0"); + profile.SetupGet(p => p.ExecutablePath).Returns(string.Empty); + + return new GameProfileItemViewModel("profile-1", profile.Object, string.Empty, string.Empty); + } } diff --git a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Launching/LaunchRegistryTests.cs b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Launching/LaunchRegistryTests.cs index 77651b67a..65e960ffc 100644 --- a/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Launching/LaunchRegistryTests.cs +++ b/GenHub/GenHub.Tests/GenHub.Tests.Core/Features/Launching/LaunchRegistryTests.cs @@ -1,3 +1,5 @@ +using GenHub.Core.Interfaces.GameProfiles; +using GenHub.Core.Models.Events; using GenHub.Core.Models.GameProfile; using GenHub.Core.Models.Launching; using GenHub.Features.Launching; @@ -87,4 +89,265 @@ public async Task UnregisterLaunchAsync_ShouldRemoveLaunchInfo() // Assert Assert.Null(result); } + + /// + /// The placeholder-PID race: the launcher registers a launch with PID -1 before + /// spawning and records the real PID only after the start operation returns. An + /// exit event landing in that gap matches nothing — it must be buffered and applied + /// when the registration with the real PID arrives, failure evidence intact. + /// + /// A task representing the asynchronous operation. + [Fact] + public async Task ExitEventBeforePidRegistration_IsAppliedWhenTheRealPidArrives() + { + var processManager = new Mock(); + var registry = new LaunchRegistry(Mock.Of>(), null, processManager.Object); + + const int realPid = 987654; + const string launchId = "race-launch"; + + // 1. Placeholder registration, exactly as GameLauncher does it. + await registry.RegisterLaunchAsync(new GameLaunchInfo + { + LaunchId = launchId, + ProfileId = "profile1", + WorkspaceId = string.Empty, + ProcessInfo = new GameProcessInfo { ProcessId = -1 }, + }); + + // 2. The process dies before the registry has learned its PID. + processManager.Raise(m => m.ProcessExited += null, new GameProcessExitedEventArgs + { + ProcessId = realPid, + ExitCode = 1, + StandardErrorTail = "init abort", + UnmountableArchives = ["TexturesZH.big"], + }); + + // 3. The launcher then updates the entry with the real PID. + await registry.RegisterLaunchAsync(new GameLaunchInfo + { + LaunchId = launchId, + ProfileId = "profile1", + WorkspaceId = "workspace1", + ProcessInfo = new GameProcessInfo { ProcessId = realPid }, + }); + + var launch = await registry.GetLaunchInfoAsync(launchId); + + Assert.NotNull(launch); + Assert.True(launch!.HasFailed); + Assert.Equal(1, launch.ExitCode); + Assert.False(launch.IsRunning); + Assert.Contains("TexturesZH.big", launch.FailureReason); + } + + /// + /// Double delivery: the same exit seen both before the PID update (buffered, applied + /// at registration) and after it (matched directly) must neither duplicate nor + /// contradict the recorded failure. + /// + /// A task representing the asynchronous operation. + [Fact] + public async Task ExitEventDeliveredBeforeAndAfterPidRegistration_IsRecordedOnce() + { + var processManager = new Mock(); + var registry = new LaunchRegistry(Mock.Of>(), null, processManager.Object); + + const int realPid = 987655; + const string launchId = "double-delivery-launch"; + + await registry.RegisterLaunchAsync(new GameLaunchInfo + { + LaunchId = launchId, + ProfileId = "profile1", + WorkspaceId = string.Empty, + ProcessInfo = new GameProcessInfo { ProcessId = -1 }, + }); + + var exitEvent = new GameProcessExitedEventArgs + { + ProcessId = realPid, + ExitCode = 1, + StandardErrorTail = "init abort", + UnmountableArchives = ["INIZH.big"], + }; + + processManager.Raise(m => m.ProcessExited += null, exitEvent); + + await registry.RegisterLaunchAsync(new GameLaunchInfo + { + LaunchId = launchId, + ProfileId = "profile1", + WorkspaceId = "workspace1", + ProcessInfo = new GameProcessInfo { ProcessId = realPid }, + }); + + var afterFirst = await registry.GetLaunchInfoAsync(launchId); + var recordedReason = afterFirst!.FailureReason; + var recordedAt = afterFirst.TerminatedAt; + + // The same exit surfaces again, now matching the registered PID directly. + processManager.Raise(m => m.ProcessExited += null, exitEvent); + + var afterSecond = await registry.GetLaunchInfoAsync(launchId); + + Assert.NotNull(afterSecond); + Assert.True(afterSecond!.HasFailed); + Assert.Equal(1, afterSecond.ExitCode); + Assert.Equal(recordedReason, afterSecond.FailureReason); + Assert.Equal(recordedAt, afterSecond.TerminatedAt); + } + + /// + /// The stranding interleaving: the exit handler's lookup misses, and the + /// registration carrying the real PID starts precisely before the handler's buffer + /// write lands. Without atomicity the registration drains a still-empty buffer and + /// the event strands until it expires. The test seam pauses the handler in exactly + /// that window while the registration runs on another thread; the lock forces the + /// registration to wait, so the buffered event must still be applied. + /// + /// A task representing the asynchronous operation. + [Fact] + public async Task ExitEventInterleavedWithRegistration_IsNotStranded() + { + var processManager = new Mock(); + var registry = new LaunchRegistry(Mock.Of>(), null, processManager.Object); + + const int realPid = 987657; + const string launchId = "stranding-launch"; + + await registry.RegisterLaunchAsync(new GameLaunchInfo + { + LaunchId = launchId, + ProfileId = "profile1", + WorkspaceId = string.Empty, + ProcessInfo = new GameProcessInfo { ProcessId = -1 }, + }); + + using var handlerInWindow = new ManualResetEventSlim(false); + using var releaseHandler = new ManualResetEventSlim(false); + registry.PendingExitBufferingHook = () => + { + handlerInWindow.Set(); + releaseHandler.Wait(TimeSpan.FromSeconds(10)); + }; + + // The exit event arrives; its lookup misses (only the placeholder is known) and + // the handler is now paused between that miss and its buffer write. + var exitDelivery = Task.Run(() => processManager.Raise(m => m.ProcessExited += null, new GameProcessExitedEventArgs + { + ProcessId = realPid, + ExitCode = 1, + StandardErrorTail = "init abort", + UnmountableArchives = ["TexturesZH.big"], + })); + + Assert.True(handlerInWindow.Wait(TimeSpan.FromSeconds(10)), "The exit handler never reached the buffering window."); + + // The registration with the real PID is started inside that window — the exact + // schedule that would drain an empty buffer and strand the event. + var registration = Task.Run(() => registry.RegisterLaunchAsync(new GameLaunchInfo + { + LaunchId = launchId, + ProfileId = "profile1", + WorkspaceId = "workspace1", + ProcessInfo = new GameProcessInfo { ProcessId = realPid }, + })); + + releaseHandler.Set(); + await exitDelivery; + await registration; + + var launch = await registry.GetLaunchInfoAsync(launchId); + + Assert.NotNull(launch); + Assert.True(launch!.HasFailed, "The exit event was stranded in the pending buffer instead of being applied."); + Assert.Equal(1, launch.ExitCode); + Assert.False(launch.IsRunning); + Assert.Contains("TexturesZH.big", launch.FailureReason); + } + + /// + /// A termination the user asked for kills the process, and the kill exits non-zero; + /// that must record a normal termination, not a failure. + /// + /// A task representing the asynchronous operation. + [Fact] + public async Task RequestedTerminationWithNonZeroExit_IsNotRecordedAsAFailure() + { + var processManager = new Mock(); + var registry = new LaunchRegistry(Mock.Of>(), null, processManager.Object); + + const int realPid = 987658; + const string launchId = "requested-stop-launch"; + + await registry.RegisterLaunchAsync(new GameLaunchInfo + { + LaunchId = launchId, + ProfileId = "profile1", + WorkspaceId = "workspace1", + ProcessInfo = new GameProcessInfo { ProcessId = realPid }, + }); + + processManager.Raise(m => m.ProcessExited += null, new GameProcessExitedEventArgs + { + ProcessId = realPid, + ExitCode = 137, + TerminationRequested = true, + }); + + var launch = await registry.GetLaunchInfoAsync(launchId); + + Assert.NotNull(launch); + Assert.False(launch!.HasFailed); + Assert.Null(launch.FailureReason); + Assert.Equal(137, launch.ExitCode); + Assert.False(launch.IsRunning); + } + + /// + /// A clean exit buffered across the same race is applied as a normal termination: + /// the launch ends, but it is not marked as failed. + /// + /// A task representing the asynchronous operation. + [Fact] + public async Task CleanExitBufferedAcrossTheRace_TerminatesWithoutFailure() + { + var processManager = new Mock(); + var registry = new LaunchRegistry(Mock.Of>(), null, processManager.Object); + + const int realPid = 987656; + const string launchId = "clean-exit-launch"; + + await registry.RegisterLaunchAsync(new GameLaunchInfo + { + LaunchId = launchId, + ProfileId = "profile1", + WorkspaceId = string.Empty, + ProcessInfo = new GameProcessInfo { ProcessId = -1 }, + }); + + processManager.Raise(m => m.ProcessExited += null, new GameProcessExitedEventArgs + { + ProcessId = realPid, + ExitCode = 0, + }); + + await registry.RegisterLaunchAsync(new GameLaunchInfo + { + LaunchId = launchId, + ProfileId = "profile1", + WorkspaceId = "workspace1", + ProcessInfo = new GameProcessInfo { ProcessId = realPid }, + }); + + var launch = await registry.GetLaunchInfoAsync(launchId); + + Assert.NotNull(launch); + Assert.False(launch!.HasFailed); + Assert.Null(launch.FailureReason); + Assert.Equal(0, launch.ExitCode); + Assert.False(launch.IsRunning); + } } \ No newline at end of file diff --git a/GenHub/GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs b/GenHub/GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs index 673e1edc7..47f30a09b 100644 --- a/GenHub/GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs +++ b/GenHub/GenHub/Features/GameProfiles/Infrastructure/GameProcessManager.cs @@ -25,6 +25,31 @@ public class GameProcessManager( { private const int CleanupIntervalMs = ProcessConstants.ProcessCleanupIntervalMs; private readonly ConcurrentDictionary _managedProcesses = new(); + + /// + /// Stderr captures for processes this manager started itself, keyed by PID. + /// + /// + /// The late-failure channel: an initialisation abort slow enough to outlive the + /// post-spawn detection window exits after the launch was reported as started, and + /// its stderr — the only explanation of the failure — would otherwise be dropped with + /// the start operation's locals. Kept per PID so can + /// attach it to the exit event. + /// + private readonly ConcurrentDictionary _stderrBuffers = new(); + + /// + /// PIDs whose termination was requested through , + /// marked before the kill is attempted. + /// + /// + /// A deliberate stop kills the process, and a killed process exits non-zero — which + /// is exactly the signature the late-failure channel treats as a crash. Every stop + /// path in the application funnels through , so + /// marking here lets the exit event distinguish "the user stopped it" from "it + /// died", and downstream consumers suppress the failure classification. + /// + private readonly ConcurrentDictionary _requestedTerminations = new(); private readonly SemaphoreSlim _terminationSemaphore = new(1, 1); /// @@ -220,14 +245,17 @@ public async Task> StartProcessAsync(GameLaunch logger.LogDebug(ex, "[Process] Could not capture stderr for process {ProcessId}", process.Id); } - // Check if process exited immediately (launcher pattern) - // Only apply delay if we need to detect a spawned process + // Exit-or-settle: wait on the process itself for up to the detection window + // rather than sleeping a fixed delay and sampling once. An initialisation + // abort takes the engine roughly a second — beyond the old 500 ms sample — + // and under the fixed delay such a process was reported as a successful + // launch and then died unobserved. A process still alive at window end is + // treated as launched; a later abort is only observable via ProcessExited. if (!isBatchFile) { - // Quick check to see if process exited immediately (launcher pattern) - await Task.Delay(ProcessConstants.LauncherDetectionDelayMs, cancellationToken); // Reduced from 2000ms - only for launcher detection + var spawnStopwatch = Stopwatch.StartNew(); - if (process.HasExited) + if (await WaitForExitWithinWindowAsync(process, cancellationToken)) { var exitCode = process.ExitCode; @@ -248,6 +276,16 @@ public async Task> StartProcessAsync(GameLaunch "[Process] Launcher process {ProcessId} exited with code 0 - attempting to find spawned game process", process.Id); + // Exit-or-settle can observe a stub exiting well before the old + // fixed delay elapsed. The fixed delay always gave the spawned + // game process that long from launch to register; preserve that + // floor so the search below is not run too early to find it. + var remainingMs = ProcessConstants.LauncherDetectionDelayMs - (int)spawnStopwatch.ElapsedMilliseconds; + if (remainingMs > 0) + { + await Task.Delay(remainingMs, cancellationToken); + } + var executableName = Path.GetFileNameWithoutExtension(configuration.ExecutablePath); var spawnedProcess = FindSpawnedGameProcess(executableName, configuration.WorkingDirectory ?? Path.GetDirectoryName(configuration.ExecutablePath)!); @@ -301,7 +339,11 @@ public async Task> StartProcessAsync(GameLaunch } } - logger.LogWarning("Process {ProcessId} exited immediately with code {ExitCode}", process.Id, exitCode); + logger.LogWarning( + "Process {ProcessId} exited during startup with code {ExitCode} after {ElapsedMs} ms", + process.Id, + exitCode, + spawnStopwatch.ElapsedMilliseconds); // The process has exited, but the async stderr handlers may not have // delivered their final lines yet — and this is exactly the path where @@ -312,21 +354,38 @@ public async Task> StartProcessAsync(GameLaunch process.Dispose(); - // If it exits immediately with a non-zero code (like a crash or missing DLL), this is a genuine failure + // If it exits during startup with a non-zero code (like a crash or missing DLL), this is a genuine failure if (exitCode != 0) { + // Advisory only: the [ggc] sentinels are emitted solely by the + // fork engine, so their presence upgrades the message to name the + // archive, but their absence says nothing about the launch. + var unmountableArchives = ExtractUnmountableArchives(capturedErrors.Snapshot()); + if (unmountableArchives.Count > 0) + { + var archiveNames = string.Join(", ", unmountableArchives); + + logger.LogError( + "[Process] Process exited during startup with code {ExitCode} after failing to mount archive(s): {Archives}", + exitCode, + archiveNames); + + return OperationResult.CreateFailure( + $"The game could not mount required archive(s): {archiveNames}. Process exited during startup with code {exitCode}."); + } + var stderrTail = capturedErrors.ToString(); var detail = string.IsNullOrWhiteSpace(stderrTail) ? "No output was captured." : stderrTail; logger.LogError( - "[Process] Process exited immediately with code {ExitCode}. Output: {Output}", + "[Process] Process exited during startup with code {ExitCode}. Output: {Output}", exitCode, detail); return OperationResult.CreateFailure( - $"Process exited immediately with code {exitCode}. {detail}"); + $"Process exited during startup with code {exitCode}. {detail}"); } else { @@ -337,17 +396,18 @@ public async Task> StartProcessAsync(GameLaunch var suffix = string.IsNullOrWhiteSpace(stderrTail) ? string.Empty : $" {stderrTail}"; logger.LogError( - "[Process] Process exited immediately with code 0 and no spawned process was found. Output: {Output}", + "[Process] Process exited during startup with code 0 and no spawned process was found. Output: {Output}", string.IsNullOrWhiteSpace(stderrTail) ? "No output was captured." : stderrTail); // Still a failure: without a process to track the UI would sit in 'running'. return OperationResult.CreateFailure( - $"Process exited immediately after launch.{suffix}"); + $"Process exited during startup, shortly after launch.{suffix}"); } } } _managedProcesses[process.Id] = process; + _stderrBuffers[process.Id] = capturedErrors; if (configuration.WaitForExit) { @@ -456,6 +516,10 @@ public async Task> TerminateProcessAsync(int processId, Ca { logger.LogInformation("[Terminate] Force killing process {ProcessId} and its process tree", processId); + // Marked before the kill so the exit event this triggers is classified + // as a requested termination, not a crash. + _requestedTerminations[processId] = 1; + // Run Kill() on a background thread to prevent UI freeze await Task.Run(() => process.Kill(entireProcessTree: true), cancellationToken); @@ -701,6 +765,8 @@ public void CleanupDeadProcesses() foreach (var processId in deadProcessIds) { _managedProcesses.TryRemove(processId, out _); + _stderrBuffers.TryRemove(processId, out _); + _requestedTerminations.TryRemove(processId, out _); logger.LogTrace("Cleaned up dead process {ProcessId} from managed processes", processId); } @@ -739,6 +805,8 @@ public void Dispose() } _managedProcesses.Clear(); + _stderrBuffers.Clear(); + _requestedTerminations.Clear(); _terminationSemaphore.Dispose(); _disposed = true; @@ -791,6 +859,77 @@ private static bool HasExecutePermission(string path) } } + /// + /// Waits for the process to exit, up to the post-spawn detection window. + /// + /// + /// The window bounds how long a launch report can be delayed, not how long a failure + /// can be detected: a process that outlives it is treated as launched, and any later + /// abort surfaces through . Cancellation requested by the + /// caller propagates; the window elapsing does not. + /// + /// The just-started process. + /// The caller's cancellation token. + /// true when the process exited within the window. + private static async Task WaitForExitWithinWindowAsync(Process process, CancellationToken cancellationToken) + { + using var windowCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + windowCts.CancelAfter(ProcessConstants.PostSpawnExitDetectionWindowMs); + + try + { + await process.WaitForExitAsync(windowCts.Token); + return true; + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + // The window elapsed. Re-check rather than assume: the process may have + // exited in the race between the timer firing and the wait observing it. + return process.HasExited; + } + } + + /// + /// Extracts the archive paths named by the engine's mount-failure stderr sentinels. + /// + /// + /// Strictly advisory. The sentinels are an external contract with the fork engine + /// (see ) + /// and no other build emits them, so an empty result must never influence whether the + /// launch is judged to have failed — it only leaves the generic stderr tail in place. + /// + /// The captured stderr lines. + /// The distinct archives named, in order of first appearance. + private static IReadOnlyList ExtractUnmountableArchives(IReadOnlyList stderrLines) + { + string[] sentinelPrefixes = + [ + RetailArchiveConstants.ArchiveMountFailedStderrPrefix, + RetailArchiveConstants.ArchiveIdentifierMismatchStderrPrefix, + ]; + + var archives = new List(); + foreach (var line in stderrLines) + { + foreach (var prefix in sentinelPrefixes) + { + var index = line.IndexOf(prefix, StringComparison.Ordinal); + if (index < 0) + { + continue; + } + + var archive = line[(index + prefix.Length)..].Trim(); + if (archive.Length > 0 && !archives.Contains(archive)) + { + archives.Add(archive); + } + } + } + + return archives; + } + private void OnProcessExited(object? sender, EventArgs e) { if (sender is not Process process) @@ -810,12 +949,41 @@ private void OnProcessExited(object? sender, EventArgs e) // Remove from managed processes _managedProcesses.TryRemove(processId, out _); + var terminationRequested = _requestedTerminations.TryRemove(processId, out _); + + // Attach the stderr capture, when this manager started the process itself. This + // is what makes an abort that outlived the detection window explicable: the exit + // is already after "launched", so the event is the only place the evidence fits. + string? stderrTail = null; + IReadOnlyList unmountableArchives = []; + if (_stderrBuffers.TryRemove(processId, out var capturedErrors)) + { + DrainStandardError(process, capturedErrors); + + var tail = capturedErrors.ToString(); + stderrTail = string.IsNullOrWhiteSpace(tail) ? null : tail; + unmountableArchives = ExtractUnmountableArchives(capturedErrors.Snapshot()); + } + + if (!terminationRequested && exitCode is int code && code != ProcessConstants.ExitCodeSuccess) + { + logger.LogWarning( + "Process {ProcessId} exited with non-zero code {ExitCode} after the launch was reported as started. Archives: {Archives}. Output: {Output}", + processId, + code, + unmountableArchives.Count > 0 ? string.Join(", ", unmountableArchives) : "none named", + stderrTail ?? "No output was captured."); + } + // Raise the event var args = new GameProcessExitedEventArgs { ProcessId = processId, ExitCode = exitCode, ExitTime = DateTime.UtcNow, + StandardErrorTail = stderrTail, + UnmountableArchives = unmountableArchives, + TerminationRequested = terminationRequested, }; ProcessExited?.Invoke(this, args); @@ -989,6 +1157,23 @@ internal bool EndOfStreamReached } } + /// + /// Returns the retained lines, head first, for line-oriented matching. + /// + /// + /// joins lines for display; matching against that joined + /// form would let one line's content bleed into the next. Lines dropped by the + /// bounds are gone from here too, which is acceptable for an advisory match. + /// + /// A snapshot of the retained lines. + internal IReadOnlyList Snapshot() + { + lock (_gate) + { + return [.. _head, .. _tail]; + } + } + /// /// Appends a line, or records end of stream when is null. /// diff --git a/GenHub/GenHub/Features/GameProfiles/ViewModels/GameProfileLauncherViewModel.cs b/GenHub/GenHub/Features/GameProfiles/ViewModels/GameProfileLauncherViewModel.cs index 838cb2241..ebd8b9818 100644 --- a/GenHub/GenHub/Features/GameProfiles/ViewModels/GameProfileLauncherViewModel.cs +++ b/GenHub/GenHub/Features/GameProfiles/ViewModels/GameProfileLauncherViewModel.cs @@ -1596,6 +1596,19 @@ private void OnProcessExited(object? sender, Core.Models.Events.GameProcessExite profile.IsProcessRunning = false; profile.ProcessId = 0; logger.LogInformation("Updated profile {ProfileName} - process no longer running", profile.Name); + + // The late-failure channel's user-facing end. A launch that outlived the + // post-spawn detection window was announced as running; if it then died + // abnormally, clearing the running state alone would leave the user with + // a game that silently vanished. Same channel as a failed launch: + // status, error message, and a notification. + var failureReason = e.DescribeFailure(); + if (failureReason != null) + { + StatusMessage = $"{profile.Name} exited unexpectedly"; + ErrorMessage = failureReason; + notificationService.ShowError("Game Exited Unexpectedly", $"{profile.Name}: {failureReason}"); + } } } catch (Exception ex) diff --git a/GenHub/GenHub/Features/Launching/LaunchRegistry.cs b/GenHub/GenHub/Features/Launching/LaunchRegistry.cs index 4ab36c1aa..f3f8cc71f 100644 --- a/GenHub/GenHub/Features/Launching/LaunchRegistry.cs +++ b/GenHub/GenHub/Features/Launching/LaunchRegistry.cs @@ -18,11 +18,49 @@ namespace GenHub.Features.Launching; /// public class LaunchRegistry : ILaunchRegistry { + /// + /// How long an exit event that matched no launch is kept for a late registration. + /// + /// + /// The gap this covers is the time between a start operation returning and the launcher + /// updating the registry — milliseconds. Two seconds is three orders of magnitude of + /// margin against that while keeping the window small, because the buffer is keyed by + /// PID and Windows recycles PIDs: a stale event drained by a later launch that happened + /// to receive the same PID would mark a running game as terminated. + /// + private static readonly TimeSpan PendingExitRetention = TimeSpan.FromSeconds(2); + private readonly ILogger _logger; private readonly IWorkspaceManager? _workspaceManager; private readonly IGameProcessManager? _processManager; private readonly ConcurrentDictionary _activeLaunches = new(); + /// + /// Exit events whose PID matched no registered launch when they arrived, keyed by PID. + /// + /// + /// The launcher registers a placeholder entry (PID -1) before spawning and records + /// the real PID only after the start operation returns. A process that dies inside + /// that gap raises its exit event while the registry still cannot match it, so the + /// event — including the late-failure evidence — would be silently lost. It is kept + /// here briefly instead and applied when a launch is registered with that PID. + /// + private readonly ConcurrentDictionary _pendingExits = new(); + + /// + /// Makes the two compound sequences around the pending-exit buffer atomic: the exit + /// handler's lookup-then-buffer and registration's install-PID-then-drain-buffer. + /// + /// + /// Without it there is a stranding interleaving: the handler's lookup misses, the + /// registration installs the real PID and drains a still-empty buffer, and only then + /// does the handler's buffer write land — leaving the event to expire unapplied and + /// the failure evidence lost. A lock is used rather than a lock-free double-check + /// because both sequences are short and run at most a handful of times per launch, + /// so contention is irrelevant, and the atomicity is auditable at a glance. + /// + private readonly object _exitSync = new(); + /// /// Initializes a new instance of the class. /// @@ -44,6 +82,17 @@ public LaunchRegistry( } } + /// + /// Gets or sets a test seam invoked between the exit handler's missed launch lookup + /// and its buffer write, inside the synchronization that makes the two atomic. + /// + /// + /// Exists so a test can start a registration in exactly the window where the + /// stranding interleaving would occur without the lock, and prove the exit event is + /// still applied. Never set in production. + /// + internal Action? PendingExitBufferingHook { get; set; } + /// /// Registers a new game launch in the registry. /// @@ -54,8 +103,30 @@ public Task RegisterLaunchAsync(GameLaunchInfo launchInfo) ArgumentNullException.ThrowIfNull(launchInfo); ArgumentException.ThrowIfNullOrWhiteSpace(launchInfo.LaunchId); - _activeLaunches[launchInfo.LaunchId] = launchInfo; - _logger.LogInformation("Registered launch {LaunchId} for profile {ProfileId}", launchInfo.LaunchId, launchInfo.ProfileId); + // Install-then-drain must be atomic against the exit handler's lookup-then- + // buffer, or an exit landing between the two strands in the buffer while the + // launch it belongs to sits registered and running forever. + lock (_exitSync) + { + _activeLaunches[launchInfo.LaunchId] = launchInfo; + _logger.LogInformation("Registered launch {LaunchId} for profile {ProfileId}", launchInfo.LaunchId, launchInfo.ProfileId); + + // The process may have exited before this registration carried its real PID — + // the placeholder-PID gap. Apply the buffered exit now so the failure is + // recorded rather than lost to the race. + var processId = launchInfo.ProcessInfo.ProcessId; + if (processId > 0 + && _pendingExits.TryRemove(processId, out var pendingExit) + && DateTime.UtcNow - pendingExit.ExitTime <= PendingExitRetention) + { + _logger.LogInformation( + "[LaunchRegistry] Applying buffered exit event for PID {ProcessId} to newly registered launch {LaunchId}", + processId, + launchInfo.LaunchId); + ApplyProcessExit(launchInfo, pendingExit); + } + } + return Task.CompletedTask; } @@ -117,23 +188,105 @@ private void OnProcessExited(object? sender, Core.Models.Events.GameProcessExite { _logger.LogInformation("[LaunchRegistry] Received process exit event for PID {ProcessId}", e.ProcessId); - // Find launch info by process ID - var launch = _activeLaunches.Values.FirstOrDefault(l => l.ProcessInfo.ProcessId == e.ProcessId); - if (launch != null) + // Lookup-then-buffer must be atomic against registration's install-then-drain: + // otherwise a registration slipping between the missed lookup and the buffer + // write drains an empty buffer, and this event strands until it expires. + lock (_exitSync) { - _logger.LogInformation("[LaunchRegistry] Updating launch {LaunchId} as terminated", launch.LaunchId); + // Find the live launch holding this PID. Terminated launches keep their PID in + // the registry, so a recycled PID would otherwise match the dead launch first + // and lose the event to the idempotency guard rather than applying it to the + // live launch the PID now belongs to. + var launch = _activeLaunches.Values.FirstOrDefault( + l => l.ProcessInfo.ProcessId == e.ProcessId && !l.TerminatedAt.HasValue); + if (launch != null) + { + ApplyProcessExit(launch, e); + return; + } - // e.ExitTime might be non-nullable DateTime - if (e.ExitTime != default) + // Only a terminated launch holds this PID, so this is the second delivery of an + // exit already applied through the buffer. Absorb it: buffering it here would + // hand a spent event to whichever launch next receives this PID. + if (_activeLaunches.Values.Any(l => l.ProcessInfo.ProcessId == e.ProcessId)) { - launch.TerminatedAt = e.ExitTime; + return; } - else + + // No launch knows this PID. Registration with the real PID may still be in + // flight — the launcher only updates the placeholder entry after the start + // operation returns — so keep the event briefly instead of dropping it. + if (e.ProcessId > 0) { - launch.TerminatedAt = DateTime.UtcNow; + PendingExitBufferingHook?.Invoke(); + PruneExpiredPendingExits(); + _pendingExits[e.ProcessId] = e; + _logger.LogDebug( + "[LaunchRegistry] No launch matches PID {ProcessId} yet; buffering the exit event in case a registration is in flight", + e.ProcessId); } + } + } + + /// + /// Applies an exit event to a launch: termination state, exit code, and — for a + /// non-zero exit — the retroactive failure record. + /// + /// + /// Idempotent. The placeholder-PID race means the same exit can be seen twice — once + /// buffered and applied at registration, once delivered against the registered PID — + /// and the second application must neither duplicate nor contradict the first. An + /// exit code already recorded means the event was applied; a termination already + /// stamped by the polling path is only kept when the event carries nothing more. + /// + /// The launch the process belonged to. + /// The exit event. + private void ApplyProcessExit(GameLaunchInfo launch, Core.Models.Events.GameProcessExitedEventArgs e) + { + if (launch.ExitCode.HasValue || (launch.TerminatedAt.HasValue && e.ExitCode is null)) + { + return; + } + + _logger.LogInformation("[LaunchRegistry] Updating launch {LaunchId} as terminated", launch.LaunchId); - launch.ProcessInfo.IsRunning = false; + // e.ExitTime might be non-nullable DateTime + launch.TerminatedAt = e.ExitTime != default ? e.ExitTime : DateTime.UtcNow; + launch.ProcessInfo.IsRunning = false; + launch.ExitCode = e.ExitCode; + + // The late-failure channel. An initialisation abort slow enough to outlive the + // post-spawn detection window was reported as a started launch; its non-zero + // exit arriving here is the first evidence to the contrary, so the failure is + // recorded retroactively. A clean exit is the user quitting and is never marked + // as failed. + var failureReason = e.DescribeFailure(); + if (failureReason != null) + { + launch.FailureReason = failureReason; + + _logger.LogWarning( + "[LaunchRegistry] Launch {LaunchId} (PID {ProcessId}) failed after it was reported as started: exit code {ExitCode}. {Reason}", + launch.LaunchId, + e.ProcessId, + e.ExitCode, + failureReason); + } + } + + /// + /// Drops buffered exit events old enough that applying them would risk matching a + /// recycled PID rather than the process that produced them. + /// + private void PruneExpiredPendingExits() + { + var cutoff = DateTime.UtcNow - PendingExitRetention; + foreach (var kvp in _pendingExits) + { + if (kvp.Value.ExitTime < cutoff) + { + _pendingExits.TryRemove(kvp.Key, out _); + } } }