diff --git a/docs/manual-checks.md b/docs/manual-checks.md index a009239..c0e77e9 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -252,6 +252,34 @@ registered in the capture phase, that no native menu is drawn once it is prevent `ClipboardWriter` and `ClipboardReader` in each `App.axaml.cs` — and the view model is deliberately silent when neither is there, because there is no status line on this screen to say it on. +### 1.12 Something is on screen while it starts + +Close DodoSSH and start it again. Watch the first second. + +**Pass:** a small panel — about 420 pixels wide, no titlebar — appears saying **DodoSSH** and *Starting…* +with a bar moving under it, and is then replaced by the main window. Measured on a debug build against a cold +profile: the panel at 668 ms, the window at 3.1 s. A release install is quicker and the shape is the same. +What must never come back is the old behaviour, which was nothing at all on screen for the whole of it. + +**Pass, and worth checking separately:** the panel is **gone** once the window is up, there is never a moment +with two windows both claiming to be DodoSSH, and clicking another application while the panel is up puts +that application in front of it. The last one is deliberate — a splash that stays on top is a splash sitting +over whatever you gave up and went back to. + +**And the failure state, which is the part with something to get wrong.** It needs provoking: put a +*directory* where `cache.db` should be — `%LOCALAPPDATA%\DodoSSH.Nightly\cache.db` for the nightly — and +start the application. + +**Pass:** the panel stays, the bar stops moving, and it reads *DodoSSH could not start:* followed by the +reason and *Press Escape to close.* Escape closes it. Remove the directory afterwards. + +**Failure means:** a panel left saying *Starting…* is the main window having failed to show with nothing +catching it, which is the one outcome worse than the crash it replaced — `DodoSshApp.ComposeBehind` exists to +make it impossible. Two windows at once is the show-then-close order in that method having been swapped: +closing the only open window ends the process, so the real window is shown while the panel is still up. +Nothing appearing at all means composition is running in the same dispatcher turn that has to paint the +panel, which is what posting it at `DispatcherPriority.Background` prevents. + --- ## Phase 2 — Known Hosts as its own page @@ -2890,6 +2918,43 @@ offer a nightly, ever, however many have been published since. It reads a differ prereleases; if a nightly version is ever offered there, stop and treat it as a release-channel incident rather than as a bug in the nightly. +### 16.11 One instance, and what a second launch does instead + +With DodoSSH running, start it again — Start menu, desktop shortcut, or the executable directly. Do it three +times: with the window in front, with it minimised, and with it behind another application. + +**Pass:** no second window appears, and the one already open comes forward, un-minimised if it was +minimised. **A flashing taskbar button rather than a raised window is also a pass**: Windows refuses the +foreground to a process that did not receive the input, which is its own rule about focus stealing and not +something to defeat. Nothing happening at all is the failure. + +**And the side-by-side case, which is the whole reason the lock lives where it does.** 16.10's arrangement, +both builds installed, both started. + +**Pass:** both run. They are separate applications with separate profiles, and the lock is a file inside the +profile — so neither can see the other's. Two people signed in to one machine is the same property from the +other direction and needs no check of its own. + +**And the update relaunch · needs a release.** Take an update (16.7) and press RESTART NOW. + +**Pass:** the application comes back. This is the race worth naming: the updater starts the new build as the +old one is exiting, and a lock held a moment too long would make the new process refuse itself and disappear +— an update that looked like an uninstall. `SingleInstance.Claim` keeps trying for three seconds while the +lock is held by something that does not answer its pipe, which is what covers it. + +**Failure means:** a second window means the lock is not being taken. A profile directory that cannot be +written to answers "you are the only one" on purpose — refusing to start an SSH client over a permissions +problem would be the wrong trade — so check that the profile exists, is writable, and has an +`instance.lock` in it while the application runs. A second launch that exits without raising the first means +the lock worked and the doorbell did not: the pipe name is derived from the profile path, so two processes +disagreeing about it means they disagree about which profile they are in. Two windows *after an update* is +the opposite failure — the outgoing process never released the lock, which means something is holding it +open past `Shutdown`. + +`SingleInstanceTests` covers everything reachable without a second process: the lock, its release, the +doorbell answering more than once, and two profiles not excluding each other. What is left for a person is a +second *executable* and a real window coming forward. + --- ## Phase 17 — Installing and updating the phone diff --git a/src/DodoSSH.Client.App/App.axaml.cs b/src/DodoSSH.Client.App/App.axaml.cs index 69e3e85..d5f7b6f 100644 --- a/src/DodoSSH.Client.App/App.axaml.cs +++ b/src/DodoSSH.Client.App/App.axaml.cs @@ -3,6 +3,7 @@ using Avalonia.Controls; using Avalonia.Controls.ApplicationLifetimes; using Avalonia.Input.Platform; using Avalonia.Markup.Xaml; +using Avalonia.Threading; using DodoSSH.Client.App.Platform; using DodoSSH.Client.App.Views; using DodoSSH.Client.Auth; @@ -33,12 +34,77 @@ internal sealed partial class DodoSshApp : Application { if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop) { - Compose(desktop); + ShowSomethingWhileComposing(desktop); } base.OnFrameworkInitializationCompleted(); } + /// + /// Puts a window on screen first, and composes the application behind it. + /// + /// + /// + /// The splash is the main window for a moment, which is what makes this work at all. The + /// desktop lifetime shows whatever MainWindow holds once this method returns and then runs the + /// message loop; a window created here and merely shown would be a window with no lifetime attached, and + /// with ShutdownMode.OnLastWindowClose the first close of anything would end the process. Handing + /// the property over is one assignment, and Avalonia reads it lazily. + /// + /// + /// And composition is posted rather than called. It is synchronous and takes long enough to be + /// worth a splash in the first place — which is the whole argument for this method — so running it here + /// would block the very dispatcher turn that has to paint the thing. Background priority puts it behind + /// rendering, which is what makes this a painted window rather than merely a shown one. + /// + /// + private static void ShowSomethingWhileComposing(IClassicDesktopStyleApplicationLifetime desktop) + { + var splash = new SplashWindow(); + + desktop.MainWindow = splash; + + // Fully qualified, and it has to be: Application inherits an instance Dispatcher property from + // AvaloniaObject, member lookup beats the namespace, and a static method here cannot reach an + // instance property — so the unqualified name is a compile error rather than the type. Same below. + Avalonia.Threading.Dispatcher.UIThread.Post( + () => ComposeBehind(desktop, splash), + DispatcherPriority.Background); + } + + /// + /// + /// The order at the end is not interchangeable. ShutdownMode is OnLastWindowClose, so the + /// real window is shown while the splash is still open and the splash is closed afterwards; closing the + /// only window there is would end the process instead of handing it over. + /// + /// + /// A failure is caught rather than left to the dispatcher, and the splash is exactly why. Before + /// this method existed a composition that threw took the process with it and left nothing behind. Now + /// there is something to leave behind — so an exception escaping a posted callback would abandon a window + /// saying "Starting…" over a process that had stopped, which is worse than the crash it replaced. What + /// arrives instead is the sentence and a way to close it. + /// + /// + private static void ComposeBehind( + IClassicDesktopStyleApplicationLifetime desktop, + SplashWindow splash) + { + try + { + Compose(desktop); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + splash.CouldNotStart(exception.Message); + + return; + } + + desktop.MainWindow?.Show(); + splash.Close(); + } + /// /// /// Composed by hand rather than through a container. The graph is a handful of objects deep and an @@ -137,12 +203,44 @@ internal sealed partial class DodoSshApp : Application desktop.MainWindow = new MainWindow { DataContext = viewModel }; + // The other half of Program's claim on this profile: a second launch was refused, and this is what + // answers it. Posted onto the UI thread because a pipe accept completes on a thread pool thread and + // a window may only be touched from the thread that owns it — see SingleInstance.Listen. + var others = SingleInstance.Listen( + paths, + () => Avalonia.Threading.Dispatcher.UIThread.Post(() => BringToFront(desktop.MainWindow))); + // Started rather than awaited: the framework's initialisation must not block on a schema // migration. The view model shows its own progress and handles its own failures, which is why // discarding the task here is safe rather than merely convenient. _ = viewModel.StartAsync(CancellationToken.None); - WireShutdown(desktop, viewModel, workspace, caches); + WireShutdown(desktop, viewModel, workspace, caches, others); + } + + /// + /// Raises the window somebody has just asked for by launching the application again. + /// + /// + /// Un-minimised first, because activating a minimised window leaves it minimised and the launch would + /// look ignored. Activate is a request rather than a guarantee — Windows refuses the foreground to + /// a process that did not receive the input, and what happens then is a flashing taskbar button, which is + /// still an answer. That is the platform's own rule about focus stealing and not a thing to work around. + /// + private static void BringToFront(Window? window) + { + if (window is null) + { + return; + } + + if (window.WindowState is WindowState.Minimized) + { + window.WindowState = WindowState.Normal; + } + + window.Show(); + window.Activate(); } /// Composes the shell itself, with everything this platform answers for it. @@ -271,7 +369,8 @@ internal sealed partial class DodoSshApp : Application IClassicDesktopStyleApplicationLifetime desktop, MainWindowViewModel viewModel, TerminalWorkspace workspace, - ClientCacheFactory caches) + ClientCacheFactory caches, + IAsyncDisposable others) { var shuttingDown = false; @@ -290,6 +389,11 @@ internal sealed partial class DodoSshApp : Application await viewModel.DisposeAsync().ConfigureAwait(true); await workspace.DisposeAsync().ConfigureAwait(true); + // Awaited rather than dropped, so the pipe name is free before this process is. It matters for + // one caller and matters a lot there: an update relaunches this application, and a successor + // that found the name still taken would be a second instance refusing itself. + await others.DisposeAsync().ConfigureAwait(true); + caches.Dispose(); desktop.Shutdown(); diff --git a/src/DodoSSH.Client.App/Platform/SingleInstance.cs b/src/DodoSSH.Client.App/Platform/SingleInstance.cs new file mode 100644 index 0000000..4ee2019 --- /dev/null +++ b/src/DodoSSH.Client.App/Platform/SingleInstance.cs @@ -0,0 +1,325 @@ +using System.IO.Pipes; +using System.Security.Cryptography; +using System.Text; +using DodoSSH.Client.Session; + +namespace DodoSSH.Client.App.Platform; + +/// +/// One desktop instance per profile, and a way for a second launch to raise the first. +/// +/// +/// +/// Per profile rather than per machine, which decides two things at once. The lock lives in +/// , so a nightly and a release build never exclude each other — +/// they are separate applications by design, ADR 0013 — and two people signed in to one machine each get +/// their own instance. A global mutex would have had to re-derive both of those from somewhere. +/// +/// +/// Why one instance at all. The cache is a SQLite file with an outbox in it, and the whole design +/// assumes one writer: says so about two machines sharing a profile through a +/// cloud sync client, and two processes on one machine is the same hazard with a shorter path to it. The +/// terminal's loopback listener is the visible half — the second process fails to bind, gets a port of its +/// own or refuses to start a workspace at all — and the cache is the half that would quietly corrupt. +/// +/// +/// A file handle rather than a named mutex, because the operating system releases it when the +/// process ends however it ends. A named mutex abandoned by a killed process is a state both +/// platforms model differently and neither models simply; an exclusive handle is closed by the kernel on +/// Windows and by the file descriptor going away on Unix, so there is no such thing as a stale lock here. +/// +/// +/// And a named pipe for the other half, because exiting silently is not an answer. Somebody who +/// double-clicks the icon and sees nothing happen has been told the application is broken. The second +/// process asks the first to come forward and then leaves, which is what every desktop application does and +/// what makes the exclusion invisible rather than mysterious. +/// +/// +internal static class SingleInstance +{ + /// The one thing a second launch has to say. + /// + /// Compared against a constant rather than parsed, for the reason MainWindow's release-focus + /// message is: one message exists and treating anything else as a command would be a wider door than + /// this needs. Anything may connect to a per-user pipe — it is not a security boundary — so what arrives + /// is checked rather than trusted. + /// + private const string ComeForward = "dodossh.come-forward"; + + /// How long a second launch waits for the first to answer its pipe. + /// + /// Short, because the pipe is local and the process on the other end is either listening or not. It is + /// not a bound on anything slow; it is how long to wait before concluding that whoever holds the lock + /// cannot be talked to. + /// + private const int AnswerTimeoutMilliseconds = 500; + + /// How long to keep trying for a lock that is held by nobody who answers. + /// + /// + /// ◆ This is the window an update relaunch lands in, and without it an update would look like a + /// crash. Velopack's apply ends this process and its updater then starts the new build; the two are + /// ordered — the updater waits on the old process id — but the whole sequence is one where a lock held a + /// moment too long would make the relaunched application exit on the spot, having apparently uninstalled + /// itself. The same window covers an ordinary double-double-click, where the first process holds the file + /// and has not reached its pipe yet. + /// + /// + /// Only reached when nobody answers the pipe. A live instance replies in milliseconds and the second + /// launch is gone before any of this matters. + /// + /// + private static readonly TimeSpan DefaultPatience = TimeSpan.FromSeconds(3); + + /// How long to wait between attempts within that window. + private static readonly TimeSpan RetryDelay = TimeSpan.FromMilliseconds(100); + + /// + /// Claims this profile for this process, or asks whoever holds it to come forward. + /// + /// + /// A handle to hold for the life of the process, or when another instance has it — + /// in which case that instance has already been asked to raise its window and this process should end. + /// + /// The profile to claim. Its directory is created if it is not there. + /// + /// How long to keep trying a lock nobody answers for, or null for . Named + /// by a test so the give-up path does not cost the suite three seconds; the application never passes it. + /// + internal static IDisposable? Claim(ClientPaths paths, TimeSpan? patience = null) + { + ArgumentNullException.ThrowIfNull(paths); + + try + { + paths.EnsureCreated(); + } + catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) + { + // No profile directory means no lock file, and every build before this one started anyway. What + // follows will fail on its own terms and say something better than this could. + return NoClaim.Instance; + } + + // Counted rather than clocked. Thread.Sleep is not a clock and reading one here would be pretending + // otherwise — what this bounds is the number of attempts, and the wall time is whatever the platform + // makes of them. + var attemptsLeft = (int)Math.Ceiling( + (patience ?? DefaultPatience).TotalMilliseconds / RetryDelay.TotalMilliseconds); + + while (true) + { + if (TryTake(paths.InstanceLockFile) is { } taken) + { + return taken; + } + + // Asked before waiting, so the ordinary case — an instance that is up and running — costs one + // round trip on a local pipe rather than the whole window below. + if (AskTheRunningOneToComeForward(paths)) + { + return null; + } + + if (attemptsLeft-- <= 0) + { + // Held by something that will not talk. Giving up rather than starting anyway, because what + // is being protected is a database with one writer, and "probably nobody" is not an answer + // worth corrupting it for. + return null; + } + + Thread.Sleep(RetryDelay); + } + } + + /// + /// Answers the launches that were refused, for as long as the returned handle is held. + /// + /// + /// is invoked on a thread pool thread, because that is where a pipe + /// accept completes. A caller that touches a window has to marshal. + /// + /// The profile this instance claimed. + /// What to do when another launch asks for the window. + internal static IAsyncDisposable Listen(ClientPaths paths, Action comeForward) + { + ArgumentNullException.ThrowIfNull(paths); + ArgumentNullException.ThrowIfNull(comeForward); + + var listener = new Doorbell(PipeNameFor(paths), comeForward); + + listener.Start(); + + return listener; + } + + /// Tells the instance that holds this profile to raise its window. + /// Whether anything was listening. + /// + /// Internal rather than private because it is the half a test can drive: the exclusion is a file handle + /// and proves itself, and this is the part with a protocol in it. + /// + internal static bool AskTheRunningOneToComeForward(ClientPaths paths) + { + ArgumentNullException.ThrowIfNull(paths); + + try + { + using var pipe = new NamedPipeClientStream( + serverName: ".", + PipeNameFor(paths), + PipeDirection.Out); + + pipe.Connect(AnswerTimeoutMilliseconds); + + using var writer = new StreamWriter(pipe, Encoding.UTF8) { AutoFlush = true }; + + writer.WriteLine(ComeForward); + + return true; + } + catch (Exception exception) + when (exception is TimeoutException or IOException or UnauthorizedAccessException) + { + // Nothing there, or nothing willing. The caller decides what that means; here it is only ever + // "no". + return false; + } + } + + /// Takes the lock, or answers null if somebody else has it. + /// + /// The file is left in place rather than deleted on close. FileOptions.DeleteOnClose looks + /// tidier and is worse: it makes the file's existence part of the protocol, so a second process opening + /// it while the first is closing gets a handle to something on its way out. An empty file that is always + /// there and sometimes locked has one fewer state. + /// + private static IDisposable? TryTake(string lockFile) + { + try + { + return new FileStream( + lockFile, + FileMode.OpenOrCreate, + FileAccess.ReadWrite, + FileShare.None, + bufferSize: 1, + FileOptions.None); + } + catch (IOException) + { + // Somebody else holds it. The one expected failure, and the whole point. + return null; + } + catch (UnauthorizedAccessException) + { + // A profile this user cannot write to, which is a broken installation rather than a second + // instance. Answered as a claim so the application starts and fails on something legible. + return NoClaim.Instance; + } + } + + /// + /// The pipe both sides name, derived from the profile so that it is scoped exactly as the lock is. + /// + /// + /// A digest of the path rather than the path itself: a pipe name may not contain a directory separator + /// on Windows, and on Unix the name becomes a file in a temporary directory whose length is bounded. + /// Eight bytes of SHA-256, which is not doing security work — it is making one string out of another + /// deterministically, and string.GetHashCode cannot because it is randomised per process. + /// + private static string PipeNameFor(ClientPaths paths) + { + var digest = SHA256.HashData(Encoding.UTF8.GetBytes(paths.DataDirectory)); + + return "dodossh-" + Convert.ToHexStringLower(digest.AsSpan(0, 8)); + } + + /// A claim over nothing, for the cases where there was nothing to claim. + private sealed class NoClaim : IDisposable + { + internal static NoClaim Instance { get; } = new(); + + public void Dispose() + { + // Nothing was taken. + } + } + + /// The accept loop that answers refused launches. + /// + /// One server instance at a time and a fresh stream per connection, which is the shape + /// wants: a stream that has served a client cannot serve another. + /// The loop is what makes a third and fourth launch work as well as the second. + /// + private sealed class Doorbell : IAsyncDisposable + { + private readonly string name; + private readonly Action comeForward; + private readonly CancellationTokenSource lifetime = new(); + + private Task? loop; + + internal Doorbell(string name, Action comeForward) + { + this.name = name; + this.comeForward = comeForward; + } + + internal void Start() => loop = RunAsync(lifetime.Token); + + /// + public async ValueTask DisposeAsync() + { + await lifetime.CancelAsync().ConfigureAwait(false); + + if (loop is { } running) + { + // Awaited rather than abandoned, so the pipe name is free by the time this returns — which + // matters for exactly one caller: an update that is about to relaunch this application and + // would otherwise race its own successor for the name. + await running.ConfigureAwait(false); + } + + lifetime.Dispose(); + } + + private async Task RunAsync(CancellationToken cancellationToken) + { + while (!cancellationToken.IsCancellationRequested) + { + try + { + using var server = new NamedPipeServerStream( + name, + PipeDirection.In, + maxNumberOfServerInstances: 1, + PipeTransmissionMode.Byte, + PipeOptions.Asynchronous); + + await server.WaitForConnectionAsync(cancellationToken).ConfigureAwait(false); + + using var reader = new StreamReader(server, Encoding.UTF8); + + var said = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false); + + if (string.Equals(said, ComeForward, StringComparison.Ordinal)) + { + comeForward(); + } + } + catch (OperationCanceledException) + { + return; + } + catch (IOException) + { + // A client that connected and hung up, or a name the platform took away. Round again: + // giving up here would leave every later launch with nothing to talk to, which is the + // silent-exit behaviour this class exists to avoid. + } + } + } + } +} diff --git a/src/DodoSSH.Client.App/Program.cs b/src/DodoSSH.Client.App/Program.cs index 23e63e8..94d80ce 100644 --- a/src/DodoSSH.Client.App/Program.cs +++ b/src/DodoSSH.Client.App/Program.cs @@ -37,6 +37,27 @@ internal static class Program KeepTheWebViewProfileOutOfTheInstallDirectory(); + /* + One instance per profile, and nothing above this line is affected by it: the installer's own runs + have already exited inside VelopackApp.Run, and the WebView2 environment variable is a property of + this process rather than of the application. + + Claimed here rather than after Avalonia is configured, so that a second launch starts nothing — no + toolkit, no window, no splash flashing on screen before it thinks better of it. What it does + instead is ask the running instance to come forward, which is why this is not simply an exit. + + The handle is held for as long as Main runs, which is as long as the message loop does. See + SingleInstance for why it is a file rather than a mutex, and for what the profile scoping buys. + */ + var paths = ClientPaths.ForChannel(DesktopChannel.Name); + + using var claim = SingleInstance.Claim(paths); + + if (claim is null) + { + return; + } + BuildAvaloniaApp().StartWithClassicDesktopLifetime(args); } diff --git a/src/DodoSSH.Client.App/Views/SplashCard.axaml b/src/DodoSSH.Client.App/Views/SplashCard.axaml new file mode 100644 index 0000000..803b371 --- /dev/null +++ b/src/DodoSSH.Client.App/Views/SplashCard.axaml @@ -0,0 +1,49 @@ + + + + + + + + + + + + + + + + + + + diff --git a/src/DodoSSH.Client.App/Views/SplashCard.axaml.cs b/src/DodoSSH.Client.App/Views/SplashCard.axaml.cs new file mode 100644 index 0000000..de29d6f --- /dev/null +++ b/src/DodoSSH.Client.App/Views/SplashCard.axaml.cs @@ -0,0 +1,32 @@ +using Avalonia.Controls; + +namespace DodoSSH.Client.App.Views; + +/// What is on screen while the application composes itself. +/// +/// The two methods below are the whole interface, and both are called by +/// DodoSshApp.ComposeBehind and by nothing else. See the control's own markup for why there is no +/// view model behind it. +/// +internal sealed partial class SplashCard : UserControl +{ + public SplashCard() => InitializeComponent(); + + /// Says what is happening, while it still is. + internal void Say(string what) => Line.Text = what; + + /// + /// Says why nothing is going to happen, and stops pretending otherwise. + /// + /// + /// The bar stopping is half the message. A composition that threw leaves this window as the only one + /// there is — there is no main window to hand over to — so an indeterminate bar would go on saying + /// "still going" over a process that has stopped, which is the one thing a splash must never do. + /// + internal void CouldNotStart(string why) + { + Line.Text = why; + Bar.IsIndeterminate = false; + Bar.Value = 0; + } +} diff --git a/src/DodoSSH.Client.App/Views/SplashWindow.axaml b/src/DodoSSH.Client.App/Views/SplashWindow.axaml new file mode 100644 index 0000000..5d7940e --- /dev/null +++ b/src/DodoSSH.Client.App/Views/SplashWindow.axaml @@ -0,0 +1,47 @@ + + + + + + diff --git a/src/DodoSSH.Client.App/Views/SplashWindow.axaml.cs b/src/DodoSSH.Client.App/Views/SplashWindow.axaml.cs new file mode 100644 index 0000000..cf7cb86 --- /dev/null +++ b/src/DodoSSH.Client.App/Views/SplashWindow.axaml.cs @@ -0,0 +1,56 @@ +using Avalonia.Controls; +using Avalonia.Input; + +namespace DodoSSH.Client.App.Views; + +/// The window that is up while the application composes itself. +/// +/// A frame around and nothing else. Its whole lifetime is one call to +/// DodoSshApp.ComposeBehind: shown by the desktop lifetime as the main window, replaced by the real +/// one, closed. The exception is a composition that fails, which is the only case this window outlives. +/// +internal sealed partial class SplashWindow : Window +{ + /// + /// Whether Escape may close this. + /// + /// + /// False while composition is running, and that is not caution for its own sake: this window has no + /// close button, so Escape is the only key it could offer — and closing it mid-composition would leave a + /// process that has opened the cache and bound a socket with no window and no way to reach either. Once + /// there is nothing left to wait for, it is the only way out and has to work. + /// + private bool escapable; + + public SplashWindow() + { + InitializeComponent(); + + KeyDown += (_, e) => + { + if (escapable && e.Key == Key.Escape) + { + Close(); + } + }; + } + + /// Says what is happening, while it still is. + internal void Say(string what) => Card.Say(what); + + /// + /// Says why nothing is going to happen, and becomes the last window there is. + /// + /// + /// The way out is named in the message rather than drawn as a button. A failure here is a broken + /// installation — a cache that cannot be opened, a port that cannot be bound — so what this window is + /// for at that point is carrying one sentence to whoever has to act on it, and a control that has to be + /// laid out and measured to say "close me" earns less than the words do. + /// + internal void CouldNotStart(string why) + { + escapable = true; + + Card.CouldNotStart($"DodoSSH could not start: {why} Press Escape to close."); + } +} diff --git a/src/DodoSSH.Client.Session/ClientPaths.cs b/src/DodoSSH.Client.Session/ClientPaths.cs index 0581b84..e441592 100644 --- a/src/DodoSSH.Client.Session/ClientPaths.cs +++ b/src/DodoSSH.Client.Session/ClientPaths.cs @@ -102,6 +102,26 @@ public sealed record ClientPaths(string DataDirectory) /// public string DeviceKeyFile => Path.Combine(DataDirectory, "device.key"); + /// + /// The file one desktop instance holds open for as long as it is running. + /// + /// + /// + /// Named here rather than in the head that uses it, because every other name in this directory is named + /// here and a filename invented somewhere else is one nobody looking at the profile can account for. The + /// Android head has no use for it: the platform allows one process per package and needs no help. + /// + /// + /// Being in the profile directory is what scopes it correctly, and that is the whole reason it is a + /// file rather than a named mutex. This path is already per user and per channel, so a nightly and a + /// release build do not exclude each other and two people signed into one machine each get their own + /// instance — neither of which a global name would give without repeating the same two decisions in + /// another form. Contents are irrelevant and it stays empty; what carries the meaning is the exclusive + /// handle, which the operating system releases when the process ends however it ends. + /// + /// + public string InstanceLockFile => Path.Combine(DataDirectory, "instance.lock"); + /// Creates the profile directory if it is not there yet. /// /// Separate from resolving the path, because resolving must never have a side effect: it is read diff --git a/tests/DodoSSH.Client.App.Layout.Tests/LayoutHarness.cs b/tests/DodoSSH.Client.App.Layout.Tests/LayoutHarness.cs index 6beee52..c5b3dc4 100644 --- a/tests/DodoSSH.Client.App.Layout.Tests/LayoutHarness.cs +++ b/tests/DodoSSH.Client.App.Layout.Tests/LayoutHarness.cs @@ -51,6 +51,18 @@ internal static class LayoutHarness /// internal const double HostDrawerWidth = 320; + /// The splash window's fixed width, from SplashWindow.axaml. + /// + /// Its height is deliberately absent from this list, because the window has none to state: it is + /// SizeToContent="Height" so that a failure message of any length is readable rather than + /// clipped. is the floor under the ordinary one-line case, and + /// SplashCardTests pins all three against the XAML. + /// + internal const double SplashWidth = 420; + + /// + internal const double SplashMinimumHeight = 160; + /// The nav rail's fixed width, from NavRail.axaml. /// v5b: 190 became 255, the design's own number rather than this bar's old approximation. internal const double NavRailWidth = 255; diff --git a/tests/DodoSSH.Client.App.Layout.Tests/SplashCardTests.cs b/tests/DodoSSH.Client.App.Layout.Tests/SplashCardTests.cs new file mode 100644 index 0000000..56d93f4 --- /dev/null +++ b/tests/DodoSSH.Client.App.Layout.Tests/SplashCardTests.cs @@ -0,0 +1,128 @@ +using Avalonia.Controls; +using DodoSSH.Client.App.Views; + +namespace DodoSSH.Client.App.Layout.Tests; + +/// +/// The splash, and the one thing about it that can go wrong. +/// +/// +/// +/// Three stacked elements in a fixed-width window is not a layout with much to get wrong — except in one +/// state. SplashWindow.CouldNotStart carries an exception's own message, whose length nothing in this +/// application decides, and that message is the whole reason that window is still on screen. Inside a fixed +/// height it would wrap and be clipped at exactly the wrong moment. +/// +/// +/// So what is measured here is that the message grows the card, and that the window is declared in a way +/// that lets the growth through. LayoutHarness.Unreachable is no help: it reports interactive controls +/// arranged where they cannot be used, and the splash has none — which is why these two assert on measured +/// heights instead. +/// +/// +public sealed class SplashCardTests +{ + private static CancellationToken Token => TestContext.Current.CancellationToken; + + /// A message about as long as a real one gets: a path, a reason and a suggestion. + private const string LongFailure = + "The process cannot access the file 'C:\\Users\\somebody\\AppData\\Local\\DodoSSH\\cache.db' " + + "because it is being used by another process, and this build cannot open a second copy of it."; + + /// + /// Pins the two constants and the mechanism against the XAML, the way + /// LayoutHarnessTests.TheHarnessMeasuresTheSizeTheWindowDeclares pins the main window's minimum. + /// SizeToContent is the one that matters: a window that lost it would clip the message and this + /// suite's other test would go on passing, because the card would still be measuring the taller size it + /// was never given. + /// + [Fact] + public async Task TheSplashIsDeclaredTheWayTheHarnessMeasuresIt() + { + await LayoutHarness.OnTheUiThreadAsync( + () => + { + var window = new SplashWindow(); + + try + { + window.Width.ShouldBe(LayoutHarness.SplashWidth); + window.MinHeight.ShouldBe(LayoutHarness.SplashMinimumHeight); + window.SizeToContent.ShouldBe( + SizeToContent.Height, + "a fixed height would clip a long failure message"); + } + finally + { + window.Close(); + } + }, + Token); + } + + /// + /// The ordinary state has to fit the floor, or the window opens taller than its own minimum for the one + /// line it normally shows — which would make a number + /// that never applies to anything. + /// + [Fact] + public async Task TheStartingMessageFitsTheHeightTheSplashOpensAt() + { + await LayoutHarness.OnTheUiThreadAsync( + () => MessageHeight(card => { }).ShouldBeLessThanOrEqualTo( + LayoutHarness.SplashMinimumHeight, + "the splash opens at its minimum and the one line it starts with has to fit"), + Token); + } + + /// + /// ◆ The test with something behind it. A message that wraps to three lines has to make the card ask for + /// more room; if it did not, the text would be sitting under the bottom edge of the window and the only + /// thing the splash exists to carry at that point would be the part nobody could read. + /// + [Fact] + public async Task ALongFailureMessageAsksForMoreRoomRatherThanBeingClipped() + { + await LayoutHarness.OnTheUiThreadAsync( + () => + { + var starting = MessageHeight(card => { }); + var failed = MessageHeight(card => card.CouldNotStart(LongFailure)); + + failed.ShouldBeGreaterThan( + starting, + "the message wraps, so the card has to want the height the window then gives it"); + }, + Token); + } + + /// + /// What one card wants to be, at the splash's width, after the caller has had its say. + /// + /// + /// Hosted in a window before being measured, because the heading and the hint take their size and their + /// wrapping from App.axaml — a card measured detached would be measuring unstyled text. Measured + /// again afterwards against an unbounded height, which is what the real window's SizeToContent + /// gives it and what the host window's own fixed height would otherwise clamp away. + /// + private static double MessageHeight(Action say) + { + var card = new SplashCard(); + + var window = LayoutHarness.HostAtMinimumSize( + card, LayoutHarness.SplashWidth, LayoutHarness.SplashMinimumHeight); + + try + { + say(card); + + card.Measure(new Avalonia.Size(LayoutHarness.SplashWidth, double.PositiveInfinity)); + + return card.DesiredSize.Height; + } + finally + { + window.Close(); + } + } +} diff --git a/tests/DodoSSH.Client.App.Tests/SingleInstanceTests.cs b/tests/DodoSSH.Client.App.Tests/SingleInstanceTests.cs new file mode 100644 index 0000000..9f0ebb1 --- /dev/null +++ b/tests/DodoSSH.Client.App.Tests/SingleInstanceTests.cs @@ -0,0 +1,182 @@ +using DodoSSH.Client.App.Platform; +using DodoSSH.Client.Session; + +namespace DodoSSH.Client.App.Tests; + +/// +/// One desktop instance per profile, and what the second launch does instead of starting. +/// +/// +/// +/// Both halves are real here: a real lock file in a real temporary profile, and a real named pipe. Neither +/// is worth faking — the whole claim being made is about what the operating system does with an exclusive +/// handle and a pipe name, and a fake of either would be a test of the fake. +/// +/// +/// What is not reachable from here is a second process. Every test below runs in one, so the lock +/// is proved against a second handle rather than a second executable, and the window that comes +/// forward is a delegate rather than a window. docs/manual-checks.md 16.11 is the other side of that +/// line. +/// +/// +public sealed class SingleInstanceTests : IDisposable +{ + /// + /// A profile of its own per test, which is what makes these safe to run beside a real installation. + /// + /// + /// The pipe name is derived from this directory — see SingleInstance.PipeNameFor — so a unique + /// directory is also a unique pipe. Without that, a test here would ring the doorbell of whatever + /// DodoSSH the developer happens to have running. + /// + private readonly ClientPaths paths = + new(Path.Combine(Path.GetTempPath(), $"dodossh-instance-{Guid.CreateVersion7():N}")); + + /// + /// No patience at all, which is what every test here wants. + /// + /// + /// The shipped three seconds exist for a lock held by a process on its way out — an update relaunching + /// itself, above all — and nothing here is on its way anywhere. Passing zero takes the give-up decision + /// on the first attempt, so a refusal costs a millisecond rather than three seconds of the suite. + /// + private static readonly TimeSpan Impatient = TimeSpan.Zero; + + /// + public void Dispose() + { + if (Directory.Exists(paths.DataDirectory)) + { + Directory.Delete(paths.DataDirectory, recursive: true); + } + } + + [Fact] + public void TheFirstLaunchClaimsTheProfile() + { + using var claim = SingleInstance.Claim(paths, Impatient); + + claim.ShouldNotBeNull(); + File.Exists(paths.InstanceLockFile).ShouldBeTrue("the claim is a handle on a real file"); + } + + /// + /// The point of the whole exercise. Nobody is listening in this test, so the second claim takes the + /// give-up path rather than the ask-and-leave one — which is why it is handed no patience at all. + /// + [Fact] + public void ASecondLaunchIsRefused() + { + using var first = SingleInstance.Claim(paths, Impatient); + first.ShouldNotBeNull(); + + SingleInstance.Claim(paths, Impatient).ShouldBeNull(); + } + + /// + /// ◆ There is no such thing as a stale lock here, and this is what says so. The handle is closed by the + /// runtime when it is disposed and by the kernel when the process ends however it ends — which is the + /// reason this is a file rather than a named mutex, whose abandonment is a state to be handled. + /// + [Fact] + public void TheProfileIsClaimableAgainOnceTheFirstInstanceLetsGo() + { + var first = SingleInstance.Claim(paths, Impatient); + first.ShouldNotBeNull(); + first.Dispose(); + + using var second = SingleInstance.Claim(paths, Impatient); + + second.ShouldNotBeNull("the lock went with the handle"); + } + + /// + /// The other half: a refused launch is not a silent one. Somebody who double-clicks the icon and sees + /// nothing has been told the application is broken, so the running instance is asked to come forward. + /// + [Fact] + public async Task ARefusedLaunchAsksTheRunningInstanceToComeForward() + { + using var first = SingleInstance.Claim(paths, Impatient); + first.ShouldNotBeNull(); + + var asked = new TaskCompletionSource(); + + await using var doorbell = SingleInstance.Listen(paths, () => asked.TrySetResult()); + + SingleInstance.Claim(paths, Impatient).ShouldBeNull("the profile is taken"); + + // Awaited rather than polled: the callback comes off a thread pool thread, and the test's own + // cancellation is what bounds a doorbell that never rings. + await asked.Task.WaitAsync(TestContext.Current.CancellationToken); + } + + /// + /// A third and a fourth launch work as well as the second, which is what the accept loop is for: a + /// NamedPipeServerStream that has served one client cannot serve another, so a doorbell that + /// forgot to go round again would answer exactly one refused launch and then leave every later one to + /// exit in silence. + /// + [Fact] + public async Task TheDoorbellAnswersMoreThanOnce() + { + using var first = SingleInstance.Claim(paths, Impatient); + first.ShouldNotBeNull(); + + var rings = 0; + var twice = new TaskCompletionSource(); + + await using var doorbell = SingleInstance.Listen( + paths, + () => + { + if (Interlocked.Increment(ref rings) == 2) + { + twice.TrySetResult(); + } + }); + + SingleInstance.AskTheRunningOneToComeForward(paths).ShouldBeTrue(); + SingleInstance.AskTheRunningOneToComeForward(paths).ShouldBeTrue(); + + await twice.Task.WaitAsync(TestContext.Current.CancellationToken); + } + + /// + /// What the give-up path turns on. A lock held by something that is not listening — a process still + /// starting, or one on its way out — answers no, and that is what makes Claim wait and try the + /// lock again rather than concluding anything from the silence. + /// + [Fact] + public void AskingWithNobodyListening_SaysSo() + { + SingleInstance.AskTheRunningOneToComeForward(paths).ShouldBeFalse(); + } + + /// + /// Two profiles, because a nightly and a release build are installed side by side on purpose and must + /// not exclude each other — ADR 0013. The lock file is inside the profile and the pipe name is derived + /// from it, so both halves are scoped by the one decision ClientPaths.ForChannel already made. + /// + [Fact] + public void TwoProfilesDoNotExcludeEachOther() + { + var other = new ClientPaths(paths.DataDirectory + ".Nightly"); + + try + { + using var release = SingleInstance.Claim(paths, Impatient); + using var nightly = SingleInstance.Claim(other, Impatient); + + release.ShouldNotBeNull(); + nightly.ShouldNotBeNull("a second profile is a second application"); + } + finally + { + if (Directory.Exists(other.DataDirectory)) + { + Directory.Delete(other.DataDirectory, recursive: true); + } + } + } +}