Simplify managed MetadataImport creation (#101353)

* Simplify managed MetadataImport creation

Consolidate the create mechanism for the managed
MetadataImport type. This also removes some
Helper Method Frame usage.

* Always get the latest metadataimport instance.
The field on the managed object can become
stale during HotReload or EnC scenarios.

---------

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

Author: AaronRobinsonMSFT

src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs

@@ -205,9 +205,6 @@ internal readonly partial struct MetadataImport
 #pragma warning restore CA1067
     {
         private readonly IntPtr m_metadataImport2;
-        private readonly object? m_keepalive;
-
-        internal static MetadataImport EmptyImport => new MetadataImport(IntPtr.Zero, null);
 
         #region Override methods from Object
         public override int GetHashCode()
@@ -304,10 +301,16 @@ internal static unsafe MarshalAsAttribute GetMarshalAs(ConstArray nativeType, Ru
         #endregion
 
         #region Constructor
-        internal MetadataImport(IntPtr metadataImport2, object? keepalive)
+        [MethodImpl(MethodImplOptions.InternalCall)]
+        private static extern unsafe IntPtr GetMetadataImport(RuntimeModule module);
+
+        internal MetadataImport(RuntimeModule module)
         {
-            m_metadataImport2 = metadataImport2;
-            m_keepalive = keepalive;
+            ArgumentNullException.ThrowIfNull(module);
+
+            // The MetadataImport instance needs to be acquired in this manner
+            // since the instance can be replaced during HotReload and EnC scenarios.
+            m_metadataImport2 = GetMetadataImport(module);
         }
         #endregion
 

src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs

@@ -215,6 +215,7 @@ internal static CustomAttributeRecord[] GetCustomAttributeRecords(RuntimeModule
                 scope.GetCustomAttributeProps(tkCustomAttributeTokens[i],
                     out records[i].tkCtor.Value, out records[i].blob);
             }
+            GC.KeepAlive(module);
 
             return records;
         }
@@ -250,13 +251,13 @@ internal static CustomAttributeTypedArgument Filter(IList<CustomAttributeData> a
         private RuntimeCustomAttributeData(RuntimeModule scope, MetadataToken caCtorToken, in ConstArray blob)
         {
             m_scope = scope;
-            m_ctor = (RuntimeConstructorInfo)RuntimeType.GetMethodBase(scope, caCtorToken)!;
+            m_ctor = (RuntimeConstructorInfo)RuntimeType.GetMethodBase(m_scope, caCtorToken)!;
 
             if (m_ctor!.DeclaringType!.IsGenericType)
             {
-                MetadataImport metadataScope = scope.MetadataImport;
-                Type attributeType = scope.ResolveType(metadataScope.GetParentToken(caCtorToken), null, null)!;
-                m_ctor = (RuntimeConstructorInfo)scope.ResolveMethod(caCtorToken, attributeType.GenericTypeArguments, null)!.MethodHandle.GetMethodInfo();
+                MetadataImport metadataScope = m_scope.MetadataImport;
+                Type attributeType = m_scope.ResolveType(metadataScope.GetParentToken(caCtorToken), null, null)!;
+                m_ctor = (RuntimeConstructorInfo)m_scope.ResolveMethod(caCtorToken, attributeType.GenericTypeArguments, null)!.MethodHandle.GetMethodInfo();
             }
 
             ReadOnlySpan<ParameterInfo> parameters = m_ctor.GetParametersAsSpan();
@@ -1466,6 +1467,7 @@ private static bool IsCustomAttributeDefined(
                     }
                 }
             }
+            GC.KeepAlive(decoratedModule);
 
             return false;
         }
@@ -1615,6 +1617,7 @@ private static void AddCustomAttributes(
 
                 attributes.Add(attribute);
             }
+            GC.KeepAlive(decoratedModule);
         }
 
         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
@@ -2194,10 +2197,11 @@ internal static bool IsDefined(RuntimeFieldInfo field, RuntimeType? caType)
             if ((method.Attributes & MethodAttributes.PinvokeImpl) == 0)
                 return null;
 
-            MetadataImport scope = ModuleHandle.GetMetadataImport(method.Module.ModuleHandle.GetRuntimeModule());
+            RuntimeModule module = method.Module.ModuleHandle.GetRuntimeModule();
+            MetadataImport scope = module.MetadataImport;
             int token = method.MetadataToken;
-
             scope.GetPInvokeMap(token, out PInvokeAttributes flags, out string entryPoint, out string dllName);
+            GC.KeepAlive(module);
 
             CharSet charSet = CharSet.None;
 
@@ -2252,7 +2256,7 @@ internal static bool IsDefined(RuntimeFieldInfo field, RuntimeType? caType)
 
         private static MarshalAsAttribute? GetMarshalAsCustomAttribute(int token, RuntimeModule scope)
         {
-            ConstArray nativeType = ModuleHandle.GetMetadataImport(scope).GetFieldMarshal(token);
+            ConstArray nativeType = scope.MetadataImport.GetFieldMarshal(token);
 
             if (nativeType.Length == 0)
                 return null;
@@ -2262,10 +2266,15 @@ internal static bool IsDefined(RuntimeFieldInfo field, RuntimeType? caType)
 
         private static FieldOffsetAttribute? GetFieldOffsetCustomAttribute(RuntimeFieldInfo field)
         {
-            if (field.DeclaringType is not null &&
-                field.GetRuntimeModule().MetadataImport.GetFieldOffset(field.DeclaringType.MetadataToken, field.MetadataToken, out int fieldOffset))
-                return new FieldOffsetAttribute(fieldOffset);
-
+            if (field.DeclaringType is not null)
+            {
+                RuntimeModule module = field.GetRuntimeModule();
+                if (module.MetadataImport.GetFieldOffset(field.DeclaringType.MetadataToken, field.MetadataToken, out int fieldOffset))
+                {
+                    return new FieldOffsetAttribute(fieldOffset);
+                }
+                GC.KeepAlive(module);
+            }
             return null;
         }
 
@@ -2291,7 +2300,9 @@ internal static bool IsDefined(RuntimeFieldInfo field, RuntimeType? caType)
                 case TypeAttributes.UnicodeClass: charSet = CharSet.Unicode; break;
                 default: Debug.Fail("Unreachable code"); break;
             }
-            type.GetRuntimeModule().MetadataImport.GetClassLayout(type.MetadataToken, out int pack, out int size);
+            RuntimeModule module = type.GetRuntimeModule();
+            module.MetadataImport.GetClassLayout(type.MetadataToken, out int pack, out int size);
+            GC.KeepAlive(module);
 
             StructLayoutAttribute attribute = new StructLayoutAttribute(layoutKind);
 

src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeModule.cs

@@ -202,7 +202,7 @@ public override byte[] ResolveSignature(int metadataToken)
 
                 if (declaringType.IsGenericType || declaringType.IsArray)
                 {
-                    int tkDeclaringType = ModuleHandle.GetMetadataImport(this).GetParentToken(metadataToken);
+                    int tkDeclaringType = MetadataImport.GetParentToken(metadataToken);
                     declaringType = (RuntimeType)ResolveType(tkDeclaringType, genericTypeArguments, genericMethodArguments);
                 }
 
@@ -353,7 +353,7 @@ public override void GetPEKind(out PortableExecutableKinds peKind, out ImageFile
         #region Internal Members
         internal RuntimeType RuntimeType => m_runtimeType ??= ModuleHandle.GetModuleType(this);
 
-        internal MetadataImport MetadataImport => ModuleHandle.GetMetadataImport(this);
+        internal MetadataImport MetadataImport => new MetadataImport(this);
         #endregion
 
         #region ICustomAttributeProvider Members

src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs

@@ -29,6 +29,11 @@ internal static ParameterInfo GetReturnParameter(IRuntimeMethodInfo method, Memb
         private static ParameterInfo[] GetParameters(
             IRuntimeMethodInfo methodHandle, MemberInfo member, Signature sig, out ParameterInfo? returnParameter, bool fetchReturnParameter)
         {
+            // The lifetime rules for MetadataImport expect these two objects to be the same instance.
+            // See the lifetime of MetadataImport, acquired through IRuntimeMethodInfo, but extended
+            // through the MemberInfo instance.
+            Debug.Assert(ReferenceEquals(methodHandle, member));
+
             returnParameter = null;
             int sigArgCount = sig.Arguments.Length;
             ParameterInfo[] args =
@@ -43,7 +48,7 @@ private static ParameterInfo[] GetParameters(
             // are generated on the fly by the runtime.
             if (!MdToken.IsNullToken(tkMethodDef))
             {
-                MetadataImport scope = RuntimeTypeHandle.GetMetadataImport(RuntimeMethodHandle.GetDeclaringType(methodHandle));
+                MetadataImport scope = RuntimeMethodHandle.GetDeclaringType(methodHandle).GetRuntimeModule().MetadataImport;
 
                 scope.EnumParams(tkMethodDef, out MetadataEnumResult tkParamDefs);
 
@@ -73,7 +78,7 @@ private static ParameterInfo[] GetParameters(
                     }
                     else if (!fetchReturnParameter && position >= 0)
                     {
-                        // position beyong sigArgCount?
+                        // position beyond sigArgCount?
                         if (position >= sigArgCount)
                             throw new BadImageFormatException(SR.BadImageFormat_ParameterSignatureMismatch);
 
@@ -86,7 +91,7 @@ private static ParameterInfo[] GetParameters(
             // Fill in empty ParameterInfos for those without tokens
             if (fetchReturnParameter)
             {
-                returnParameter ??= new RuntimeParameterInfo(sig, MetadataImport.EmptyImport, 0, -1, (ParameterAttributes)0, member);
+                returnParameter ??= new RuntimeParameterInfo(sig, default, 0, -1, (ParameterAttributes)0, member);
             }
             else
             {
@@ -97,7 +102,7 @@ private static ParameterInfo[] GetParameters(
                         if (args[i] != null)
                             continue;
 
-                        args[i] = new RuntimeParameterInfo(sig, MetadataImport.EmptyImport, 0, i, (ParameterAttributes)0, member);
+                        args[i] = new RuntimeParameterInfo(sig, default, 0, i, (ParameterAttributes)0, member);
                     }
                 }
             }
@@ -165,7 +170,7 @@ private RuntimeParameterInfo(RuntimeParameterInfo accessor, MemberInfo member)
             PositionImpl = accessor.Position;
             AttrsImpl = accessor.Attributes;
 
-            // Strictly speeking, property's don't contain parameter tokens
+            // Strictly speaking, properties don't contain parameter tokens
             // However we need this to make ca's work... oh well...
             m_tkParamDef = MdToken.IsNullToken(accessor.MetadataToken) ? (int)MetadataTokenType.ParamDef : accessor.MetadataToken;
             m_scope = accessor.m_scope;
@@ -176,7 +181,7 @@ private RuntimeParameterInfo(
             int position, ParameterAttributes attributes, MemberInfo member)
         {
             Debug.Assert(member != null);
-            Debug.Assert(MdToken.IsNullToken(tkParamDef) == scope.Equals(MetadataImport.EmptyImport));
+            Debug.Assert(MdToken.IsNullToken(tkParamDef) == scope.Equals((MetadataImport)default));
             Debug.Assert(MdToken.IsNullToken(tkParamDef) || MdToken.IsTokenOfType(tkParamDef, MetadataTokenType.ParamDef));
 
             PositionImpl = position;
@@ -201,7 +206,7 @@ internal RuntimeParameterInfo(MethodInfo owner, string? name, Type parameterType
             PositionImpl = position;
             AttrsImpl = ParameterAttributes.None;
             m_tkParamDef = (int)MetadataTokenType.ParamDef;
-            m_scope = MetadataImport.EmptyImport;
+            m_scope = default;
         }
         #endregion
 
@@ -239,6 +244,7 @@ public override string? Name
                     if (!MdToken.IsNullToken(m_tkParamDef))
                     {
                         string name = m_scope.GetName(m_tkParamDef).ToString();
+                        GC.KeepAlive(this);
                         NameImpl = name;
                     }
 
@@ -339,6 +345,7 @@ private bool TryGetDefaultValueInternal(bool raw, out object? defaultValue)
             #region Look for a default value in metadata
             // This will return DBNull.Value if no constant value is defined on m_tkParamDef in the metadata.
             defaultValue = MdConstant.GetValue(m_scope, m_tkParamDef, ParameterType.TypeHandle, raw);
+            GC.KeepAlive(this);
 
             // If default value is not specified in metadata, look for it in custom attributes
             if (defaultValue == DBNull.Value)

src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs

@@ -35,7 +35,8 @@ internal RuntimePropertyInfo(
             Debug.Assert(reflectedTypeCache != null);
             Debug.Assert(!reflectedTypeCache.IsGlobal);
 
-            MetadataImport scope = declaredType.GetRuntimeModule().MetadataImport;
+            RuntimeModule module = declaredType.GetRuntimeModule();
+            MetadataImport scope = module.MetadataImport;
 
             m_token = tkProperty;
             m_reflectedTypeCache = reflectedTypeCache;
@@ -47,6 +48,7 @@ internal RuntimePropertyInfo(
                 out _, out _, out _,
                 out m_getterMethod, out m_setterMethod, out m_otherMethod,
                 out isPrivate, out m_bindingFlags);
+            GC.KeepAlive(module);
         }
         #endregion
 
@@ -65,9 +67,9 @@ internal Signature Signature
             {
                 if (m_signature == null)
                 {
-
                     GetRuntimeModule().MetadataImport.GetPropertyProps(
                         m_token, out _, out _, out ConstArray sig);
+                    GC.KeepAlive(this);
 
                     m_signature = new Signature(sig.Signature.ToPointer(), sig.Length, m_declaringType);
                 }
@@ -210,6 +212,7 @@ public override Type[] GetOptionalCustomModifiers()
         internal object GetConstantValue(bool raw)
         {
             object? defaultValue = MdConstant.GetValue(GetRuntimeModule().MetadataImport, m_token, PropertyType.TypeHandle, raw);
+            GC.KeepAlive(this);
 
             if (defaultValue == DBNull.Value)
                 // Arg_EnumLitValueNotFound -> "Literal value was not found."

src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs

@@ -650,14 +650,6 @@ internal static bool SatisfiesConstraints(RuntimeType paramType, RuntimeType[]?
             }
         }
 
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern IntPtr _GetMetadataImport(RuntimeType type);
-
-        internal static MetadataImport GetMetadataImport(RuntimeType type)
-        {
-            return new MetadataImport(_GetMetadataImport(type), type);
-        }
-
         [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "RuntimeTypeHandle_RegisterCollectibleTypeDependency")]
         private static partial void RegisterCollectibleTypeDependency(QCallTypeHandle type, QCallAssembly assembly);
 
@@ -1247,8 +1239,6 @@ internal ModuleHandle(RuntimeModule module)
         }
         #endregion
 
-        #region Internal FCalls
-
         internal RuntimeModule GetRuntimeModule()
         {
             return m_ptr;
@@ -1278,6 +1268,7 @@ public bool Equals(ModuleHandle handle)
 
         public static bool operator !=(ModuleHandle left, ModuleHandle right) => !left.Equals(right);
 
+        #region Internal FCalls
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern IRuntimeMethodInfo GetDynamicMethod(Reflection.Emit.DynamicMethod method, RuntimeModule module, string name, byte[] sig, Resolver resolver);
 
@@ -1336,7 +1327,7 @@ public RuntimeTypeHandle ResolveTypeHandle(int typeToken, RuntimeTypeHandle[]? t
                 }
                 catch (Exception)
                 {
-                    if (!GetMetadataImport(module).IsValidToken(typeToken))
+                    if (!module.MetadataImport.IsValidToken(typeToken))
                         throw new ArgumentOutOfRangeException(nameof(typeToken),
                             SR.Format(SR.Argument_InvalidToken, typeToken, new ModuleHandle(module)));
                     throw;
@@ -1389,7 +1380,7 @@ internal static RuntimeMethodHandleInternal ResolveMethodHandleInternal(RuntimeM
             }
             catch (Exception)
             {
-                if (!GetMetadataImport(module).IsValidToken(methodToken))
+                if (!module.MetadataImport.IsValidToken(methodToken))
                     throw new ArgumentOutOfRangeException(nameof(methodToken),
                         SR.Format(SR.Argument_InvalidToken, methodToken, new ModuleHandle(module)));
                 throw;
@@ -1442,7 +1433,7 @@ public RuntimeFieldHandle ResolveFieldHandle(int fieldToken, RuntimeTypeHandle[]
                 }
                 catch (Exception)
                 {
-                    if (!GetMetadataImport(module).IsValidToken(fieldToken))
+                    if (!module.MetadataImport.IsValidToken(fieldToken))
                         throw new ArgumentOutOfRangeException(nameof(fieldToken),
                             SR.Format(SR.Argument_InvalidToken, fieldToken, new ModuleHandle(module)));
                     throw;
@@ -1485,14 +1476,6 @@ internal static void GetPEKind(RuntimeModule module, out PortableExecutableKinds
         internal static extern int GetMDStreamVersion(RuntimeModule module);
 
         public int MDStreamVersion => GetMDStreamVersion(GetRuntimeModule());
-
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern IntPtr _GetMetadataImport(RuntimeModule module);
-
-        internal static MetadataImport GetMetadataImport(RuntimeModule module)
-        {
-            return new MetadataImport(_GetMetadataImport(module), module);
-        }
         #endregion
     }