Merge pull request #155 from ionite34/fix-package-dispose

Fix shutdown errors and AsyncStreamReader sending extra empty string

StabilityMatrix.Avalonia/ViewModels/ConsoleViewModel.cs

@@ -8,6 +8,7 @@
 using AvaloniaEdit.Document;
 using CommunityToolkit.Mvvm.ComponentModel;
 using Nito.AsyncEx;
+using Nito.AsyncEx.Synchronous;
 using NLog;
 using StabilityMatrix.Core.Extensions;
 using StabilityMatrix.Core.Processes;
@@ -454,9 +455,26 @@ public void PostLine(string text)
     public void Dispose()
     {
         updateCts?.Cancel();
-        updateTask?.Dispose();
         updateCts?.Dispose();
 
+        if (updateTask is not null)
+        {
+            try
+            {
+                updateTask.WaitWithoutException(
+                    new CancellationTokenSource(5000).Token);
+                updateTask.Dispose();
+            }
+            catch (OperationCanceledException)
+            {
+                Logger.Error("During shutdown - Console update task cancellation timed out");
+            }
+            catch (InvalidOperationException e)
+            {
+                Logger.Error(e, "During shutdown - Console update task cancellation failed");
+            }
+        }
+
         GC.SuppressFinalize(this);
     }
 }

StabilityMatrix.Avalonia/ViewModels/LaunchPageViewModel.cs

@@ -433,8 +433,9 @@ private void RunningPackageOnStartupComplete(object? sender, string e)
 
     public void Dispose()
     {
-        Console.Dispose();
         RunningPackage?.Shutdown();
+        Console.Dispose();
+
         GC.SuppressFinalize(this);
     }
 }

StabilityMatrix.Core/Extensions/StringExtensions.cs

@@ -27,25 +27,30 @@ private static string EncodeNonAsciiCharacters(string value) {
     /// <summary>
     /// Converts string to repr
     /// </summary>
-    public static string ToRepr(this string str)
+    public static string ToRepr(this string? str)
     {
+        if (str is null)
+        {
+            return "<null>";
+        }
         using var writer = new StringWriter();
-        using var provider = CodeDomProvider.CreateProvider("CSharp");
-
-        provider.GenerateCodeFromExpression(
-            new CodePrimitiveExpression(str), 
-            writer, 
-            new CodeGeneratorOptions {IndentString = "\t"});
+        writer.Write("'");
+        foreach (var ch in str)
+        {
+            writer.Write(ch switch
+            {
+                '\0' => "\\0",
+                '\n' => "\\n",
+                '\r' => "\\r",
+                '\t' => "\\t",
+                // Non ascii
+                _ when ch > 127 || ch < 32 => $"\\u{(int) ch:x4}",
+                _ => ch.ToString()
+            });
+        }
+        writer.Write("'");
 
-        var literal = writer.ToString();
-        // Replace split lines
-        literal = literal.Replace($"\" +{Environment.NewLine}\t\"", "");
-        // Encode non-ascii characters
-        literal = EncodeNonAsciiCharacters(literal);
-        // Surround with single quotes
-        literal = literal.Trim('"');
-        literal = $"'{literal}'";
-        return literal;
+        return writer.ToString();
     }
 
     /// <summary>

StabilityMatrix.Core/Processes/AsyncStreamReader.cs

@@ -254,6 +254,9 @@ private void MoveLinesFromStringBuilderToMessageQueue()
                     else
                     {
                         // Send buffer up to this point, not including \r
+                        // But skip if there's no content
+                        if (currentIndex == lineStart) break;
+
                         var line = _sb.ToString(lineStart, currentIndex - lineStart);
                         lock (_messageQueue)
                         {

StabilityMatrix.Tests/Core/AsyncStreamReaderTests.cs

@@ -13,62 +13,68 @@ public class AsyncStreamReaderTests
 
     [DataTestMethod]
     // Test newlines handling for \r\n, \n
-    [DataRow("a\r\nb\nc", "a\r\n", "b\n", "c")]
+    [DataRow("a\r\nb\nc", "a\r\n", "b\n", "c", null)]
     // Carriage returns \r should be sent as is
-    [DataRow("a\rb\rc", "a", "\rb", "\rc")]
-    [DataRow("a1\ra2\nb1\rb2", "a1", "\ra2\n", "b1", "\rb2")]
+    [DataRow("a\rb\rc", "a", "\rb", "\rc", null)]
+    [DataRow("a1\ra2\nb1\rb2", "a1", "\ra2\n", "b1", "\rb2", null)]
     // Ansi escapes should be seperated
-    [DataRow("\x1b[A\x1b[A", "\x1b[A", "\x1b[A")]
+    [DataRow("\x1b[A\x1b[A", "\x1b[A", "\x1b[A", null)]
     // Mixed Ansi and newlines
-    [DataRow("a \x1b[A\r\n\r xyz", "a ", "\x1b[A", "\r\n", "\r xyz")]
-    public async Task TestRead(string source, params string[] expected)
+    [DataRow("a \x1b[A\r\n\r xyz", "a ", "\x1b[A", "\r\n", "\r xyz", null)]
+    public async Task TestRead(string source, params string?[] expected)
     {
-        var queue = new Queue<string?>(expected);
+        var results = new List<string?>();
 
         var callback = new Action<string?>(s =>
         {
-            Assert.IsTrue(queue.Count > 0);
-            Assert.AreEqual(queue.Dequeue(), s);
+            results.Add(s);
         });
 
-        var stream = new MemoryStream(Encoding.UTF8.GetBytes(source));
-
-        // Make the reader
-        using var reader = new AsyncStreamReader(stream, callback, Encoding.UTF8);
-
-        // Begin read line and wait until finish
-        reader.BeginReadLine();
-        await reader.EOF;
-
-        // Check if all expected strings were read
-        Assert.AreEqual(0, queue.Count, "Remaining: " + string.Join(", ", queue.ToArray()
-            .Select(s => (s ?? "<null>").ToRepr())));
+        using var stream = new MemoryStream(Encoding.UTF8.GetBytes(source));
+        using (var reader = new AsyncStreamReader(stream, callback, Encoding.UTF8))
+        {
+            // Begin read line and wait until finish
+            reader.BeginReadLine();
+            // Wait for maximum 1 second
+            await reader.EOF.WaitAsync(new CancellationTokenSource(1000).Token);
+        }
+
+        // Check expected output matches
+        Assert.IsTrue(expected.SequenceEqual(results.ToArray()), 
+            "Results [{0}] do not match expected [{1}]", 
+            string.Join(", ", results.Select(s => s?.ToRepr() ?? "<null>")), 
+            string.Join(", ", expected.Select(s => s?.ToRepr() ?? "<null>")));
     }
 
     [TestMethod]
     public async Task TestCarriageReturnHandling()
     {
-        // The previous buffer should be sent when \r is encountered
-        const string source = "dog\r\ncat\r123\r456";
-        var stream = new MemoryStream(Encoding.UTF8.GetBytes(source));
+        var expected = new[] {"dog\r\n", "cat", "\r123", "\r456", null};
 
-        var queue = new Queue<string?>(new[] {"dog\r\n", "cat", "\r123", "\r456"});
+        var results = new List<string?>();
 
         var callback = new Action<string?>(s =>
         {
-            Assert.IsTrue(queue.Count > 0);
-            Assert.AreEqual(queue.Dequeue(), s);
+            results.Add(s);
         });
 
-        // Make the reader
-        using var reader = new AsyncStreamReader(stream, callback, Encoding.UTF8);
+        // The previous buffer should be sent when \r is encountered
+        const string source = "dog\r\ncat\r123\r456";
 
-        // Begin read line and wait until finish
-        reader.BeginReadLine();
-        await reader.EOF;
+        // Make the reader
+        using var stream = new MemoryStream(Encoding.UTF8.GetBytes(source));
+        using (var reader = new AsyncStreamReader(stream, callback, Encoding.UTF8))
+        {
+            // Begin read line and wait until finish
+            reader.BeginReadLine();
+            // Wait for maximum 1 second
+            await reader.EOF.WaitAsync(new CancellationTokenSource(1000).Token);
+        }
 
         // Check if all expected strings were read
-        Assert.AreEqual(0, queue.Count, "Remaining: " + string.Join(", ", queue.ToArray()
-            .Select(s => (s ?? "<null>").ToRepr())));
+        Assert.IsTrue(expected.SequenceEqual(results.ToArray()), 
+            "Results [{0}] do not match expected [{1}]", 
+            string.Join(", ", results.Select(s => s?.ToRepr() ?? "<null>")), 
+            string.Join(", ", expected.Select(s => s?.ToRepr() ?? "<null>")));
     }
 }