Log crashes to the diagnostics file; decode each MXP picture once

CHANGELOG.md

@@ -7,6 +7,17 @@ and a release's notes are its section. The procedure is in [`docs/RELEASING.md`]
 
 ## [Unreleased]
 
+### Added
+
+- A crash now leaves its stack in `logs/client-diagnostics-*.log`, in the window and the terminal alike,
+  along with errors the UI framework used to swallow without a trace.
+
+### Changed
+
+- An MXP picture is downloaded and decoded once per session. A room picture shown on every look used
+  to be fetched again whenever the pane's width had changed, and once per look that arrived while it was
+  still loading.
+
 ## [0.1.5] - 2026-10-25
 
 ### Added

src/SharpMUTerm.Tui/ClientDiagnostics.cs

@@ -120,12 +120,83 @@ public static ClientDiagnostics Create(string folder, LogLevel minimum = LogLeve
         }
     }
 
+    /// <summary>
+    /// Routes the framework's own log into this pipeline, from Warning up, for as long as the result is held.
+    /// <para>
+    /// The framework catches what it runs and logs it to a <see cref="SharpConsoleUI.Logging.ILogService"/> of its
+    /// own that writes nowhere unless an environment variable names a file: every exception thrown by an
+    /// <c>EnqueueOnUIThread</c> action (which is most of this app's cross-thread work, through <c>OnUiThread</c>), and
+    /// the one that ends the main loop. That last one returns 1 from <c>Run</c> and prints its stack to the console
+    /// once the console is restored, which a terminal shows and a window has nowhere to put: the window just closed,
+    /// and no file on the machine said why.
+    /// </para>
+    /// </summary>
+    public IDisposable Forward(SharpConsoleUI.Logging.ILogService framework)
+    {
+        var logger = For("SharpConsoleUI");
+        void OnEntry(object? sender, SharpConsoleUI.Logging.LogEntry entry)
+        {
+            var level = entry.Level switch
+            {
+                SharpConsoleUI.Logging.LogLevel.Critical => LogLevel.Critical,
+                SharpConsoleUI.Logging.LogLevel.Error => LogLevel.Error,
+                SharpConsoleUI.Logging.LogLevel.Warning => LogLevel.Warning,
+                _ => LogLevel.None,
+            };
+            if (level != LogLevel.None)
+            {
+                logger.Log(level, entry.Exception, "{Message}", entry.Message);
+            }
+        }
+
+        framework.LogAdded += OnEntry;
+        return new Subscription(() => framework.LogAdded -= OnEntry);
+    }
+
+    /// <summary>
+    /// Records what would otherwise end the process without a word, for as long as the result is held: an exception
+    /// no thread caught (<see cref="AppDomain.UnhandledException"/>, which also sees one escaping the window's own
+    /// UI thread) and a faulted task nobody awaited (<see cref="TaskScheduler.UnobservedTaskException"/>, which does
+    /// not end the process and is logged as an error). Neither is handled here: the process ends as it would have,
+    /// with the stack in the file first. The file sink is unbuffered, so the line is on disk when the handler returns.
+    /// </summary>
+    public IDisposable RecordUnhandled()
+    {
+        var logger = For("SharpMUTerm.Crash");
+        void OnUnhandled(object? sender, UnhandledExceptionEventArgs e)
+        {
+            logger.LogCritical(
+                e.ExceptionObject as Exception,
+                "Unhandled exception on thread {Thread} (terminating: {Terminating})",
+                Thread.CurrentThread.Name ?? Environment.CurrentManagedThreadId.ToString(),
+                e.IsTerminating);
+        }
+
+        void OnUnobserved(object? sender, UnobservedTaskExceptionEventArgs e) =>
+            logger.LogError(e.Exception, "A task failed and nothing awaited it");
+
+        AppDomain.CurrentDomain.UnhandledException += OnUnhandled;
+        TaskScheduler.UnobservedTaskException += OnUnobserved;
+        return new Subscription(() =>
+        {
+            AppDomain.CurrentDomain.UnhandledException -= OnUnhandled;
+            TaskScheduler.UnobservedTaskException -= OnUnobserved;
+        });
+    }
+
     public void Dispose()
     {
         _factory.Dispose();
         _serilog.Dispose();
     }
 
+    private sealed class Subscription(Action undo) : IDisposable
+    {
+        private Action? _undo = undo;
+
+        public void Dispose() => Interlocked.Exchange(ref _undo, null)?.Invoke();
+    }
+
     private static LogEventLevel ToSerilog(LogLevel level) => level switch
     {
         LogLevel.Trace => LogEventLevel.Verbose,

src/SharpMUTerm.Tui/InlineImageHost.cs

@@ -12,10 +12,17 @@ namespace SharpMUTerm.Tui;
 /// Conservative in the ways a world could abuse it. Fetching goes through <see cref="WebImageLoader"/>'s
 /// rules (http(s) and <c>data:</c>, an image content type, a byte cap, a timeout). At most
 /// <see cref="MaxConcurrentFetches"/> run at once and at most <see cref="MaxQueuedLines"/> lines wait,
-/// so a server spamming tags gets placeholders rather than a client with a hundred sockets open. A
-/// result is kept per URL, presentation and box, so the room map a game re-sends on every <c>look</c>
-/// is fetched once and, under Kitty, transmitted once — later copies are placeholder cells pointing at
-/// the image the terminal already holds.
+/// so a server spamming tags gets placeholders rather than a client with a hundred sockets open.
+/// </para>
+/// <para>
+/// Two caches, because a picture is used at more than one size. The decoded picture is kept per URL
+/// (<see cref="SourceCacheBytes"/>), and a fetch already running is shared, so the room picture a game
+/// re-sends on every <c>look</c> is fetched and decoded once for the session, whatever width the pane is
+/// when it arrives and however many looks land before the first fetch is back. Decoding is the costly
+/// half: a WebP the size of a desktop background is tens of milliseconds of SIMD work each time. The rows
+/// are kept as well, per URL, presentation and box, so a repeat at the same size is not even re-scaled
+/// and, under Kitty, not re-transmitted — later copies are placeholder cells pointing at the image the
+/// terminal already holds.
 /// </para>
 /// </summary>
 internal sealed class InlineImageHost : IDisposable
@@ -29,6 +36,13 @@ internal sealed class InlineImageHost : IDisposable
     /// <summary>Finished results kept for reuse.</summary>
     public const int CacheSize = 32;
 
+    /// <summary>
+    /// Decoded pictures kept for reuse, in bytes of pixels. A 1942×809 room picture is about 6 MB, so this
+    /// holds a session's worth of rooms; the least recently used goes first, and one larger than the whole
+    /// budget is used once and not kept.
+    /// </summary>
+    public const long SourceCacheBytes = 64L * 1024 * 1024;
+
     /// <summary>
     /// First Kitty image id this host hands out. The framework numbers its own images (the web view's)
     /// from 1; starting high keeps the two apart, and every id still fits the 24 bits a placeholder's
@@ -41,14 +55,23 @@ internal sealed class InlineImageHost : IDisposable
     private readonly SemaphoreSlim _fetches = new(MaxConcurrentFetches);
     private readonly CancellationTokenSource _cts = new();
     private readonly LinkedList<(string Key, IReadOnlyList<string> Rows)> _cache = new();
+    private readonly Dictionary<string, LinkedListNode<Source>> _sources = new(StringComparer.Ordinal);
+    private readonly LinkedList<Source> _sourceOrder = new();
+    private readonly long _sourceBudget;
+    private long _sourceBytes;
     private readonly object _gate = new();
     private int _queued;
     private int _nextKittyId = FirstKittyId;
 
     /// <param name="fetch">Fetches an image's bytes, or null.</param>
     /// <param name="onUi">Marshals onto the UI thread.</param>
-    public InlineImageHost(Func<string, CancellationToken, Task<byte[]?>> fetch, Action<Action> onUi)
+    /// <param name="sourceCacheBytes">The decoded-picture budget; <see cref="SourceCacheBytes"/> unless a test says otherwise.</param>
+    public InlineImageHost(
+        Func<string, CancellationToken, Task<byte[]?>> fetch,
+        Action<Action> onUi,
+        long sourceCacheBytes = SourceCacheBytes)
     {
+        _sourceBudget = sourceCacheBytes;
         _fetch = fetch ?? throw new ArgumentNullException(nameof(fetch));
         _onUi = onUi ?? throw new ArgumentNullException(nameof(onUi));
     }
@@ -176,22 +199,8 @@ presentation is not (InlineImagePresentation.Kitty or InlineImagePresentation.Ha
             return hit;
         }
 
-        byte[]? bytes;
-        await _fetches.WaitAsync(_cts.Token).ConfigureAwait(false);
-        try
-        {
-            bytes = await _fetch(request.Url, _cts.Token).ConfigureAwait(false);
-        }
-        catch (OperationCanceledException)
-        {
-            return null;
-        }
-        finally
-        {
-            _fetches.Release();
-        }
-
-        var picture = bytes is null ? null : Decode(bytes, request, availableColumns, maxRows);
+        var source = await SourceFor(request.Url).ConfigureAwait(false);
+        var picture = source is null ? null : Fit(source, request, availableColumns, maxRows);
         if (picture is null)
         {
             return null;
@@ -221,6 +230,125 @@ presentation is not (InlineImagePresentation.Kitty or InlineImagePresentation.Ha
         return rows;
     }
 
+    /// <summary>
+    /// The decoded picture at <paramref name="url"/>: from the cache, from a fetch already under way, or
+    /// fetched and decoded now. Null when it cannot be had, and a failure is not remembered, so the next
+    /// mention tries again.
+    /// </summary>
+    private Task<PixelBuffer?> SourceFor(string url)
+    {
+        lock (_gate)
+        {
+            if (_sources.TryGetValue(url, out var node))
+            {
+                _sourceOrder.Remove(node);
+                _sourceOrder.AddFirst(node);
+                return node.Value.Picture;
+            }
+
+            // In the cache before it starts, so a fetch that finishes at once still finds itself there to
+            // settle; and off this thread, so a fetch that completes at once does not decode under the lock.
+            var source = new Source(url);
+            _sources[url] = _sourceOrder.AddFirst(source);
+            source.Picture = Task.Run(async () =>
+            {
+                PixelBuffer? picture = null;
+                try
+                {
+                    picture = await FetchAndDecodeAsync(url).ConfigureAwait(false);
+                    return picture;
+                }
+                finally
+                {
+                    Settle(source, picture);
+                }
+            });
+            return source.Picture;
+        }
+    }
+
+    private async Task<PixelBuffer?> FetchAndDecodeAsync(string url)
+    {
+        byte[]? bytes;
+        await _fetches.WaitAsync(_cts.Token).ConfigureAwait(false);
+        try
+        {
+            bytes = await _fetch(url, _cts.Token).ConfigureAwait(false);
+        }
+        catch (OperationCanceledException)
+        {
+            return null;
+        }
+        finally
+        {
+            _fetches.Release();
+        }
+
+        if (bytes is null)
+        {
+            return null;
+        }
+
+        try
+        {
+            return ImageDecoder.Decode(bytes);
+        }
+        catch (Exception ex) when (ex is not OutOfMemoryException)
+        {
+            return null;
+        }
+    }
+
+    /// <summary>Accounts for a finished fetch: forgets a failure, and trims the cache to its budget.</summary>
+    private void Settle(Source source, PixelBuffer? picture)
+    {
+        lock (_gate)
+        {
+            if (!_sources.TryGetValue(source.Url, out var node) || !ReferenceEquals(node.Value, source))
+            {
+                return;
+            }
+
+            if (picture is null)
+            {
+                Forget(node);
+                return;
+            }
+
+            source.Bytes = (long)picture.Width * picture.Height * 4;
+            _sourceBytes += source.Bytes;
+            for (var last = _sourceOrder.Last; _sourceBytes > _sourceBudget && last is not null;)
+            {
+                var previous = last.Previous;
+                if (last.Value.Bytes > 0)
+                {
+                    Forget(last);
+                }
+
+                last = previous;
+            }
+        }
+    }
+
+    private void Forget(LinkedListNode<Source> node)
+    {
+        _sourceOrder.Remove(node);
+        _sources.Remove(node.Value.Url);
+        _sourceBytes -= node.Value.Bytes;
+    }
+
+    /// <summary>Decoded pictures held, and fetches under way — for tests.</summary>
+    internal int SourcesHeld
+    {
+        get
+        {
+            lock (_gate)
+            {
+                return _sources.Count;
+            }
+        }
+    }
+
     /// <summary>
     /// Decodes and sizes a picture. The pixels kept are the most either presentation needs: under
     /// Kitty the terminal scales into the box, so anything past <see cref="InlineImageLayout.CellPixelWidth"/>
@@ -242,6 +370,16 @@ presentation is not (InlineImagePresentation.Kitty or InlineImagePresentation.Ha
             return null;
         }
 
+        return Fit(source, request, availableColumns, maxRows);
+    }
+
+    /// <summary>Sizes a decoded picture to its cell box, keeping the pixels either presentation needs.</summary>
+    internal static DecodedPicture? Fit(
+        PixelBuffer source,
+        InlineImageRequest request,
+        int availableColumns,
+        int maxRows = InlineImageLayout.MaxRows)
+    {
         var box = InlineImageLayout.Fit(source.Width, source.Height, request.Width, request.Height, availableColumns, maxRows);
         if (box.Columns <= 0 || box.Rows <= 0)
         {
@@ -364,6 +502,17 @@ private void Remember(string key, IReadOnlyList<string> rows)
         }
     }
 
+    /// <summary>A decoded picture held by the cache, or the fetch that will produce it.</summary>
+    private sealed class Source(string url)
+    {
+        public string Url { get; } = url;
+
+        public Task<PixelBuffer?> Picture { get; set; } = Task.FromResult<PixelBuffer?>(null);
+
+        /// <summary>Its size in the cache's budget; zero while the fetch is under way.</summary>
+        public long Bytes { get; set; }
+    }
+
     public void Dispose()
     {
         _cts.Cancel();

src/SharpMUTerm.Tui/Program.cs

@@ -127,6 +127,10 @@ internal static int RunLive(
         // the World.Character-*.log transcripts). Never a console sink — this app owns the screen.
         using var diagnostics = ClientDiagnostics.Create(logRoot);
 
+        // A crash leaves its stack in that file. Without this an exception nothing caught ended the process with
+        // nothing written anywhere a user could find, which in the window (no console) meant it simply vanished.
+        using var crashes = diagnostics.RecordUnhandled();
+
         // Drain what the configuration load had to say now that there is somewhere to say it. A secrets file
         // that could not be read means characters start with no password — worth one line in the client
         // message log (⌃P), and never more than that: the client still runs, still connects, and the password
@@ -173,7 +177,23 @@ internal static int RunLive(
             logRoot: logRoot,
             restore: restore,
             mssp: mssp);
-        var exitCode = liveApp.Run(startup); // blocks on the SharpConsoleUI main loop until exit
+        int exitCode;
+        try
+        {
+            exitCode = liveApp.Run(startup); // blocks on the SharpConsoleUI main loop until exit
+        }
+        catch (Exception ex)
+        {
+            // Run itself catches what its loop throws; this is what escapes it (startup, shutdown). The window's
+            // client thread catches it next, and has nowhere but a console that does not exist to write it.
+            diagnostics.Logger.LogCritical(ex, "The client stopped on an exception");
+            throw;
+        }
+
+        if (exitCode != 0)
+        {
+            diagnostics.Logger.LogWarning("The client loop ended with exit code {ExitCode}", exitCode);
+        }
 
         // Persist the workspace so the next launch resumes where this one left off.
         try