Fix concurrency issues over Console.Out

Author: jnm2

tools/ExampleTester/ExampleTests.cs

@@ -8,7 +8,6 @@ namespace ExampleTester;
 public static class ExampleTests
 {
     private static TesterConfiguration TesterConfiguration { get; } = new(FindTmpDirectory());
-    private static StatusCheckLogger StatusCheckLogger { get; } = new("..", "Example tester");
 
     public static IEnumerable<object[]> LoadExamples() =>
         from example in GeneratedExample.LoadAllExamples(TesterConfiguration.ExtractedOutputDirectory)
@@ -17,7 +16,9 @@ from example in GeneratedExample.LoadAllExamples(TesterConfiguration.ExtractedOu
     [TestCaseSource(nameof(LoadExamples))]
     public static async Task ExamplePasses(GeneratedExample example)
     {
-        if (!await example.Test(TesterConfiguration, StatusCheckLogger))
+        var logger = new StatusCheckLogger(TestContext.Out, "..", "Example tester");
+
+        if (!await example.Test(TesterConfiguration, logger))
             Assert.Fail("There were one or more failures. See the logged output for details.");
     }
 

tools/ExampleTester/GeneratedExample.cs

@@ -9,8 +9,10 @@
 
 namespace ExampleTester;
 
-internal class GeneratedExample
+public class GeneratedExample
 {
+    private static readonly object ConsoleAccessLock = new();
+
     static GeneratedExample()
     {
         MSBuildLocator.RegisterDefaults();
@@ -135,46 +137,52 @@ bool ValidateOutput()
                 ? new object[] { Metadata.ExecutionArgs ?? new string[0] }
                 : new object[0];
 
-            var oldOut = Console.Out;
             List<string> actualLines;
             Exception? actualException = null;
-            try
+            lock (ConsoleAccessLock)
             {
-                var builder = new StringBuilder();
-                Console.SetOut(new StringWriter(builder));
+                var oldOut = Console.Out;
                 try
                 {
-                    var result = method.Invoke(null, arguments);
-                    // For async Main methods, the compilation's entry point is still the Main
-                    // method, so we explicitly wait for the returned task just like the synthesized
-                    // entry point would.
-                    if (result is Task task)
+                    var builder = new StringBuilder();
+                    Console.SetOut(new StringWriter(builder));
+                    try
+                    {
+                        var result = method.Invoke(null, arguments);
+                        // For async Main methods, the compilation's entry point is still the Main
+                        // method, so we explicitly wait for the returned task just like the synthesized
+                        // entry point would.
+                        if (result is Task task)
+                        {
+                            task.GetAwaiter().GetResult();
+                        }
+
+                        // For some reason, we don't *actually* get the result of all finalizers
+                        // without this. We shouldn't need it (as relevant examples already have it) but
+                        // code that works outside the test harness doesn't work inside it.
+                        GC.Collect();
+                        GC.WaitForPendingFinalizers();
+                    }
+                    catch (TargetInvocationException outer)
                     {
-                        task.GetAwaiter().GetResult();
+                        actualException = outer.InnerException ?? throw new InvalidOperationException("TargetInvocationException had no nested exception");
                     }
-                    // For some reason, we don't *actually* get the result of all finalizers
-                    // without this. We shouldn't need it (as relevant examples already have it) but
-                    // code that works outside the test harness doesn't work inside it.
-                    GC.Collect();
-                    GC.WaitForPendingFinalizers();
+
+                    // Skip blank lines, to avoid unnecessary trailing empties.
+                    // Also trim the end of each actual line, to avoid trailing spaces being necessary in the metadata
+                    // or listed console output.
+                    actualLines = builder.ToString()
+                        .Replace("\r\n", "\n")
+                        .Split('\n')
+                        .Select(line => line.TrimEnd())
+                        .Where(line => line != "").ToList();
                 }
-                catch (TargetInvocationException outer)
+                finally
                 {
-                    actualException = outer.InnerException ?? throw new InvalidOperationException("TargetInvocationException had no nested exception");
+                    Console.SetOut(oldOut);
                 }
-                // Skip blank lines, to avoid unnecessary trailing empties.
-                // Also trim the end of each actual line, to avoid trailing spaces being necessary in the metadata
-                // or listed console output.
-                actualLines = builder.ToString()
-                    .Replace("\r\n", "\n")
-                    .Split('\n')
-                    .Select(line => line.TrimEnd())
-                    .Where(line => line != "").ToList();
-            }
-            finally
-            {
-                Console.SetOut(oldOut);
             }
+
             var expectedLines = Metadata.ExpectedOutput ?? new List<string>();
             return ValidateException(actualException, Metadata.ExpectedException) &&
                 (Metadata.IgnoreOutput || ValidateExpectedAgainstActual("output", expectedLines, actualLines));

tools/ExampleTester/Program.cs

@@ -2,7 +2,7 @@
 using System.CommandLine;
 using Utilities;
 
-var logger = new StatusCheckLogger("..", "Example tester");
+var logger = new StatusCheckLogger(Console.Out, "..", "Example tester");
 var headSha = Environment.GetEnvironmentVariable("HEAD_SHA");
 var token = Environment.GetEnvironmentVariable("GH_TOKEN");
 

tools/MarkdownConverter/Spec/Reporter.cs

@@ -28,7 +28,7 @@ public Reporter() : this(null, null) { }
     public Reporter(Reporter? parent, string? filename)
     {
         // This is needed so that all Reporters share the same GitHub logger.
-        this.githubLogger = parent?.githubLogger ?? new StatusCheckLogger("..", "Markdown to Word Converter");
+        this.githubLogger = parent?.githubLogger ?? new StatusCheckLogger(Console.Out, "..", "Markdown to Word Converter");
         this.parent = parent;
         Location = new SourceLocation(filename, null, null, null);
     }

tools/StandardAnchorTags/Program.cs

@@ -25,7 +25,7 @@ public class Program
     /// <returns>0 on success, non-zero on failure</returns>
     static async Task<int> Main(string owner, string repo, bool dryrun =false)
     {
-        var logger = new StatusCheckLogger("..", "TOC and Anchor updater");
+        var logger = new StatusCheckLogger(Console.Out, "..", "TOC and Anchor updater");
         var headSha = Environment.GetEnvironmentVariable("HEAD_SHA");
         var token = Environment.GetEnvironmentVariable("GH_TOKEN");
         using FileStream openStream = File.OpenRead(FilesPath);

tools/Utilities/StatusCheckLogger.cs

@@ -22,15 +22,15 @@ public record StatusCheckMessage(string file, int StartLine, int EndLine, string
 /// </remarks>
 /// <param name="pathToRoot">The path to the root of the repository</param>
 /// <param name="toolName">The name of the tool that is running the check</param>
-public class StatusCheckLogger(string pathToRoot, string toolName)
+public class StatusCheckLogger(TextWriter writer, string pathToRoot, string toolName)
 {
     private List<NewCheckRunAnnotation> annotations = [];
     public bool Success { get; private set; } = true;
 
     // Utility method to format the path to unix style, from the root of the repository.
     private string FormatPath(string path) => Path.GetRelativePath(pathToRoot, path).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
 
-    private void WriteMessageToConsole(string prefix, StatusCheckMessage d) => Console.WriteLine($"{prefix}{toolName}-{d.Id}::file={FormatPath(d.file)},line={d.StartLine}::{d.Message}");
+    private void WriteMessageToConsole(string prefix, StatusCheckMessage d) => writer.WriteLine($"{prefix}{toolName}-{d.Id}::file={FormatPath(d.file)},line={d.StartLine}::{d.Message}");
 
     /// <summary>
     /// Log a notice from the status check to the console only
@@ -178,9 +178,9 @@ public async Task BuildCheckRunResult(string token, string owner, string repo, s
         // Once running on a branch on the dotnet org, this should work correctly.
         catch (ForbiddenException e)
         {
-            Console.WriteLine("===== WARNING: Could not create a check run.=====");
-            Console.WriteLine("Exception details:");
-            Console.WriteLine(e);
+            writer.WriteLine("===== WARNING: Could not create a check run.=====");
+            writer.WriteLine("Exception details:");
+            writer.WriteLine(e);
         }
     }
 }