Merge pull request #17880 from cston/17788

Handle unloaded modules
dotnet · Mar 22, 2017 · 1379834 · 1379834
2 parents 1168c4c + 0eace55
commit 1379834
Show file tree

Hide file tree

Showing 9 changed files with 113 additions and 69 deletions.
diff --git a/...xpressionEvaluator/CSharp/Source/ExpressionCompiler/CSharpExpressionCompiler.vsdconfigxml b/...xpressionEvaluator/CSharp/Source/ExpressionCompiler/CSharpExpressionCompiler.vsdconfigxml
@@ -22,6 +22,7 @@
           CallOnlyWhenLoaded="true">
           <NoFilter/>
           <Interface Name="IDkmModuleModifiedNotification"/>
+          <Interface Name="IDkmModuleInstanceUnloadNotification"/>
         </InterfaceGroup>
       </Implements>
     </Class>

diff --git a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/MissingAssemblyTests.cs b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/MissingAssemblyTests.cs
@@ -440,46 +440,72 @@ public unsafe void ShouldTryAgain_Mixed()
         [Fact]
         public void ShouldTryAgain_CORDBG_E_MISSING_METADATA()
         {
-            DkmUtilities.GetMetadataBytesPtrFunction gmdbpf = (AssemblyIdentity assemblyIdentity, out uint uSize) =>
-            {
-                Marshal.ThrowExceptionForHR(unchecked((int)MetadataUtilities.CORDBG_E_MISSING_METADATA));
-                throw ExceptionUtilities.Unreachable;
-            };
-
-            var references = ImmutableArray<MetadataBlock>.Empty;
-            var missingAssemblyIdentities = ImmutableArray.Create(new AssemblyIdentity("A"));
-            Assert.False(ExpressionCompiler.ShouldTryAgainWithMoreMetadataBlocks(gmdbpf, missingAssemblyIdentities, ref references));
-            Assert.Empty(references);
+            ShouldTryAgain_False(
+                (AssemblyIdentity assemblyIdentity, out uint uSize) =>
+                {
+                    Marshal.ThrowExceptionForHR(unchecked((int)MetadataUtilities.CORDBG_E_MISSING_METADATA));
+                    throw ExceptionUtilities.Unreachable;
+                });
         }
 
         [Fact]
         public void ShouldTryAgain_COR_E_BADIMAGEFORMAT()
         {
-            DkmUtilities.GetMetadataBytesPtrFunction gmdbpf = (AssemblyIdentity assemblyIdentity, out uint uSize) =>
-            {
-                Marshal.ThrowExceptionForHR(unchecked((int)MetadataUtilities.COR_E_BADIMAGEFORMAT));
-                throw ExceptionUtilities.Unreachable;
-            };
+            ShouldTryAgain_False(
+                (AssemblyIdentity assemblyIdentity, out uint uSize) =>
+                {
+                    Marshal.ThrowExceptionForHR(unchecked((int)MetadataUtilities.COR_E_BADIMAGEFORMAT));
+                    throw ExceptionUtilities.Unreachable;
+                });
+        }
+
+        [Fact]
+        public void ShouldTryAgain_ObjectDisposedException()
+        {
+            ShouldTryAgain_False(
+                (AssemblyIdentity assemblyIdentity, out uint uSize) =>
+                {
+                    throw new ObjectDisposedException("obj");
+                });
+        }
+
+        [Fact]
+        public void ShouldTryAgain_RPC_E_DISCONNECTED()
+        {
+            DkmUtilities.GetMetadataBytesPtrFunction gmdbpf =
+                (AssemblyIdentity assemblyIdentity, out uint uSize) =>
+                {
+                    Marshal.ThrowExceptionForHR(unchecked((int)0x80010108));
+                    throw ExceptionUtilities.Unreachable;
+                };
 
             var references = ImmutableArray<MetadataBlock>.Empty;
             var missingAssemblyIdentities = ImmutableArray.Create(new AssemblyIdentity("A"));
-            Assert.False(ExpressionCompiler.ShouldTryAgainWithMoreMetadataBlocks(gmdbpf, missingAssemblyIdentities, ref references));
-            Assert.Empty(references);
+            Assert.Throws<COMException>(() => ExpressionCompiler.ShouldTryAgainWithMoreMetadataBlocks(gmdbpf, missingAssemblyIdentities, ref references));
         }
 
         [Fact]
-        public void ShouldTryAgain_OtherException()
+        public void ShouldTryAgain_Exception()
         {
-            DkmUtilities.GetMetadataBytesPtrFunction gmdbpf = (AssemblyIdentity assemblyIdentity, out uint uSize) =>
-            {
-                throw new Exception();
-            };
+            DkmUtilities.GetMetadataBytesPtrFunction gmdbpf =
+                (AssemblyIdentity assemblyIdentity, out uint uSize) =>
+                {
+                    throw new Exception();
+                };
 
             var references = ImmutableArray<MetadataBlock>.Empty;
             var missingAssemblyIdentities = ImmutableArray.Create(new AssemblyIdentity("A"));
             Assert.Throws<Exception>(() => ExpressionCompiler.ShouldTryAgainWithMoreMetadataBlocks(gmdbpf, missingAssemblyIdentities, ref references));
         }
 
+        private static void ShouldTryAgain_False(DkmUtilities.GetMetadataBytesPtrFunction gmdbpf)
+        {
+            var references = ImmutableArray<MetadataBlock>.Empty;
+            var missingAssemblyIdentities = ImmutableArray.Create(new AssemblyIdentity("A"));
+            Assert.False(ExpressionCompiler.ShouldTryAgainWithMoreMetadataBlocks(gmdbpf, missingAssemblyIdentities, ref references));
+            Assert.Empty(references);
+        }
+
         [WorkItem(1124725, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1124725")]
         [Fact]
         public void PseudoVariableType()

diff --git a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.cs b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.cs
@@ -19,7 +19,8 @@ namespace Microsoft.CodeAnalysis.ExpressionEvaluator
     public abstract class ExpressionCompiler :
         IDkmClrExpressionCompiler,
         IDkmClrExpressionCompilerCallback,
-        IDkmModuleModifiedNotification
+        IDkmModuleModifiedNotification,
+        IDkmModuleInstanceUnloadNotification
     {
         static ExpressionCompiler()
         {
@@ -217,6 +218,16 @@ internal static bool ShouldTryAgainWithMoreMetadataBlocks(DkmUtilities.GetMetada
         }
 
         void IDkmModuleModifiedNotification.OnModuleModified(DkmModuleInstance moduleInstance)
+        {
+            RemoveDataItemIfNecessary(moduleInstance);
+        }
+
+        void IDkmModuleInstanceUnloadNotification.OnModuleInstanceUnload(DkmModuleInstance moduleInstance, DkmWorkList workList, DkmEventDescriptor eventDescriptor)
+        {
+            RemoveDataItemIfNecessary(moduleInstance);
+        }
+
+        private void RemoveDataItemIfNecessary(DkmModuleInstance moduleInstance)
         {
             // If the module is not a managed module, the module change has no effect.
             var module = moduleInstance as DkmClrModuleInstance;

diff --git a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.csproj b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.csproj
@@ -29,6 +29,7 @@
     <Compile Include="DynamicFlagsCustomTypeInfo.cs" />
     <Compile Include="EESymbolProvider.cs" />
     <Compile Include="ILSpan.cs" />
+    <Compile Include="MetadataUtilities_Exceptions.cs" />
     <Compile Include="NotImplementedMetadataException.cs" />
     <Compile Include="PDB\MethodDebugInfo.Native.cs" />
     <Compile Include="PDB\MethodDebugInfo.Portable.cs" />

diff --git a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/MetadataUtilities.cs b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/MetadataUtilities.cs
@@ -15,11 +15,8 @@
 
 namespace Microsoft.CodeAnalysis.ExpressionEvaluator
 {
-    internal static class MetadataUtilities
+    internal static partial class MetadataUtilities
     {
-        internal const uint COR_E_BADIMAGEFORMAT = 0x8007000b;
-        internal const uint CORDBG_E_MISSING_METADATA = 0x80131c35;
-
         /// <summary>
         /// Group module metadata into assemblies.
         /// If <paramref name="moduleVersionId"/> is set, the
@@ -412,31 +409,5 @@ internal static ImmutableArray<int> GetSynthesizedMethods(byte[] assembly, strin
             }
             return builder.ToImmutableAndFree();
         }
-
-        private static bool IsBadMetadataException(Exception e)
-        {
-            return GetHResult(e) == COR_E_BADIMAGEFORMAT;
-        }
-
-        internal static bool IsBadOrMissingMetadataException(Exception e, string moduleName)
-        {
-            Debug.Assert(moduleName != null);
-            switch (GetHResult(e))
-            {
-                case COR_E_BADIMAGEFORMAT:
-                    Debug.WriteLine($"Module '{moduleName}' contains corrupt metadata.");
-                    return true;
-                case CORDBG_E_MISSING_METADATA:
-                    Debug.WriteLine($"Module '{moduleName}' is missing metadata.");
-                    return true;
-                default:
-                    return false;
-            }
-        }
-
-        private static uint GetHResult(Exception e)
-        {
-            return unchecked((uint)e.HResult);
-        }
     }
 }
diff --git a/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/MetadataUtilities_Exceptions.cs b/src/ExpressionEvaluator/Core/Source/ExpressionCompiler/MetadataUtilities_Exceptions.cs
@@ -0,0 +1,45 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System;
+using System.Diagnostics;
+
+namespace Microsoft.CodeAnalysis.ExpressionEvaluator
+{
+    internal static partial class MetadataUtilities
+    {
+        internal const uint COR_E_BADIMAGEFORMAT = 0x8007000b;
+        internal const uint CORDBG_E_MISSING_METADATA = 0x80131c35;
+
+        internal static bool IsBadOrMissingMetadataException(Exception e, string moduleName)
+        {
+            Debug.Assert(moduleName != null);
+            var objectDisposed = e as ObjectDisposedException;
+            if (objectDisposed != null)
+            {
+                Debug.WriteLine($"Module '{moduleName}' has been disposed.");
+                return true;
+            }
+            switch (GetHResult(e))
+            {
+                case COR_E_BADIMAGEFORMAT:
+                    Debug.WriteLine($"Module '{moduleName}' contains corrupt metadata.");
+                    return true;
+                case CORDBG_E_MISSING_METADATA:
+                    Debug.WriteLine($"Module '{moduleName}' is missing metadata.");
+                    return true;
+                default:
+                    return false;
+            }
+        }
+
+        private static bool IsBadMetadataException(Exception e)
+        {
+            return GetHResult(e) == COR_E_BADIMAGEFORMAT;
+        }
+
+        private static uint GetHResult(Exception e)
+        {
+            return unchecked((uint)e.HResult);
+        }
+    }
+}
diff --git a/src/ExpressionEvaluator/Core/Source/FunctionResolver/FunctionResolver.cs b/src/ExpressionEvaluator/Core/Source/FunctionResolver/FunctionResolver.cs
@@ -119,7 +119,7 @@ internal sealed override MetadataReader GetModuleMetadata(DkmClrModuleInstance m
             {
                 ptr = module.GetMetaDataBytesPtr(out length);
             }
-            catch (Exception e) when (IsBadOrMissingMetadataException(e))
+            catch (Exception e) when (MetadataUtilities.IsBadOrMissingMetadataException(e, module.FullName))
             {
                 return null;
             }
@@ -192,21 +192,6 @@ private static bool ShouldEnable(DkmProcess process)
             }
         }
 
-        private const uint COR_E_BADIMAGEFORMAT = 0x8007000b;
-        private const uint CORDBG_E_MISSING_METADATA = 0x80131c35;
-
-        private static bool IsBadOrMissingMetadataException(Exception e)
-        {
-            switch (unchecked((uint)e.HResult))
-            {
-                case COR_E_BADIMAGEFORMAT:
-                case CORDBG_E_MISSING_METADATA:
-                    return true;
-                default:
-                    return false;
-            }
-        }
-
         private sealed class FunctionResolverDataItem : DkmDataItem
         {
             internal FunctionResolverDataItem(bool enabled)

diff --git a/src/ExpressionEvaluator/Core/Source/FunctionResolver/FunctionResolver.csproj b/src/ExpressionEvaluator/Core/Source/FunctionResolver/FunctionResolver.csproj
@@ -36,6 +36,9 @@
     <Compile Include="..\..\..\..\Compilers\CSharp\Portable\Syntax\SyntaxKindFacts.cs">
       <Link>CSharp\Compiler\SyntaxKindFacts.cs</Link>
     </Compile>
+    <Compile Include="..\ExpressionCompiler\MetadataUtilities_Exceptions.cs">
+      <Link>ExpressionCompiler\MetadataUtilities_Exceptions.cs</Link>
+    </Compile>
     <Compile Include="CSharp\CSharpFunctionResolver.cs" />
     <Compile Include="CSharp\GreenNode.cs" />
     <Compile Include="CSharp\MemberSignatureParser.cs" />

diff --git a/...ssionEvaluator/VisualBasic/Source/ExpressionCompiler/BasicExpressionCompiler.vsdconfigxml b/...ssionEvaluator/VisualBasic/Source/ExpressionCompiler/BasicExpressionCompiler.vsdconfigxml
@@ -24,6 +24,7 @@
           CallOnlyWhenLoaded="true">
           <NoFilter/>
           <Interface Name="IDkmModuleModifiedNotification"/>
+          <Interface Name="IDkmModuleInstanceUnloadNotification"/>
         </InterfaceGroup>
       </Implements>
     </Class>