Remove most uses of RuntimeTypeHandle.Allocate (#45085)

- Refactoring paves way for related work in https://github.com/dotnet/runtime/pull/32520
- Fixes some possible GC holes in the reflection stack

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.CoreCLR.cs
index 5e7dd7d..da6adab 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.CoreCLR.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.CoreCLR.cs
@@ -27,7 +27,7 @@ namespace System.Collections.Generic
 
             if (typeof(IComparable<T>).IsAssignableFrom(typeof(T)))
             {
-                defaultArraySortHelper = (IArraySortHelper<T>)RuntimeTypeHandle.Allocate(typeof(GenericArraySortHelper<string>).TypeHandle.Instantiate(new Type[] { typeof(T) }));
+                defaultArraySortHelper = (IArraySortHelper<T>)RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericArraySortHelper<string>), (RuntimeType)typeof(T));
             }
             else
             {
@@ -62,7 +62,7 @@ namespace System.Collections.Generic
 
             if (typeof(IComparable<TKey>).IsAssignableFrom(typeof(TKey)))
             {
-                defaultArraySortHelper = (IArraySortHelper<TKey, TValue>)RuntimeTypeHandle.Allocate(typeof(GenericArraySortHelper<string, string>).TypeHandle.Instantiate(new Type[] { typeof(TKey), typeof(TValue) }));
+                defaultArraySortHelper = (IArraySortHelper<TKey, TValue>)RuntimeTypeHandle.CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericArraySortHelper<string, string>), (RuntimeType)typeof(TKey), (RuntimeType)typeof(TValue));
             }
             else
             {

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs
index caaedd6..95de9b7 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs
@@ -80,6 +80,9 @@ namespace System.Runtime.CompilerServices
 
         public static void PrepareMethod(RuntimeMethodHandle method, RuntimeTypeHandle[]? instantiation)
         {
+            // defensive copy of user-provided array, per CopyRuntimeTypeHandles contract
+            instantiation = (RuntimeTypeHandle[]?)instantiation?.Clone();
+
             unsafe
             {
                 IntPtr[]? instantiationHandles = RuntimeTypeHandle.CopyRuntimeTypeHandles(instantiation, out int length);

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs
index adc808e..30c9df8 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs
@@ -162,6 +162,12 @@ namespace System
                    || (corElemType == CorElementType.ELEMENT_TYPE_BYREF);                                      // IsByRef
         }
 
+        // ** WARNING **
+        // Caller bears responsibility for ensuring that the provided Types remain
+        // GC-reachable while the unmanaged handles are being manipulated. The caller
+        // may need to make a defensive copy of the input array to ensure it's not
+        // mutated by another thread, and this defensive copy should be passed to
+        // a KeepAlive routine.
         internal static IntPtr[]? CopyRuntimeTypeHandles(RuntimeTypeHandle[]? inHandles, out int length)
         {
             if (inHandles == null || inHandles.Length == 0)
@@ -179,6 +185,12 @@ namespace System
             return outHandles;
         }
 
+        // ** WARNING **
+        // Caller bears responsibility for ensuring that the provided Types remain
+        // GC-reachable while the unmanaged handles are being manipulated. The caller
+        // may need to make a defensive copy of the input array to ensure it's not
+        // mutated by another thread, and this defensive copy should be passed to
+        // a KeepAlive routine.
         internal static IntPtr[]? CopyRuntimeTypeHandles(Type[]? inHandles, out int length)
         {
             if (inHandles == null || inHandles.Length == 0)
@@ -202,8 +214,49 @@ namespace System
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern object Allocate(RuntimeType type);
 
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        internal static extern object CreateInstanceForAnotherGenericParameter(RuntimeType type, RuntimeType genericParameter);
+        internal static object CreateInstanceForAnotherGenericParameter(RuntimeType type, RuntimeType genericParameter)
+        {
+            object? instantiatedObject = null;
+
+            IntPtr typeHandle = genericParameter.GetTypeHandleInternal().Value;
+            CreateInstanceForAnotherGenericParameter(
+                new QCallTypeHandle(ref type),
+                &typeHandle,
+                1,
+                ObjectHandleOnStack.Create(ref instantiatedObject));
+
+            GC.KeepAlive(genericParameter);
+            return instantiatedObject!;
+        }
+
+        internal static object CreateInstanceForAnotherGenericParameter(RuntimeType type, RuntimeType genericParameter1, RuntimeType genericParameter2)
+        {
+            object? instantiatedObject = null;
+
+            IntPtr* pTypeHandles = stackalloc IntPtr[]
+            {
+                genericParameter1.GetTypeHandleInternal().Value,
+                genericParameter2.GetTypeHandleInternal().Value
+            };
+
+            CreateInstanceForAnotherGenericParameter(
+                new QCallTypeHandle(ref type),
+                pTypeHandles,
+                2,
+                ObjectHandleOnStack.Create(ref instantiatedObject));
+
+            GC.KeepAlive(genericParameter1);
+            GC.KeepAlive(genericParameter2);
+
+            return instantiatedObject!;
+        }
+
+        [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)]
+        private static extern void CreateInstanceForAnotherGenericParameter(
+            QCallTypeHandle baseType,
+            IntPtr* pTypeHandles,
+            int cTypeHandles,
+            ObjectHandleOnStack instantiatedObject);
 
         internal RuntimeType GetRuntimeType()
         {
@@ -471,7 +524,6 @@ namespace System
 
         internal RuntimeType Instantiate(Type[]? inst)
         {
-            // defensive copy to be sure array is not mutated from the outside during processing
             IntPtr[]? instHandles = CopyRuntimeTypeHandles(inst, out int instCount);
 
             fixed (IntPtr* pInst = instHandles)
@@ -1199,6 +1251,10 @@ namespace System
                 throw new ArgumentOutOfRangeException(nameof(typeToken),
                     SR.Format(SR.Argument_InvalidToken, typeToken, new ModuleHandle(module)));
 
+            // defensive copy of user-provided array, per CopyRuntimeTypeHandles contract
+            typeInstantiationContext = (RuntimeTypeHandle[]?)typeInstantiationContext?.Clone();
+            methodInstantiationContext = (RuntimeTypeHandle[]?)methodInstantiationContext?.Clone();
+
             IntPtr[]? typeInstantiationContextHandles = RuntimeTypeHandle.CopyRuntimeTypeHandles(typeInstantiationContext, out int typeInstCount);
             IntPtr[]? methodInstantiationContextHandles = RuntimeTypeHandle.CopyRuntimeTypeHandles(methodInstantiationContext, out int methodInstCount);
 
@@ -1232,6 +1288,9 @@ namespace System
 
         internal static IRuntimeMethodInfo ResolveMethodHandleInternal(RuntimeModule module, int methodToken, RuntimeTypeHandle[]? typeInstantiationContext, RuntimeTypeHandle[]? methodInstantiationContext)
         {
+            // defensive copy of user-provided array, per CopyRuntimeTypeHandles contract
+            typeInstantiationContext = (RuntimeTypeHandle[]?)typeInstantiationContext?.Clone();
+            methodInstantiationContext = (RuntimeTypeHandle[]?)methodInstantiationContext?.Clone();
 
             IntPtr[]? typeInstantiationContextHandles = RuntimeTypeHandle.CopyRuntimeTypeHandles(typeInstantiationContext, out int typeInstCount);
             IntPtr[]? methodInstantiationContextHandles = RuntimeTypeHandle.CopyRuntimeTypeHandles(methodInstantiationContext, out int methodInstCount);
@@ -1277,6 +1336,10 @@ namespace System
                 throw new ArgumentOutOfRangeException(nameof(fieldToken),
                     SR.Format(SR.Argument_InvalidToken, fieldToken, new ModuleHandle(module)));
 
+            // defensive copy of user-provided array, per CopyRuntimeTypeHandles contract
+            typeInstantiationContext = (RuntimeTypeHandle[]?)typeInstantiationContext?.Clone();
+            methodInstantiationContext = (RuntimeTypeHandle[]?)methodInstantiationContext?.Clone();
+
             // defensive copy to be sure array is not mutated from the outside during processing
             IntPtr[]? typeInstantiationContextHandles = RuntimeTypeHandle.CopyRuntimeTypeHandles(typeInstantiationContext, out int typeInstCount);
             IntPtr[]? methodInstantiationContextHandles = RuntimeTypeHandle.CopyRuntimeTypeHandles(methodInstantiationContext, out int methodInstCount);

diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h
index f66c599..e45decc 100644 (file)
--- a/src/coreclr/src/vm/ecalllist.h
+++ b/src/coreclr/src/vm/ecalllist.h
@@ -190,7 +190,7 @@ FCFuncEnd()
 
 FCFuncStart(gCOMTypeHandleFuncs)
     FCFuncElement("CreateInstance", RuntimeTypeHandle::CreateInstance)
-    FCFuncElement("CreateInstanceForAnotherGenericParameter", RuntimeTypeHandle::CreateInstanceForGenericType)
+    QCFuncElement("CreateInstanceForAnotherGenericParameter", RuntimeTypeHandle::CreateInstanceForAnotherGenericParameter)
     QCFuncElement("GetGCHandle", RuntimeTypeHandle::GetGCHandle)
     QCFuncElement("FreeGCHandle", RuntimeTypeHandle::FreeGCHandle)
 

diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp
index e8bc4be..22be44f 100644 (file)
--- a/src/coreclr/src/vm/reflectioninvocation.cpp
+++ b/src/coreclr/src/vm/reflectioninvocation.cpp
@@ -515,30 +515,28 @@ DoneCreateInstance:
 }
 FCIMPLEND
 
-FCIMPL2(Object*, RuntimeTypeHandle::CreateInstanceForGenericType, ReflectClassBaseObject* pTypeUNSAFE, ReflectClassBaseObject* pParameterTypeUNSAFE) {
-    FCALL_CONTRACT;
-
-    struct _gc
-    {
-        OBJECTREF rv;
-        REFLECTCLASSBASEREF refType;
-        REFLECTCLASSBASEREF refParameterType;
-    } gc;
-
-    gc.rv = NULL;
-    gc.refType = (REFLECTCLASSBASEREF)ObjectToOBJECTREF(pTypeUNSAFE);
-    gc.refParameterType = (REFLECTCLASSBASEREF)ObjectToOBJECTREF(pParameterTypeUNSAFE);
+void QCALLTYPE RuntimeTypeHandle::CreateInstanceForAnotherGenericParameter(
+    QCall::TypeHandle pTypeHandle,
+    TypeHandle* pInstArray,
+    INT32 cInstArray,
+    QCall::ObjectHandleOnStack pInstantiatedObject
+)
+{
+    CONTRACTL{
+        QCALL_CHECK;
+        PRECONDITION(!pTypeHandle.AsTypeHandle().IsNull());
+        PRECONDITION(cInstArray >= 0);
+        PRECONDITION(cInstArray == 0 || pInstArray != NULL);
+    }
+    CONTRACTL_END;
 
-    MethodDesc* pMeth;
-    TypeHandle genericType = gc.refType->GetType();
+    TypeHandle genericType = pTypeHandle.AsTypeHandle();
 
-    TypeHandle parameterHandle = gc.refParameterType->GetType();
+    BEGIN_QCALL;
 
     _ASSERTE (genericType.HasInstantiation());
 
-    HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc);
-
-    TypeHandle instantiatedType = ((TypeHandle)genericType.GetCanonicalMethodTable()).Instantiate(Instantiation(&parameterHandle, 1));
+    TypeHandle instantiatedType = ((TypeHandle)genericType.GetCanonicalMethodTable()).Instantiate(Instantiation(pInstArray, (DWORD)cInstArray));
 
     // Get the type information associated with refThis
     MethodTable* pVMT = instantiatedType.GetMethodTable();
@@ -546,24 +544,24 @@ FCIMPL2(Object*, RuntimeTypeHandle::CreateInstanceForGenericType, ReflectClassBa
     _ASSERTE( !pVMT->IsAbstract() ||! instantiatedType.ContainsGenericVariables());
     _ASSERTE(!pVMT->IsByRefLike() && pVMT->HasDefaultConstructor());
 
-    pMeth = pVMT->GetDefaultConstructor();
-    MethodDescCallSite ctor(pMeth);
-
     // We've got the class, lets allocate it and call the constructor
 
     // Nullables don't take this path, if they do we need special logic to make an instance
     _ASSERTE(!Nullable::IsNullableType(instantiatedType));
-    gc.rv = instantiatedType.GetMethodTable()->Allocate();
 
-    ARG_SLOT arg = ObjToArgSlot(gc.rv);
+    {
+        GCX_COOP();
 
-    // Call the method
-    TryCallMethod(&ctor, &arg, true);
+        OBJECTREF newObj = instantiatedType.GetMethodTable()->Allocate();
+        GCPROTECT_BEGIN(newObj);
+        CallDefaultConstructor(newObj);
+        GCPROTECT_END();
 
-    HELPER_METHOD_FRAME_END();
-    return OBJECTREFToObject(gc.rv);
+        pInstantiatedObject.Set(newObj);
+    }
+
+    END_QCALL;
 }
-FCIMPLEND
 
 NOINLINE FC_BOOL_RET IsInstanceOfTypeHelper(OBJECTREF obj, REFLECTCLASSBASEREF refType)
 {

diff --git a/src/coreclr/src/vm/runtimehandles.h b/src/coreclr/src/vm/runtimehandles.h
index 4063fd9..4c5e946 100644 (file)
--- a/src/coreclr/src/vm/runtimehandles.h
+++ b/src/coreclr/src/vm/runtimehandles.h
@@ -247,8 +247,8 @@ public:
     static FCDECL1(MethodDesc *, GetFirstIntroducedMethod, ReflectClassBaseObject* pType);
     static FCDECL1(void, GetNextIntroducedMethod, MethodDesc **ppMethod);
 
-    static FCDECL2(Object*, CreateInstanceForGenericType, ReflectClassBaseObject* pType
-        , ReflectClassBaseObject* parameterType );
+    static
+    void QCALLTYPE CreateInstanceForAnotherGenericParameter(QCall::TypeHandle pTypeHandle, TypeHandle *pInstArray, INT32 cInstArray, QCall::ObjectHandleOnStack pInstantiatedObject);
 
     static
     FCDECL1(IMDInternalImport*, GetMetadataImport, ReflectClassBaseObject * pModuleUNSAFE);