Add dispose rule and handle it in solution (production part)

.editorconfig

@@ -1,4 +1,4 @@
-# editorconfig.org
+# editorconfig.org
 
 # top-most EditorConfig file
 root = true
@@ -163,6 +163,9 @@ dotnet_code_quality.ca2208.api_surface = public
 # CA1852: Seal internal types
 dotnet_diagnostic.ca1852.severity = warning
 
+# CA2000: Dispose objects before losing scope
+dotnet_diagnostic.ca2000.severity = error
+
 # RS0037: Enable tracking of nullability of reference types in the declared API
 # Our API is not annotated but new classes get nullable enabled so disable this.
 # We'd be happy if everything was annotated and this could be removed.

eng/Common.globalconfig

@@ -321,9 +321,6 @@ dotnet_diagnostic.CA1837.severity = suggestion
 # Avoid 'StringBuilder' parameters for P/Invokes
 dotnet_diagnostic.CA1838.severity = warning
 
-# Dispose objects before losing scope
-dotnet_diagnostic.CA2000.severity = none
-
 # Do not lock on objects with weak identity
 dotnet_diagnostic.CA2002.severity = none
 

src/Build.UnitTests/BackEnd/RedirectConsoleWriter_Tests.cs

@@ -18,7 +18,7 @@ public async Task EmitConsoleMessages()
         {
             StringBuilder sb = new StringBuilder();
 
-            using (TextWriter writer = OutOfProcServerNode.RedirectConsoleWriter.Create(text => sb.Append(text)))
+            using (var writer = OutOfProcServerNode.RedirectConsoleWriter.Create(text => sb.Append(text)))
             {
                 writer.WriteLine("Line 1");
                 await Task.Delay(80); // should be somehow bigger than `RedirectConsoleWriter` flush period - see its constructor

src/Build/BackEnd/BuildManager/BuildManager.cs

@@ -2004,7 +2004,7 @@ private Dictionary<ProjectGraphNode, BuildResult> BuildGraph(
             IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> targetsPerNode,
             GraphBuildRequestData graphBuildRequestData)
         {
-            var waitHandle = new AutoResetEvent(true);
+            using var waitHandle = new AutoResetEvent(true);
             var graphBuildStateLock = new object();
 
             var blockedNodes = new HashSet<ProjectGraphNode>(projectGraph.ProjectNodes);

src/Build/BackEnd/BuildManager/LegacyThreadingData.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Build.BackEnd;
@@ -82,6 +83,7 @@ internal int MainThreadSubmissionId
         /// Given a submission ID, assign it "start" and "finish" events to track its use of
         /// the legacy thread.
         /// </summary>
+        [SuppressMessage("Microsoft.Dispose", "CA2000:Dispose objects before losing scope", Justification = "The events are disposed in UnregisterSubmissionForLegacyThread")]
         internal void RegisterSubmissionForLegacyThread(int submissionId)
         {
             lock (_legacyThreadingEventsLock)
@@ -104,6 +106,10 @@ internal void UnregisterSubmissionForLegacyThread(int submissionId)
             {
                 ErrorUtilities.VerifyThrow(_legacyThreadingEventsById.ContainsKey(submissionId), "Submission {0} should have been previously registered with LegacyThreadingData", submissionId);
 
+                // Dispose the events
+                _legacyThreadingEventsById[submissionId].Item1?.Dispose();
+                _legacyThreadingEventsById[submissionId].Item2?.Dispose();
+
                 _legacyThreadingEventsById.Remove(submissionId);
             }
         }

src/Build/BackEnd/Node/OutOfProcServerNode.cs

@@ -4,6 +4,9 @@
 using System;
 using System.Collections.Concurrent;
 using System.IO;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Build.BackEnd;
@@ -430,28 +433,115 @@ private void HandleServerNodeBuildCommand(ServerNodeBuildCommand command)
             _shutdownReason = _cancelRequested ? NodeEngineShutdownReason.BuildComplete : NodeEngineShutdownReason.BuildCompleteReuse;
             _shutdownEvent.Set();
         }
-        internal sealed class RedirectConsoleWriter : StringWriter
+
+        internal sealed class RedirectConsoleWriter : TextWriter
         {
             private readonly Action<string> _writeCallback;
             private readonly Timer _timer;
             private readonly TextWriter _syncWriter;
 
+            private readonly StringWriter _internalWriter;
+
             private RedirectConsoleWriter(Action<string> writeCallback)
             {
                 _writeCallback = writeCallback;
-                _syncWriter = Synchronized(this);
+                _internalWriter = new StringWriter();
+                _syncWriter = Synchronized(_internalWriter);
                 _timer = new Timer(TimerCallback, null, 0, 40);
             }
 
+            public override Encoding Encoding => _internalWriter.Encoding;
+
             public static TextWriter Create(Action<string> writeCallback)
             {
-                RedirectConsoleWriter writer = new(writeCallback);
-                return writer._syncWriter;
+                RedirectConsoleWriter writer = new RedirectConsoleWriter(writeCallback);
+
+                return writer;
             }
 
+            public override void Flush()
+            {
+                var sb = _internalWriter.GetStringBuilder();
+                string captured = sb.ToString();
+                sb.Clear();
+
+                _writeCallback(captured);
+                _internalWriter.Flush();
+            }
+
+            public override void Write(char value) => _syncWriter.Write(value);
+
+            public override void Write(char[]? buffer) => _syncWriter.Write(buffer);
+
+            public override void Write(char[] buffer, int index, int count) => _syncWriter.Write(buffer, index, count);
+
+            public override void Write(bool value) => _syncWriter.Write(value);
+
+            public override void Write(int value) => _syncWriter.Write(value);
+
+            public override void Write(uint value) => _syncWriter.Write(value);
+
+            public override void Write(long value) => _syncWriter.Write(value);
+
+            public override void Write(ulong value) => _syncWriter.Write(value);
+
+            public override void Write(float value) => _syncWriter.Write(value);
+
+            public override void Write(double value) => _syncWriter.Write(value);
+
+            public override void Write(decimal value) => _syncWriter.Write(value);
+
+            public override void Write(string? value) => _syncWriter.Write(value);
+
+            public override void Write(object? value) => _syncWriter.Write(value);
+
+            public override void Write(string format, object? arg0) => _syncWriter.Write(format, arg0);
+
+            public override void Write(string format, object? arg0, object? arg1) => _syncWriter.Write(format, arg0, arg1);
+
+            public override void Write(string format, object? arg0, object? arg1, object? arg2) => _syncWriter.Write(format, arg0, arg1, arg2);
+
+            public override void Write(string format, params object?[] arg) => _syncWriter.WriteLine(format, arg);
+
+            public override void WriteLine() => _syncWriter.WriteLine();
+
+            public override void WriteLine(char value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(decimal value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(char[]? buffer) => _syncWriter.WriteLine(buffer);
+
+            public override void WriteLine(char[] buffer, int index, int count) => _syncWriter.WriteLine(buffer, index, count);
+
+            public override void WriteLine(bool value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(int value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(uint value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(long value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(ulong value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(float value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(double value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(string? value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(object? value) => _syncWriter.WriteLine(value);
+
+            public override void WriteLine(string format, object? arg0) => _syncWriter.WriteLine(format, arg0);
+
+            public override void WriteLine(string format, object? arg0, object? arg1) => _syncWriter.WriteLine(format, arg0, arg1);
+
+            public override void WriteLine(string format, object? arg0, object? arg1, object? arg2) => _syncWriter.WriteLine(format, arg0, arg1, arg2);
+
+            public override void WriteLine(string format, params object?[] arg) => _syncWriter.WriteLine(format, arg);
+
             private void TimerCallback(object? state)
             {
-                if (GetStringBuilder().Length > 0)
+                if (_internalWriter.GetStringBuilder().Length > 0)
                 {
                     _syncWriter.Flush();
                 }
@@ -463,20 +553,11 @@ protected override void Dispose(bool disposing)
                 {
                     _timer.Dispose();
                     Flush();
+                    _internalWriter?.Dispose();
                 }
 
                 base.Dispose(disposing);
             }
-
-            public override void Flush()
-            {
-                var sb = GetStringBuilder();
-                var captured = sb.ToString();
-                sb.Clear();
-                _writeCallback(captured);
-
-                base.Flush();
-            }
         }
     }
 }

src/Build/BackEnd/Shared/BuildRequestConfiguration.cs

@@ -698,9 +698,21 @@ public void CacheIfPossible()
                 {
                     if (IsCacheable)
                     {
-                        using ITranslator translator = GetConfigurationTranslator(TranslationDirection.WriteToStream);
+                        string cacheFile = GetCacheFile();
+                        try
+                        {
+                            Directory.CreateDirectory(Path.GetDirectoryName(cacheFile));
+                            using Stream stream = File.Create(cacheFile);
+                            using ITranslator translator = GetConfigurationTranslator(TranslationDirection.WriteToStream, stream);
+
+                            _project.Cache(translator);
+                        }
+                        catch (Exception e) when (e is DirectoryNotFoundException or UnauthorizedAccessException)
+                        {
+                            ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e);
+                            throw;
+                        }
 
-                        _project.Cache(translator);
                         _baseLookup = null;
 
                         IsCached = true;
@@ -726,9 +738,19 @@ public void RetrieveFromCache()
                     return;
                 }
 
-                using ITranslator translator = GetConfigurationTranslator(TranslationDirection.ReadFromStream);
+                string cacheFile = GetCacheFile();
+                try
+                {
+                    using Stream stream = File.OpenRead(cacheFile);
+                    using ITranslator translator = GetConfigurationTranslator(TranslationDirection.ReadFromStream, stream);
 
-                _project.RetrieveFromCache(translator);
+                    _project.RetrieveFromCache(translator);
+                }
+                catch (Exception e) when (e is DirectoryNotFoundException or UnauthorizedAccessException)
+                {
+                    ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e);
+                    throw;
+                }
 
                 IsCached = false;
             }
@@ -939,6 +961,7 @@ internal void ApplyTransferredState(ProjectInstance instance)
         internal string GetCacheFile()
         {
             string filename = Path.Combine(FileUtilities.GetCacheDirectory(), String.Format(CultureInfo.InvariantCulture, "Configuration{0}.cache", _configId));
+
             return filename;
         }
 
@@ -1024,27 +1047,10 @@ private static string ResolveToolsVersion(BuildRequestData data, string defaultT
         /// <summary>
         /// Gets the translator for this configuration.
         /// </summary>
-        private ITranslator GetConfigurationTranslator(TranslationDirection direction)
-        {
-            string cacheFile = GetCacheFile();
-            try
-            {
-                if (direction == TranslationDirection.WriteToStream)
-                {
-                    Directory.CreateDirectory(Path.GetDirectoryName(cacheFile));
-                    return BinaryTranslator.GetWriteTranslator(File.Create(cacheFile));
-                }
-                else
-                {
+        private ITranslator GetConfigurationTranslator(TranslationDirection direction, Stream stream) =>
+            direction == TranslationDirection.WriteToStream
+                    ? BinaryTranslator.GetWriteTranslator(stream)
                     // Not using sharedReadBuffer because this is not a memory stream and so the buffer won't be used anyway.
-                    return BinaryTranslator.GetReadTranslator(File.OpenRead(cacheFile), InterningBinaryReader.PoolingBuffer);
-                }
-            }
-            catch (Exception e) when (e is DirectoryNotFoundException || e is UnauthorizedAccessException)
-            {
-                ErrorUtilities.ThrowInvalidOperation("CacheFileInaccessible", cacheFile, e);
-                throw;
-            }
-        }
+                    : BinaryTranslator.GetReadTranslator(stream, InterningBinaryReader.PoolingBuffer);
     }
 }

src/Build/BackEnd/Shared/TargetResult.cs

@@ -245,15 +245,23 @@ internal void CacheItems(int configId, string targetName)
                     return;
                 }
 
-                using ITranslator translator = GetResultsCacheTranslator(configId, targetName, TranslationDirection.WriteToStream);
+                string cacheFile = GetCacheFile(configId, targetName);
+                Directory.CreateDirectory(Path.GetDirectoryName(cacheFile));
 
-                // If the translator is null, it means these results were cached once before.  Since target results are immutable once they
-                // have been created, there is no point in writing them again.
-                if (translator != null)
+                // If the file doesn't already exists, then we haven't cached this once before. We need to cache it again since it could have changed.
+                if (!FileSystems.Default.FileExists(cacheFile))
                 {
-                    TranslateItems(translator);
-                    _items = null;
-                    _cacheInfo = new CacheInfo(configId, targetName);
+                    using Stream stream = File.Create(cacheFile);
+                    using ITranslator translator = GetResultsCacheTranslator(TranslationDirection.WriteToStream, stream);
+
+                    // If the translator is null, it means these results were cached once before.  Since target results are immutable once they
+                    // have been created, there is no point in writing them again.
+                    if (translator != null)
+                    {
+                        TranslateItems(translator);
+                        _items = null;
+                        _cacheInfo = new CacheInfo(configId, targetName);
+                    }
                 }
             }
         }
@@ -279,7 +287,9 @@ private void RetrieveItemsFromCache()
             {
                 if (_items == null)
                 {
-                    using ITranslator translator = GetResultsCacheTranslator(_cacheInfo.ConfigId, _cacheInfo.TargetName, TranslationDirection.ReadFromStream);
+                    string cacheFile = GetCacheFile(_cacheInfo.ConfigId, _cacheInfo.TargetName);
+                    using Stream stream = File.OpenRead(cacheFile);
+                    using ITranslator translator = GetResultsCacheTranslator(TranslationDirection.ReadFromStream, stream);
 
                     TranslateItems(translator);
                     _cacheInfo = new CacheInfo();
@@ -339,25 +349,10 @@ private void TranslateItems(ITranslator translator)
         /// <summary>
         /// Gets the translator for this configuration.
         /// </summary>
-        private static ITranslator GetResultsCacheTranslator(int configId, string targetToCache, TranslationDirection direction)
-        {
-            string cacheFile = GetCacheFile(configId, targetToCache);
-            if (direction == TranslationDirection.WriteToStream)
-            {
-                Directory.CreateDirectory(Path.GetDirectoryName(cacheFile));
-                if (FileSystems.Default.FileExists(cacheFile))
-                {
-                    // If the file already exists, then we have cached this once before.  No need to cache it again since it cannot have changed.
-                    return null;
-                }
-
-                return BinaryTranslator.GetWriteTranslator(File.Create(cacheFile));
-            }
-            else
-            {
-                return BinaryTranslator.GetReadTranslator(File.OpenRead(cacheFile), InterningBinaryReader.PoolingBuffer);
-            }
-        }
+        private static ITranslator GetResultsCacheTranslator(TranslationDirection direction, Stream stream) =>
+            direction == TranslationDirection.WriteToStream
+                    ? BinaryTranslator.GetWriteTranslator(stream)
+                    : BinaryTranslator.GetReadTranslator(stream, InterningBinaryReader.PoolingBuffer);
 
         /// <summary>
         /// Information about where the cache for the items in this result are stored.