diff --git a/src/UniGetUI.Avalonia/App.axaml.cs b/src/UniGetUI.Avalonia/App.axaml.cs index a508f1dfbb..593850905c 100644 --- a/src/UniGetUI.Avalonia/App.axaml.cs +++ b/src/UniGetUI.Avalonia/App.axaml.cs @@ -119,12 +119,60 @@ private static void StartMainWindow(IClassicDesktopStyleApplicationLifetime desk // AppIcon (scripts/macos/AppIcon.icon → Assets.car, via CFBundleIconName) and rendered by // the system — for packaged releases and for Debug builds, which also build into a .app // (see UniGetUI.Avalonia.csproj). There is nothing to do at runtime. - ProcessEnvironmentConfigurator.PrepareForCurrentPlatform(); + // + // Only macOS reads its environment from a login shell, so only macOS finishes startup + // asynchronously; every other platform stays on the synchronous path below. + ResumeStartupAfterMacOSEnvironment(desktop, splash); + return; + } + + ProcessEnvironmentConfigurator.ApplyProxySettingsToProcess(); + CreateAndShowMainWindow(desktop, splash); + } + + /// + /// #5236: resolving PATH spawns a login shell that can be slow, or stuck for good. Keep it off + /// the UI thread so the splash keeps painting, then finish startup once it answers. + /// + /// + /// `async void` on purpose: it hands failures to Dispatcher.UnhandledException (and from there + /// to the crash handler), whereas a dropped Task would swallow them. + /// + private static async void ResumeStartupAfterMacOSEnvironment( + IClassicDesktopStyleApplicationLifetime desktop, SplashWindow? splash) + { + // The dispatcher keeps pumping meanwhile, so the app can be asked to quit before the main + // window exists. Nothing routes that through MainWindow.QuitApplication() yet, so watch for + // it here and abort instead of resurrecting a window on a lifetime that is shutting down. + bool quitRequested = false; + void MarkQuitRequested(object? _, EventArgs __) => quitRequested = true; + + desktop.ShutdownRequested += MarkQuitRequested; + desktop.Exit += MarkQuitRequested; + try + { + await Task.Run(ProcessEnvironmentConfigurator.PrepareForCurrentPlatform); } - else + finally { - ProcessEnvironmentConfigurator.ApplyProxySettingsToProcess(); + desktop.ShutdownRequested -= MarkQuitRequested; + desktop.Exit -= MarkQuitRequested; } + + if (quitRequested) + { + Logger.Warn("The application was asked to quit before startup completed; " + + "the main window will not be created"); + splash?.Close(); + return; + } + + CreateAndShowMainWindow(desktop, splash); + } + + private static void CreateAndShowMainWindow( + IClassicDesktopStyleApplicationLifetime desktop, SplashWindow? splash) + { PEInterface.LoadLoaders(); var mainWindow = new MainWindow(); desktop.MainWindow = mainWindow; diff --git a/src/UniGetUI.Avalonia/Infrastructure/ProcessEnvironmentConfigurator.cs b/src/UniGetUI.Avalonia/Infrastructure/ProcessEnvironmentConfigurator.cs index 469b374798..7abe3ce280 100644 --- a/src/UniGetUI.Avalonia/Infrastructure/ProcessEnvironmentConfigurator.cs +++ b/src/UniGetUI.Avalonia/Infrastructure/ProcessEnvironmentConfigurator.cs @@ -1,11 +1,16 @@ using System.Diagnostics; using UniGetUI.Core.Logging; using UniGetUI.Core.SettingsEngine; +using UniGetUI.Core.Tools; namespace UniGetUI.Avalonia.Infrastructure; internal static class ProcessEnvironmentConfigurator { + // #5236: a login shell whose startup files never return (a recursive `exec zsh -l`, + // a prompt waiting on input, ...) must not hold up startup forever. + private static readonly TimeSpan LoginShellTimeout = TimeSpan.FromSeconds(5); + public static void PrepareForCurrentPlatform() { if (OperatingSystem.IsMacOS()) @@ -58,28 +63,31 @@ public static void ApplyProxySettingsToProcess() private static void ExpandMacOSPath() { + // This runs on a thread pool thread whose result is awaited from an `async void` + // startup path, so it must never throw: a faulty PATH is not worth a crash. try { - using var process = new Process + var startInfo = new ProcessStartInfo("zsh", ["-l", "-c", "printenv PATH"]) { - StartInfo = new ProcessStartInfo("zsh", ["-l", "-c", "printenv PATH"]) - { - UseShellExecute = false, - RedirectStandardOutput = true, - CreateNoWindow = true, - }, + UseShellExecute = false, + RedirectStandardOutput = true, + CreateNoWindow = true, }; - process.Start(); - string shellPath = process.StandardOutput.ReadToEnd().Trim(); - process.WaitForExit(5000); - if (!string.IsNullOrEmpty(shellPath)) + + if (CoreTools.TryReadStandardOutput(startInfo, LoginShellTimeout, out string shellPath) + && shellPath.Length > 0) { Environment.SetEnvironmentVariable("PATH", shellPath); + return; } + + Logger.Warn("Could not read PATH from the login shell; keeping the PATH inherited from the " + + "launcher. Package managers installed outside the system directories may not be found."); } - catch + catch (Exception ex) { - // Keep the existing PATH if the shell can't be launched. + Logger.Error("Failed to expand the PATH from the login shell:"); + Logger.Error(ex); } } } diff --git a/src/UniGetUI.Core.Tools.Tests/ProcessOutputTests.cs b/src/UniGetUI.Core.Tools.Tests/ProcessOutputTests.cs new file mode 100644 index 0000000000..7fb11ebdbc --- /dev/null +++ b/src/UniGetUI.Core.Tools.Tests/ProcessOutputTests.cs @@ -0,0 +1,103 @@ +using System.Diagnostics; + +namespace UniGetUI.Core.Tools.Tests +{ + public class ProcessOutputTests + { + [Fact] + public void TryReadStandardOutput_ReturnsTheCommandOutput() + { + ProcessStartInfo startInfo = OperatingSystem.IsWindows() + ? Redirected("cmd.exe", "/c", "echo", "unigetui") + : Redirected("/bin/sh", "-c", "echo unigetui"); + + Assert.True(CoreTools.TryReadStandardOutput(startInfo, TimeSpan.FromSeconds(30), out string output)); + Assert.Equal("unigetui", output); + } + + [Fact] + public void TryReadStandardOutput_GivesUpOnAChildThatNeverExits() + { + // #5236: a child holding stdout open (a login shell stuck in a recursive `exec zsh -l`) + // used to block ReadToEnd() forever, so the timeout was never reached. + ProcessStartInfo startInfo = OperatingSystem.IsWindows() + // A child that stays alive and silent holds stdout open exactly like the stuck + // login shell did. Sleeping avoids depending on the network stack or on stdin. + ? Redirected("powershell.exe", "-NoProfile", "-Command", "Start-Sleep", "-Seconds", "60") + : Redirected("/bin/sh", "-c", "sleep 60"); + + var watch = Stopwatch.StartNew(); + bool succeeded = CoreTools.TryReadStandardOutput(startInfo, TimeSpan.FromSeconds(2), out string output); + watch.Stop(); + + Assert.False(succeeded); + Assert.Equal("", output); + Assert.True(watch.Elapsed < TimeSpan.FromSeconds(30), $"Gave up after {watch.Elapsed}"); + } + + [Fact] + public void TryReadStandardOutput_KeepsTheTotalWaitWithinTheTimeout() + { + // A child that drops stdout late but never exits used to cost a second full timeout + // waiting for the exit, so the advertised budget nearly doubled. + // Unix-only: neither cmd nor PowerShell can release their stdout handle mid-script. + if (!OperatingSystem.IsWindows()) + { + ProcessStartInfo startInfo = + Redirected("/bin/sh", "-c", "sleep 3; echo late; exec 1>&-; sleep 60"); + + var watch = Stopwatch.StartNew(); + bool succeeded = + CoreTools.TryReadStandardOutput(startInfo, TimeSpan.FromSeconds(4), out string output); + watch.Stop(); + + Assert.True(succeeded); + Assert.Equal("late", output); + Assert.True(watch.Elapsed < TimeSpan.FromSeconds(5.5), $"Took {watch.Elapsed}"); + } + } + + [Fact] + public void TryReadStandardOutput_StillGivesUpWhenADescendantHoldsTheOutputOpen() + { + // Worst case: the command exits at once but leaves a background process holding stdout, + // so no EOF ever arrives and the reparented descendant is beyond Kill's reach. The + // budget must still be honoured -- losing the output beats hanging on the splash. + // Unix-only: cmd and PowerShell cannot detach a child onto the same stdout handle. + if (!OperatingSystem.IsWindows()) + { + ProcessStartInfo startInfo = Redirected("/bin/sh", "-c", "echo orphaned; sleep 5 &"); + + var watch = Stopwatch.StartNew(); + bool succeeded = + CoreTools.TryReadStandardOutput(startInfo, TimeSpan.FromSeconds(2), out string output); + watch.Stop(); + + Assert.False(succeeded); + Assert.Equal("", output); + Assert.True(watch.Elapsed < TimeSpan.FromSeconds(4), $"Took {watch.Elapsed}"); + } + } + + [Fact] + public void TryReadStandardOutput_ReturnsFalseWhenTheCommandDoesNotExist() + { + Assert.False(CoreTools.TryReadStandardOutput( + Redirected("unigetui-this-command-does-not-exist"), TimeSpan.FromSeconds(30), out string output)); + Assert.Equal("", output); + } + + private static ProcessStartInfo Redirected(string fileName, params string[] args) + { + var startInfo = new ProcessStartInfo(fileName) + { + UseShellExecute = false, + RedirectStandardOutput = true, + CreateNoWindow = true, + }; + foreach (string arg in args) + startInfo.ArgumentList.Add(arg); + return startInfo; + } + } +} diff --git a/src/UniGetUI.Core.Tools/Tools.cs b/src/UniGetUI.Core.Tools/Tools.cs index 16b571a329..2350e2f0a7 100644 --- a/src/UniGetUI.Core.Tools/Tools.cs +++ b/src/UniGetUI.Core.Tools/Tools.cs @@ -966,6 +966,84 @@ DictionaryEntry env in Environment.GetEnvironmentVariables( return info; } + /// + /// Runs a command and returns its trimmed standard output. is the + /// total budget; the command and the children it still owns are killed once it elapses. + /// + /// + /// A descendant that outlives the command itself cannot be reached: the OS reparents it as + /// soon as the root exits, so it stays out of the process tree while still holding the + /// output pipe open. Such a call times out and reports failure instead of returning output. + /// + public static bool TryReadStandardOutput( + ProcessStartInfo startInfo, + TimeSpan timeout, + out string output + ) + { + output = ""; + Process? process = null; + Task? reader = null; + var budget = Stopwatch.StartNew(); + try + { + process = Process.Start(startInfo); + if (process is null) + return false; + + // StandardOutput.ReadToEnd() blocks until the child closes stdout, which a hung + // child never does, making any later WaitForExit(timeout) unreachable. Wait on + // the read itself instead, and kill the child so the pipe gets released. + reader = process.StandardOutput.ReadToEndAsync(); + if (!reader.Wait(timeout)) + { + // An exited root means a descendant inherited stdout and is holding it open; + // it is already reparented, so killing the tree below cannot reach it. + string cause = process.HasExited + ? "it exited but something it started still holds its output open" + : "it did not respond in time and will be terminated"; + Logger.Warn( + $"Could not read the output of '{startInfo.FileName}' within " + + $"{timeout.TotalSeconds:0.#}s: {cause}" + ); + return false; + } + + // stdout reached EOF, so the output is complete whether or not the child has left + // yet. Give it what remains of the budget to exit on its own; the finally kills it + // otherwise, so a child that drops stdout early cannot stretch the wait past it. + TimeSpan remaining = timeout - budget.Elapsed; + if (remaining > TimeSpan.Zero) + process.WaitForExit((int)remaining.TotalMilliseconds); + + output = reader.Result.Trim(); + return true; + } + catch (Exception ex) + { + Logger.Warn($"Could not read the output of '{startInfo.FileName}':"); + Logger.Warn(ex); + return false; + } + finally + { + try + { + if (process is not null && !process.HasExited) + process.Kill(entireProcessTree: true); + } + catch + { + // Best effort: the process may have exited on its own in the meantime. + } + + // Dispose() leaves StandardOutput open (we read it in sync mode), so an abandoned + // read ends by itself once the killed child's pipe hits EOF. Observe it regardless. + reader?.ContinueWith(static t => _ = t.Exception, TaskScheduler.Default); + process?.Dispose(); + } + } + /// /// Pings the update server and 3 well-known sites to check for internet availability ///