Merge pull request #2083 from KevinH-MS/master

Avoid a hang in CompileWithRetry...

src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/MissingAssemblyTests.cs

@@ -456,6 +456,47 @@ static void M(Windows.UI.Colors c)
             Assert.Equal(expectedMissingAssemblyIdentity, actualMissingAssemblyIdentities.Single());
         }
 
+        [WorkItem(1154988)] 
+        [Fact] 
+        public void CompileWithRetrySameErrorReported()
+        { 
+            var source = @" 
+class C 
+{ 
+    void M() 
+    { 
+    } 
+}"; 
+            var comp = CreateCompilationWithMscorlib(source); 
+            var runtime = CreateRuntimeInstance(comp); 
+            var context = CreateMethodContext(runtime, "C.M"); 
+ 
+            var missingModule = runtime.Modules.First(); 
+            var missingIdentity = missingModule.MetadataReader.ReadAssemblyIdentityOrThrow(); 
+ 
+            var numRetries = 0; 
+            string errorMessage;
+            ExpressionCompilerTestHelpers.CompileExpressionWithRetry( 
+                runtime.Modules.Select(m => m.MetadataBlock).ToImmutableArray(), 
+                context, 
+                (_, diagnostics) => 
+                {
+                    numRetries++;
+                    Assert.InRange(numRetries, 0, 2); // We don't want to loop forever... 
+                    diagnostics.Add(new CSDiagnostic(new CSDiagnosticInfo(ErrorCode.ERR_NoTypeDef, "MissingType", missingIdentity), Location.None));
+                    return default(CompileResult);   
+                }, 
+                (AssemblyIdentity assemblyIdentity, out uint uSize) => 
+                { 
+                    uSize = (uint)missingModule.MetadataLength; 
+                    return missingModule.MetadataAddress; 
+                }, 
+                out errorMessage); 
+ 
+            Assert.Equal(2, numRetries); // Ensure that we actually retried and that we bailed out on the second retry if the same identity was seen in the diagnostics.
+            Assert.Equal($"error CS0012: The type 'MissingType' is defined in an assembly that is not referenced. You must add a reference to assembly '{missingIdentity}'.", errorMessage); 
+        } 
+
         private EvaluationContext CreateMethodContextWithReferences(Compilation comp, string methodName, params MetadataReference[] references)
         {
             return CreateMethodContextWithReferences(comp, methodName, ImmutableArray.CreateRange(references));

src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.cs

@@ -6,6 +6,7 @@
 using System.Diagnostics;
 using System.IO;
 using System.Threading;
+using Microsoft.CodeAnalysis.Collections;
 using Microsoft.VisualStudio.Debugger;
 using Microsoft.VisualStudio.Debugger.Clr;
 using Microsoft.VisualStudio.Debugger.ComponentInterfaces;
@@ -290,6 +291,7 @@ private EvaluationContextBase CreateMethodContext(DkmClrInstructionAddress instr
             errorMessage = null;
             TResult compileResult;
 
+            PooledHashSet<AssemblyIdentity> assembliesLoadedInRetryLoop = null;
             bool tryAgain;
             do
             {
@@ -322,11 +324,23 @@ private EvaluationContextBase CreateMethodContext(DkmClrInstructionAddress instr
                     }
                     else
                     {
-                        tryAgain = ShouldTryAgainWithMoreMetadataBlocks(getMetaDataBytesPtr, missingAssemblyIdentities, ref metadataBlocks);
+                        if (!missingAssemblyIdentities.IsEmpty)
+                        {
+                            if (assembliesLoadedInRetryLoop == null)
+                            {
+                                assembliesLoadedInRetryLoop = PooledHashSet<AssemblyIdentity>.GetInstance();
+                            }
+                            // If any identities failed to add (they were already in the list), then don't retry. 
+                            if (assembliesLoadedInRetryLoop.AddAll(missingAssemblyIdentities))
+                            {
+                                tryAgain = ShouldTryAgainWithMoreMetadataBlocks(getMetaDataBytesPtr, missingAssemblyIdentities, ref metadataBlocks);
+                            }
+                        }
                     }
                 }
                 diagnostics.Free();
             } while (tryAgain);
+            assembliesLoadedInRetryLoop?.Free();
 
             return compileResult;
         }

src/ExpressionEvaluator/Core/Test/ExpressionCompiler/ExpressionCompilerTestHelpers.cs

@@ -98,6 +98,22 @@ internal static class ExpressionCompilerTestHelpers
             return result;
         }
 
+        internal static CompileResult CompileExpressionWithRetry(
+            ImmutableArray<MetadataBlock> metadataBlocks,
+            EvaluationContextBase context,
+            ExpressionCompiler.CompileDelegate<CompileResult> compile,
+            DkmUtilities.GetMetadataBytesPtrFunction getMetaDataBytesPtr,
+            out string errorMessage)
+        {
+            return ExpressionCompiler.CompileWithRetry(
+                metadataBlocks,
+                DiagnosticFormatter.Instance,
+                (blocks, useReferencedModulesOnly) => context,
+                compile,
+                getMetaDataBytesPtr,
+                out errorMessage);
+        }
+
         internal static CompileResult CompileExpressionWithRetry(
             ImmutableArray<MetadataBlock> metadataBlocks,
             string expr,

src/ExpressionEvaluator/VisualBasic/Test/ExpressionCompiler/MissingAssemblyTests.vb

@@ -8,6 +8,7 @@ Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
 Imports Microsoft.VisualStudio.Debugger.Evaluation
 Imports Roslyn.Test.PdbUtilities
 Imports Roslyn.Test.Utilities
+Imports Roslyn.Utilities
 Imports Xunit
 
 Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests
@@ -410,6 +411,43 @@ End Class
             Assert.Equal(expectedMissingAssemblyIdentity, actualMissingAssemblyIdentities.Single())
         End Sub
 
+        <WorkItem(1154988)>
+        <Fact>
+        Public Sub CompileWithRetrySameErrorReported()
+            Dim source = " 
+Class C 
+    Sub M() 
+    End Sub 
+End Class 
+"
+            Dim comp = CreateCompilationWithMscorlib({source}, options:=TestOptions.DebugDll)
+            Dim runtime = CreateRuntimeInstance(comp)
+            Dim context = CreateMethodContext(runtime, "C.M")
+
+            Dim missingModule = runtime.Modules.First()
+            Dim missingIdentity = missingModule.MetadataReader.ReadAssemblyIdentityOrThrow()
+
+            Dim numRetries = 0
+            Dim errorMessage As String = Nothing
+            ExpressionCompilerTestHelpers.CompileExpressionWithRetry(
+                runtime.Modules.Select(Function(m) m.MetadataBlock).ToImmutableArray(),
+                context,
+                Function(unused, diagnostics)
+                    numRetries += 1
+                    Assert.InRange(numRetries, 0, 2) ' We don't want to loop forever... 
+                    diagnostics.Add(New VBDiagnostic(ErrorFactory.ErrorInfo(ERRID.ERR_UnreferencedAssembly3, missingIdentity, "MissingType"), Location.None))
+                    Return DirectCast(Nothing, CompileResult)
+                End Function,
+                Function(assemblyIdentity, ByRef uSize)
+                    uSize = CUInt(missingModule.MetadataLength)
+                    Return missingModule.MetadataAddress
+                End Function,
+                errorMessage)
+
+            Assert.Equal(2, numRetries) ' Ensure that we actually retried and that we bailed out on the second retry if the same identity was seen in the diagnostics.
+            Assert.Equal($"error BC30652: Reference required to assembly '{missingIdentity}' containing the type 'MissingType'. Add one to your project.", errorMessage)
+        End Sub
+
         Private Function CreateMethodContextWithReferences(comp As Compilation, methodName As String, ParamArray references As MetadataReference()) As EvaluationContext
             Return CreateMethodContextWithReferences(comp, methodName, ImmutableArray.CreateRange(references))
         End Function