Make delegates immutable

src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs

@@ -17,10 +17,6 @@ public abstract partial class Delegate : ICloneable, ISerializable
         // _target is the object we will invoke on
         internal object? _target; // Initialized by VM as needed; null if static delegate
 
-        // MethodBase, either cached after first request or assigned from a DynamicMethod
-        // For open delegates to collectible types, this may be a LoaderAllocator object
-        internal object? _methodBase; // Initialized by VM as needed
-
         // _methodPtr is a pointer to the method we will invoke
         // It could be a small thunk if this is a static or UM call
         internal IntPtr _methodPtr;
@@ -130,10 +126,10 @@ public override bool Equals([NotNullWhen(true)] object? obj)
 
             // method ptrs don't match, go down long path
             //
-            if (_methodBase == null || d._methodBase == null || !(_methodBase is MethodInfo) || !(d._methodBase is MethodInfo))
-                return InternalEqualMethodHandles(this, d);
-            else
-                return _methodBase.Equals(d._methodBase);
+            if (Cache.s_methodCache.TryGetValue(this, out object? thisCache) && thisCache is MethodInfo thisMethod &&
+                Cache.s_methodCache.TryGetValue(d, out object? otherCache) && otherCache is MethodInfo otherMethod)
+                return thisMethod.Equals(otherMethod);
+            return InternalEqualMethodHandles(this, d);
         }
 
         public override int GetHashCode()
@@ -156,57 +152,71 @@ public override int GetHashCode()
 
         protected virtual MethodInfo GetMethodImpl()
         {
-            if ((_methodBase == null) || !(_methodBase is MethodInfo))
+            if (Cache.s_methodCache.TryGetValue(this, out object? cachedValue) && cachedValue is MethodInfo methodInfo)
+            {
+                return methodInfo;
+            }
+
+            return GetMethodImplUncached();
+        }
+
+        internal MethodInfo GetMethodImplUncached()
+        {
+            IRuntimeMethodInfo method = FindMethodHandle();
+            RuntimeType? declaringType = RuntimeMethodHandle.GetDeclaringType(method);
+            // need a proper declaring type instance method on a generic type
+            if (declaringType.IsGenericType)
             {
-                IRuntimeMethodInfo method = FindMethodHandle();
-                RuntimeType? declaringType = RuntimeMethodHandle.GetDeclaringType(method);
-                // need a proper declaring type instance method on a generic type
-                if (declaringType.IsGenericType)
+                bool isStatic = (RuntimeMethodHandle.GetAttributes(method) & MethodAttributes.Static) != (MethodAttributes)0;
+                if (!isStatic)
                 {
-                    bool isStatic = (RuntimeMethodHandle.GetAttributes(method) & MethodAttributes.Static) != (MethodAttributes)0;
-                    if (!isStatic)
+                    if (_methodPtrAux == IntPtr.Zero)
                     {
-                        if (_methodPtrAux == IntPtr.Zero)
+                        // The target may be of a derived type that doesn't have visibility onto the
+                        // target method. We don't want to call RuntimeType.GetMethodBase below with that
+                        // or reflection can end up generating a MethodInfo where the ReflectedType cannot
+                        // see the MethodInfo itself and that breaks an important invariant. But the
+                        // target type could include important generic type information we need in order
+                        // to work out what the exact instantiation of the method's declaring type is. So
+                        // we'll walk up the inheritance chain (which will yield exactly instantiated
+                        // types at each step) until we find the declaring type. Since the declaring type
+                        // we get from the method is probably shared and those in the hierarchy we're
+                        // walking won't be we compare using the generic type definition forms instead.
+                        Type? currentType = _target!.GetType();
+                        Type targetType = declaringType.GetGenericTypeDefinition();
+                        while (currentType != null)
                         {
-                            // The target may be of a derived type that doesn't have visibility onto the
-                            // target method. We don't want to call RuntimeType.GetMethodBase below with that
-                            // or reflection can end up generating a MethodInfo where the ReflectedType cannot
-                            // see the MethodInfo itself and that breaks an important invariant. But the
-                            // target type could include important generic type information we need in order
-                            // to work out what the exact instantiation of the method's declaring type is. So
-                            // we'll walk up the inheritance chain (which will yield exactly instantiated
-                            // types at each step) until we find the declaring type. Since the declaring type
-                            // we get from the method is probably shared and those in the hierarchy we're
-                            // walking won't be we compare using the generic type definition forms instead.
-                            Type? currentType = _target!.GetType();
-                            Type targetType = declaringType.GetGenericTypeDefinition();
-                            while (currentType != null)
+                            if (currentType.IsGenericType &&
+                                currentType.GetGenericTypeDefinition() == targetType)
                             {
-                                if (currentType.IsGenericType &&
-                                    currentType.GetGenericTypeDefinition() == targetType)
-                                {
-                                    declaringType = currentType as RuntimeType;
-                                    break;
-                                }
-                                currentType = currentType.BaseType;
+                                declaringType = currentType as RuntimeType;
+                                break;
                             }
-
-                            // RCWs don't need to be "strongly-typed" in which case we don't find a base type
-                            // that matches the declaring type of the method. This is fine because interop needs
-                            // to work with exact methods anyway so declaringType is never shared at this point.
-                            Debug.Assert(currentType != null || _target.GetType().IsCOMObject, "The class hierarchy should declare the method");
-                        }
-                        else
-                        {
-                            // it's an open one, need to fetch the first arg of the instantiation
-                            MethodInfo invoke = this.GetType().GetMethod("Invoke")!;
-                            declaringType = (RuntimeType)invoke.GetParametersAsSpan()[0].ParameterType;
+                            currentType = currentType.BaseType;
                         }
+
+                        // RCWs don't need to be "strongly-typed" in which case we don't find a base type
+                        // that matches the declaring type of the method. This is fine because interop needs
+                        // to work with exact methods anyway so declaringType is never shared at this point.
+                        Debug.Assert(currentType != null || _target.GetType().IsCOMObject, "The class hierarchy should declare the method");
+                    }
+                    else
+                    {
+                        // it's an open one, need to fetch the first arg of the instantiation
+                        MethodInfo invoke = GetType().GetMethod("Invoke")!;
+                        declaringType = (RuntimeType)invoke.GetParametersAsSpan()[0].ParameterType;
                     }
                 }
-                _methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
             }
-            return (MethodInfo)_methodBase;
+            MethodInfo methodInfo = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
+            SetCachedMethod(methodInfo);
+            return methodInfo;
+        }
+
+        // this is called by the VM in addition to calls from managed code
+        internal void SetCachedMethod(object? value)
+        {
+            Cache.s_methodCache.AddOrUpdate(this, value);
         }
 
         public object? Target => GetTarget();
@@ -519,6 +529,14 @@ internal void InitializeVirtualCallStub(IntPtr methodPtr)
         {
             return (_methodPtrAux == IntPtr.Zero) ? _target : null;
         }
+
+        // Caches MethodInfos, added either after first request or assigned from a DynamicMethod
+        // For open delegates to collectible types, this may contain a LoaderAllocator object
+        // that prevents the type from being unloaded.
+        internal static class Cache
+        {
+            public static readonly ConditionalWeakTable<Delegate, object?> s_methodCache = new();
+        }
     }
 
     // These flags effect the way BindToMethodInfo and BindToMethodName are allowed to bind a delegate to a target method. Their

src/coreclr/System.Private.CoreLib/src/System/MulticastDelegate.CoreCLR.cs

@@ -200,10 +200,10 @@ internal void StoreDynamicMethod(MethodInfo dynamicMethod)
                 Debug.Assert(!IsUnmanagedFunctionPtr(), "dynamic method and unmanaged fntptr delegate combined");
                 // must be a secure/wrapper one, unwrap and save
                 MulticastDelegate d = ((MulticastDelegate?)_invocationList)!;
-                d._methodBase = dynamicMethod;
+                d.SetCachedMethod(dynamicMethod);
             }
             else
-                _methodBase = dynamicMethod;
+                SetCachedMethod(dynamicMethod);
         }
 
         // This method will combine this delegate with the passed delegate
@@ -495,6 +495,16 @@ public sealed override int GetHashCode()
         }
 
         protected override MethodInfo GetMethodImpl()
+        {
+            if (Cache.s_methodCache.TryGetValue(this, out object? cachedValue) && cachedValue is MethodInfo methodInfo)
+            {
+                return methodInfo;
+            }
+
+            return GetMulticastMethodImplUncached();
+        }
+
+        internal MethodInfo GetMulticastMethodImplUncached()
         {
             if (_invocationCount != 0 && _invocationList != null)
             {
@@ -515,25 +525,24 @@ protected override MethodInfo GetMethodImpl()
             {
                 // we handle unmanaged function pointers here because the generic ones (used for WinRT) would otherwise
                 // be treated as open delegates by the base implementation, resulting in failure to get the MethodInfo
-                if ((_methodBase == null) || !(_methodBase is MethodInfo))
-                {
-                    IRuntimeMethodInfo method = FindMethodHandle();
-                    RuntimeType declaringType = RuntimeMethodHandle.GetDeclaringType(method);
 
-                    // need a proper declaring type instance method on a generic type
-                    if (declaringType.IsGenericType)
-                    {
-                        // we are returning the 'Invoke' method of this delegate so use this.GetType() for the exact type
-                        RuntimeType reflectedType = (RuntimeType)GetType();
-                        declaringType = reflectedType;
-                    }
-                    _methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
+                IRuntimeMethodInfo method = FindMethodHandle();
+                RuntimeType declaringType = RuntimeMethodHandle.GetDeclaringType(method);
+
+                // need a proper declaring type instance method on a generic type
+                if (declaringType.IsGenericType)
+                {
+                    // we are returning the 'Invoke' method of this delegate so use this.GetType() for the exact type
+                    RuntimeType reflectedType = (RuntimeType)GetType();
+                    declaringType = reflectedType;
                 }
-                return (MethodInfo)_methodBase;
+                MethodInfo methodInfo = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
+                SetCachedMethod(methodInfo);
+                return methodInfo;
             }
 
             // Otherwise, must be an inner delegate of a wrapper delegate of an open virtual method. In that case, call base implementation
-            return base.GetMethodImpl();
+            return GetMethodImplUncached();
         }
 
         // this should help inlining
@@ -595,7 +604,7 @@ private void CtorCollectibleClosedStatic(object target, IntPtr methodPtr, IntPtr
         {
             this._target = target;
             this._methodPtr = methodPtr;
-            this._methodBase = GCHandle.InternalGet(gchandle);
+            this.SetCachedMethod(GCHandle.InternalGet(gchandle));
         }
 
         [DebuggerNonUserCode]
@@ -605,7 +614,7 @@ private void CtorCollectibleOpened(object target, IntPtr methodPtr, IntPtr shuff
             this._target = this;
             this._methodPtr = shuffleThunk;
             this._methodPtrAux = methodPtr;
-            this._methodBase = GCHandle.InternalGet(gchandle);
+            this.SetCachedMethod(GCHandle.InternalGet(gchandle));
         }
 
         [DebuggerNonUserCode]
@@ -614,7 +623,7 @@ private void CtorCollectibleVirtualDispatch(object target, IntPtr methodPtr, Int
         {
             this._target = this;
             this._methodPtr = shuffleThunk;
-            this._methodBase = GCHandle.InternalGet(gchandle);
+            this.SetCachedMethod(GCHandle.InternalGet(gchandle));
             this.InitializeVirtualCallStub(methodPtr);
         }
 #pragma warning restore IDE0060

src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs

@@ -456,7 +456,7 @@ unsafe partial class CorInfoImpl
     {
         private const CORINFO_RUNTIME_ABI TargetABI = CORINFO_RUNTIME_ABI.CORINFO_CORECLR_ABI;
 
-        private uint OffsetOfDelegateFirstTarget => (uint)(3 * PointerSize); // Delegate::m_functionPointer
+        private uint OffsetOfDelegateFirstTarget => (uint)(2 * PointerSize); // Delegate::m_functionPointer
 
         private readonly ReadyToRunCodegenCompilation _compilation;
         private MethodWithGCInfo _methodCodeNode;
@@ -2114,7 +2114,7 @@ private void ceeInfoGetCallInfo(
                 // of shared generic code calling a shared generic implementation method, which should be rare.
                 //
                 // An alternative design would be to add a new generic dictionary entry kind to hold the MethodDesc
-                // of the constrained target instead, and use that in some circumstances; however, implementation of 
+                // of the constrained target instead, and use that in some circumstances; however, implementation of
                 // that design requires refactoring variuos parts of the JIT interface as well as
                 // TryResolveConstraintMethodApprox. In particular we would need to be abled to embed a constrained lookup
                 // via EmbedGenericHandle, as well as decide in TryResolveConstraintMethodApprox if the call can be made

src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp

@@ -4370,7 +4370,7 @@ void MethodContext::repGetEEInfo(CORINFO_EE_INFO* pEEInfoOut)
         pEEInfoOut->offsetOfThreadFrame                        = (unsigned)0x10;
         pEEInfoOut->offsetOfGCState                            = (unsigned)0xc;
         pEEInfoOut->offsetOfDelegateInstance                   = (unsigned)0x8;
-        pEEInfoOut->offsetOfDelegateFirstTarget                = (unsigned)0x18;
+        pEEInfoOut->offsetOfDelegateFirstTarget                = (unsigned)0x10;
         pEEInfoOut->offsetOfWrapperDelegateIndirectCell        = (unsigned)0x40;
         pEEInfoOut->sizeOfReversePInvokeFrame                  = (unsigned)0x8;
         pEEInfoOut->osPageSize                                 = (size_t)0x1000;

src/coreclr/vm/amd64/asmconstants.h

@@ -138,7 +138,7 @@ ASMCONSTANTS_C_ASSERT(OFFSETOF__ThreadExceptionState__m_pCurrentTracker
 
 
 
-#define           OFFSETOF__DelegateObject___methodPtr      0x18
+#define           OFFSETOF__DelegateObject___methodPtr      0x10
 ASMCONSTANT_OFFSETOF_ASSERT(DelegateObject, _methodPtr);
 
 #define           OFFSETOF__DelegateObject___target         0x08
@@ -680,13 +680,13 @@ template<size_t N>
 class FindCompileTimeConstant
 {
 private:
-	FindCompileTimeConstant();
+    FindCompileTimeConstant();
 };
 
 void BogusFunction()
 {
-	// Sample usage to generate the error
-	FindCompileTimeConstant<offsetof(Thread, m_pDomain)> bogus_variable;
-	FindCompileTimeConstant<offsetof(Thread, m_ExceptionState)> bogus_variable2;
+    // Sample usage to generate the error
+    FindCompileTimeConstant<offsetof(Thread, m_pDomain)> bogus_variable;
+    FindCompileTimeConstant<offsetof(Thread, m_ExceptionState)> bogus_variable2;
 }
 #endif // defined(__cplusplus) && defined(USE_COMPILE_TIME_CONSTANT_FINDER)

src/coreclr/vm/arm/asmconstants.h

@@ -64,7 +64,7 @@ ASMCONSTANTS_C_ASSERT(LazyMachState_captureSp == offsetof(LazyMachState, capture
 #define LazyMachState_captureIp     (LazyMachState_captureSp+4)
 ASMCONSTANTS_C_ASSERT(LazyMachState_captureIp == offsetof(LazyMachState, captureIp))
 
-#define DelegateObject___methodPtr      0x0c
+#define DelegateObject___methodPtr      0x08
 ASMCONSTANTS_C_ASSERT(DelegateObject___methodPtr == offsetof(DelegateObject, _methodPtr));
 
 #define DelegateObject___target         0x04

src/coreclr/vm/arm64/asmconstants.h

@@ -109,7 +109,7 @@ ASMCONSTANTS_C_ASSERT(LazyMachState_captureIp == offsetof(LazyMachState, capture
 #define VASigCookie__pNDirectILStub 0x8
 ASMCONSTANTS_C_ASSERT(VASigCookie__pNDirectILStub == offsetof(VASigCookie, pNDirectILStub))
 
-#define DelegateObject___methodPtr      0x18
+#define DelegateObject___methodPtr      0x10
 ASMCONSTANTS_C_ASSERT(DelegateObject___methodPtr == offsetof(DelegateObject, _methodPtr));
 
 #define DelegateObject___target         0x08

src/coreclr/vm/comdelegate.cpp

@@ -2808,8 +2808,8 @@ MethodDesc* COMDelegate::GetDelegateCtor(TypeHandle delegateType, MethodDesc *pT
     LoaderAllocator *pTargetMethodLoaderAllocator = pTargetMethod->GetLoaderAllocator();
     BOOL isCollectible = pTargetMethodLoaderAllocator->IsCollectible();
     // A method that may be instantiated over a collectible type, and is static will require a delegate
-    // that has the _methodBase field filled in with the LoaderAllocator of the collectible assembly
-    // associated with the instantiation.
+    // that has the LoaderAllocator of the collectible assembly associated with the instantiation
+    // stored in the MethodInfo cache.
     BOOL fMaybeCollectibleAndStatic = FALSE;
 
     // Do not allow static methods with [UnmanagedCallersOnlyAttribute] to be a delegate target.

src/coreclr/vm/corelib.h

@@ -249,7 +249,6 @@ DEFINE_METHOD(DECIMAL,              CURRENCY_CTOR,          .ctor,
 
 DEFINE_CLASS_U(System,                 Delegate,            NoClass)
 DEFINE_FIELD_U(_target,                    DelegateObject,   _target)
-DEFINE_FIELD_U(_methodBase,                DelegateObject,   _methodBase)
 DEFINE_FIELD_U(_methodPtr,                 DelegateObject,   _methodPtr)
 DEFINE_FIELD_U(_methodPtrAux,              DelegateObject,   _methodPtrAux)
 DEFINE_CLASS(DELEGATE,              System,                 Delegate)
@@ -258,6 +257,7 @@ DEFINE_FIELD(DELEGATE,            METHOD_PTR,             _methodPtr)
 DEFINE_FIELD(DELEGATE,            METHOD_PTR_AUX,         _methodPtrAux)
 DEFINE_METHOD(DELEGATE,             CONSTRUCT_DELEGATE,     DelegateConstruct,          IM_Obj_IntPtr_RetVoid)
 DEFINE_METHOD(DELEGATE,             GET_INVOKE_METHOD,      GetInvokeMethod,            IM_RetIntPtr)
+DEFINE_METHOD(DELEGATE,             SET_CACHED_METHOD,      SetCachedMethod,            IM_Obj_RetVoid)
 
 DEFINE_CLASS(INT128,               System,                 Int128)
 DEFINE_CLASS(UINT128,              System,                 UInt128)