Use GCHandle to hold onto RuntimeTypeInfos

dotnet · Oct 31, 2023 · d43a822 · d43a822
1 parent 7d4fefb
commit d43a822
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 8 deletions.
diff --git a/...m.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.BindingFlags.cs b/...m.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.BindingFlags.cs
@@ -188,7 +188,6 @@ private QueryResult<M> Query<M>(MemberPolicies<M> policies, string optionalName,
         private TypeComponentsCache Cache => _lazyCache ??= new TypeComponentsCache(this);
 
         // Generic cache for scenario specific data. For example, it is used to cache Enum names and values.
-        // TODO: This cache should be attached to the RuntimeType via weak reference, similar to how it is done in CoreCLR.
         internal object? GenericCache
         {
             get => _lazyCache?._genericCache;

diff --git a/...tiveaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs b/...tiveaot/System.Private.CoreLib/src/System/Reflection/Runtime/TypeInfos/RuntimeTypeInfo.cs
@@ -594,7 +594,9 @@ private RuntimeType InitializeType()
             RuntimeTypeHandle runtimeTypeHandle = InternalTypeHandleIfAvailable;
             if (runtimeTypeHandle.IsNull)
             {
-                Interlocked.CompareExchange(ref _type, new RuntimeType(this), null);
+                RuntimeType type = new RuntimeType(this);
+                if (Interlocked.CompareExchange(ref _type, type, null) != null)
+                    type.Free();
             }
             else
             {

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeType.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeType.cs
@@ -2,14 +2,15 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
 using System.Reflection.Runtime.TypeInfos;
 using System.Runtime;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
+using System.Threading;
+
 using Internal.Reflection.Augments;
 using Internal.Reflection.Core.Execution;
 using Internal.Runtime;
@@ -19,7 +20,7 @@ namespace System
     internal sealed unsafe class RuntimeType : TypeInfo, ICloneable
     {
         private MethodTable* _pUnderlyingEEType;
-        private RuntimeTypeInfo? _runtimeTypeInfo;
+        private IntPtr _runtimeTypeInfoHandle;
 
         internal RuntimeType(MethodTable* pEEType)
         {
@@ -28,7 +29,13 @@ internal RuntimeType(MethodTable* pEEType)
 
         internal RuntimeType(RuntimeTypeInfo runtimeTypeInfo)
         {
-            _runtimeTypeInfo = runtimeTypeInfo;
+            // This needs to be a strong handle to prevent the type from being collected and re-created that would end up leaking the handle.
+            _runtimeTypeInfoHandle = RuntimeImports.RhHandleAlloc(runtimeTypeInfo, GCHandleType.Normal);
+        }
+
+        internal void Free()
+        {
+            RuntimeImports.RhHandleFree(_runtimeTypeInfoHandle);
         }
 
         private static bool IsReflectionDisabled => false;
@@ -39,15 +46,44 @@ internal RuntimeType(RuntimeTypeInfo runtimeTypeInfo)
 
         internal EETypePtr ToEETypePtrMayBeNull() => new EETypePtr(_pUnderlyingEEType);
 
-        internal RuntimeTypeInfo GetRuntimeTypeInfo() => _runtimeTypeInfo ?? CreateRuntimeTypeInfo();
+        internal RuntimeTypeInfo GetRuntimeTypeInfo()
+        {
+            IntPtr handle = _runtimeTypeInfoHandle;
+            if (handle != default)
+            {
+                object? runtimeTypeInfo = RuntimeImports.RhHandleGet(handle);
+                if (runtimeTypeInfo != null)
+                {
+                    return Unsafe.As<RuntimeTypeInfo>(runtimeTypeInfo);
+                }
+            }
+            return InitializeRuntimeTypeInfoHandle();
+        }
 
         [MethodImpl(MethodImplOptions.NoInlining)]
-        private RuntimeTypeInfo CreateRuntimeTypeInfo()
+        private RuntimeTypeInfo InitializeRuntimeTypeInfoHandle()
         {
             if (IsReflectionDisabled)
                 throw new NotSupportedException(SR.Reflection_Disabled);
 
-            return (_runtimeTypeInfo = ExecutionDomain.GetRuntimeTypeInfo(_pUnderlyingEEType));
+            RuntimeTypeInfo runtimeTypeInfo = ExecutionDomain.GetRuntimeTypeInfo(_pUnderlyingEEType);
+
+            // We assume that the RuntimeTypeInfo unifiers pick a winner when multiple threads
+            // race to create RuntimeTypeInfo.
+
+            IntPtr handle = _runtimeTypeInfoHandle;
+            if (handle == default)
+            {
+                IntPtr tempHandle = RuntimeImports.RhHandleAlloc(runtimeTypeInfo, GCHandleType.Weak);
+                if (Interlocked.CompareExchange(ref _runtimeTypeInfoHandle, tempHandle, default) != default)
+                    RuntimeImports.RhHandleFree(tempHandle);
+            }
+            else
+            {
+                RuntimeImports.RhHandleSet(handle, runtimeTypeInfo);
+            }
+
+            return runtimeTypeInfo;
         }
 
         public override string? GetEnumName(object value)