Addressing PR issues

Author: WardenGnaw

IL/Microsoft.Internal.VisualStudio.Shell.Embeddable.il

@@ -5,6 +5,8 @@
 }
 .assembly Microsoft.Internal.VisualStudio.Shell.Embeddable
 {
+	.custom instance void [mscorlib]System.Runtime.InteropServices.ImportedFromTypeLibAttribute::.ctor(string) = { string('Microsoft.Internal.VisualStudio.Shell.Embeddable') }
+	.custom instance void [mscorlib]System.Runtime.InteropServices.GuidAttribute::.ctor(string) = { string('190F8999-290E-4111-AE15-1509B895B58C') }
 	.publickey = (00 24 00 00 04 80 00 00 94 00 00 00 06 02 00 00 00 24 00 00 52 53 41 31 00 04 00 00 01 00 01 00 07 D1 FA 57 C4 AE D9 F0 A3 2E 84 AA 0F AE FD 0D E9 E8 FD 6A EC 8F 87 FB 03 76 6C 83 4C 99 92 1E B2 3B E7 9A D9 D5 DC C1 DD 9A D2 36 13 21 02 90 0B 72 3C F9 80 95 7F C4 E1 77 10 8F C6 07 77 4F 29 E8 32 0E 92 EA 05 EC E4 E8 21 C0 A5 EF E8 F1 64 5C 4C 0C 93 C1 AB 99 28 5D 62 2C AA 65 2C 1D FA D6 3D 74 5D 6F 2D E5 F1 7E 5E AF 0F C4 96 3D 26 1C 8A 12 43 65 18 20 6D C0 93 34 4D 5A D2 93)
 	.hash algorithm 0x00008004
 	.ver 16:0:0:0

src/DebugEngineHost.Stub/DebugEngineHost.ref.cs

@@ -309,6 +309,18 @@ public static void Reset()
             throw new NotImplementedException();
         }
 
+        /// <summary>
+        /// Returns true if the feedback log is currently active
+        /// </summary>
+        public static bool IsFeedbackLogEnabled
+        {
+            get { throw new NotImplementedException(); }
+        }
+
+        /// <summary>
+        /// Writes a message to the feedback circular buffer and, if active, to the feedback log file.
+        /// </summary>
+        /// <param name="message">The message to write to the feedback log.</param>
         public static void WriteFeedbackLog(string message)
         {
             throw new NotImplementedException();

src/DebugEngineHost.VSCode/HostLogger.cs

@@ -52,6 +52,11 @@ public static void Reset()
             s_engineLogChannel = null;
         }
 
+        public static bool IsFeedbackLogEnabled
+        {
+            get { return false; }
+        }
+
         public static void WriteFeedbackLog(string message)
         {
         }

src/DebugEngineHost/DebugEngineHost.csproj

@@ -109,6 +109,7 @@
     </Reference>
     <Reference Include="Microsoft.Internal.VisualStudio.Shell.Embeddable">
       <HintPath>$(GeneratedAssembliesDir)Microsoft.Internal.VisualStudio.Shell.Embeddable.dll</HintPath>
+      <EmbedInteropTypes>true</EmbedInteropTypes>
       <Private>false</Private>
     </Reference>
   </ItemGroup>

src/DebugEngineHost/FeedbackDiagnosticFileProvider.cs

@@ -1,11 +1,12 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Collections.Generic;
 using System.ComponentModel.Composition;
 using System.Diagnostics;
 using System.IO;
-using System.Text;
+using System.Threading.Tasks;
 using Microsoft.Internal.VisualStudio.Shell.Embeddable.Feedback;
 
 namespace Microsoft.DebugEngineHost
@@ -15,35 +16,34 @@ public class FeedbackDiagnosticFileProvider : IFeedbackDiagnosticFileProvider
     {
         public IReadOnlyCollection<string> GetFiles()
         {
+            if (!HostLogger.HasFeedbackEntries)
+            {
+                return Array.Empty<string>();
+            }
+
             string logFileName = HostLogger.GetFeedbackLogFilePath(Process.GetCurrentProcess().Id);
 
+            IReadOnlyCollection<string> entries = HostLogger.GetNewFeedbackEntries();
+            if (entries.Count > 0)
+            {
+                _ = Task.Run(() => WriteFeedbackEntries(logFileName, entries));
+            }
+
+            return new[] { logFileName };
+        }
+
+        private static void WriteFeedbackEntries(string logFileName, IReadOnlyCollection<string> entries)
+        {
             try
             {
-                IReadOnlyCollection<string> entries = HostLogger.GetNewFeedbackEntries();
-                if (entries.Count > 0)
+                using (StreamWriter logWriter = FeedbackLogBuffer.OpenLogFile(logFileName))
                 {
-                    using (var fs = new FileStream(logFileName, FileMode.Append, FileAccess.Write, FileShare.ReadWrite))
-                    using (var logWriter = new StreamWriter(fs, Encoding.UTF8))
-                    {
-                        foreach (string logLine in entries)
-                        {
-                            logWriter.WriteLine(logLine);
-                        }
-
-                        logWriter.Flush();
-                    }
+                    FeedbackLogBuffer.WriteEntries(logWriter, entries);
                 }
             }
             catch
             {
             }
-
-            if (File.Exists(logFileName))
-            {
-                return new[] { logFileName };
-            }
-
-            return new string[0];
         }
     }
 }

src/DebugEngineHost/FeedbackLogBuffer.cs

@@ -1,8 +1,11 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
+using System.Text;
 
 namespace Microsoft.DebugEngineHost
 {
@@ -19,6 +22,20 @@ internal class FeedbackLogBuffer
         private long _lastFlushSequence;
         private readonly object _syncObj = new object();
 
+        /// <summary>
+        /// Returns true if no entries have been written since the last flush.
+        /// </summary>
+        internal bool IsEmpty
+        {
+            get
+            {
+                lock (_syncObj)
+                {
+                    return _writeSequence == _lastFlushSequence;
+                }
+            }
+        }
+
         internal void Write(string logLine)
         {
             lock (_syncObj)
@@ -50,12 +67,42 @@ internal IReadOnlyCollection<string> FlushNewEntries()
 
                 if (newEntryCount <= 0)
                 {
-                    return new string[0];
+                    return Array.Empty<string>();
                 }
 
                 int skipCount = _logBuffer.Count - (int)newEntryCount;
                 return _logBuffer.Skip(skipCount).ToList().AsReadOnly();
             }
         }
+
+        /// <summary>
+        /// Opens the feedback log file for appending with shared read/write access.
+        /// </summary>
+        internal static StreamWriter OpenLogFile(string logFileName)
+        {
+            var fs = new FileStream(logFileName, FileMode.Append, FileAccess.Write, FileShare.ReadWrite);
+            try
+            {
+                return new StreamWriter(fs, Encoding.UTF8);
+            }
+            catch
+            {
+                fs.Dispose();
+                throw;
+            }
+        }
+
+        /// <summary>
+        /// Writes a collection of log entries to the given writer.
+        /// </summary>
+        internal static void WriteEntries(StreamWriter writer, IEnumerable<string> entries)
+        {
+            foreach (string logLine in entries)
+            {
+                writer.WriteLine(logLine);
+            }
+
+            writer.Flush();
+        }
     }
 }

src/DebugEngineHost/HostLogger.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
+using System.Threading;
 
 namespace Microsoft.DebugEngineHost
 {
@@ -15,7 +16,7 @@ public static class HostLogger
 
         private static string s_engineLogFile;
 
-        private static readonly FeedbackLogBuffer s_circularBuffer = new FeedbackLogBuffer();
+        private static FeedbackLogBuffer s_circularBuffer;
         private static VSFeedbackLogger s_feedbackLogger;
 
         public static void EnableHostLogging(Action<string> callback, LogLevel level = LogLevel.Verbose)
@@ -27,7 +28,7 @@ public static void EnableHostLogging(Action<string> callback, LogLevel level = L
 
             if (s_feedbackLogger == null)
             {
-                s_feedbackLogger = new VSFeedbackLogger(s_circularBuffer);
+                s_feedbackLogger = new VSFeedbackLogger(EnsureFeedbackBuffer());
             }
         }
 
@@ -59,6 +60,14 @@ public static ILogChannel GetNatvisLogChannel()
             return s_natvisLogChannel;
         }
 
+        /// <summary>
+        /// Returns true if the feedback log is currently active (buffer has been created).
+        /// </summary>
+        public static bool IsFeedbackLogEnabled
+        {
+            get { return s_circularBuffer != null; }
+        }
+
         /// <summary>
         /// Writes a message to the feedback circular buffer and, if active, to the feedback log file.
         /// </summary>
@@ -72,16 +81,39 @@ public static void WriteFeedbackLog(string message)
             string timestamp = DateTime.Now.ToString("MM/dd/yyyy HH:mm:ss.fff", CultureInfo.InvariantCulture);
             string logLine = string.Format(CultureInfo.InvariantCulture, "{0}: {1}", timestamp, message);
 
-            s_circularBuffer.Write(logLine);
+            EnsureFeedbackBuffer().Write(logLine);
             s_feedbackLogger?.Write(logLine);
         }
 
+        private static FeedbackLogBuffer EnsureFeedbackBuffer()
+        {
+            if (s_circularBuffer == null)
+            {
+                Interlocked.CompareExchange(ref s_circularBuffer, new FeedbackLogBuffer(), null);
+            }
+
+            return s_circularBuffer;
+        }
+
+        /// <summary>
+        /// Returns true if the feedback buffer has been created and has entries.
+        /// This is a cheap check to avoid unnecessary work in non-MIEngine scenarios.
+        /// </summary>
+        internal static bool HasFeedbackEntries
+        {
+            get
+            {
+                FeedbackLogBuffer buffer = s_circularBuffer;
+                return buffer != null && !buffer.IsEmpty;
+            }
+        }
+
         /// <summary>
         /// Returns log entries added since the last flush, then advances the flush marker.
         /// </summary>
         internal static IReadOnlyCollection<string> GetNewFeedbackEntries()
         {
-            return s_circularBuffer.FlushNewEntries();
+            return s_circularBuffer?.FlushNewEntries() ?? Array.Empty<string>();
         }
 
         /// <summary>

src/DebugEngineHost/VSFeedbackLogger.cs

@@ -72,14 +72,10 @@ private void OnFeedbackSemaphoreCreated(object sender, FileSystemEventArgs e)
                         if (!_enabled)
                         {
                             string logFileName = HostLogger.GetFeedbackLogFilePath(_vsPid);
-                            StreamWriter writer = new StreamWriter(logFileName, append: true, encoding: Encoding.UTF8);
+                            StreamWriter writer = FeedbackLogBuffer.OpenLogFile(logFileName);
                             try
                             {
-                                foreach (string logLine in _circularBuffer.FlushNewEntries())
-                                {
-                                    writer.WriteLine(logLine);
-                                }
-                                writer.Flush();
+                                FeedbackLogBuffer.WriteEntries(writer, _circularBuffer.FlushNewEntries());
                             }
                             catch
                             {

src/MICore/Logger.cs

@@ -108,12 +108,15 @@ public static void Reset()
         /// <param name="line">[Required] line to write</param>
         public void WriteLine(LogLevel level, string line)
         {
-            HostLogger.WriteFeedbackLog(line);
-
             if (s_isEnabled)
             {
                 WriteLineImpl(level, line);
             }
+
+            if (HostLogger.IsFeedbackLogEnabled)
+            {
+                HostLogger.WriteFeedbackLog(line);
+            }
         }
 
         /// <summary>
@@ -124,12 +127,10 @@ public void WriteLine(LogLevel level, string line)
         /// <param name="args">arguments to use in the format string</param>
         public void WriteLine(LogLevel level, string format, params object[] args)
         {
-            string message = string.Format(CultureInfo.CurrentCulture, format, args);
-            HostLogger.WriteFeedbackLog(message);
-
-            if (s_isEnabled)
+            if (s_isEnabled || HostLogger.IsFeedbackLogEnabled)
             {
-                WriteLineImpl(level, message);
+                string message = string.Format(CultureInfo.CurrentCulture, format, args);
+                WriteLine(level, message);
             }
         }
 
@@ -141,12 +142,15 @@ public void WriteLine(LogLevel level, string format, params object[] args)
         /// <param name="textBlock">Block of text to write</param>
         public void WriteTextBlock(LogLevel level, string prefix, string textBlock)
         {
-            HostLogger.WriteFeedbackLog((!string.IsNullOrEmpty(prefix) ? prefix : string.Empty) + textBlock);
-
             if (s_isEnabled)
             {
                 WriteTextBlockImpl(level, prefix, textBlock);
             }
+
+            if (HostLogger.IsFeedbackLogEnabled)
+            {
+                HostLogger.WriteFeedbackLog((!string.IsNullOrEmpty(prefix) ? prefix : string.Empty) + textBlock);
+            }
         }
 
         /// <summary>