MethodInfo objects for same method are not equal

[atykhyy]

Description

In some cases, MethodInfo objects that refer to the same method but are obtained in different ways (reflection vs delegate creation, i.e. ldftn IL instruction) do not compare equal, and have different hash codes. DeclaringType and MetadataToken properties of the two MethodInfos are the same, but their MethodHandles are different. This happens when the method is declared in a value type, but not if it is declared in a reference type. This is very inconvenient because this means that MethodInfos cannot be used as dictionary keys.

Reproduction Steps

MWE:

using System;
using System.Linq.Expressions;
static class Program
{
    static void Main()
    {
        var m1 = ((MethodCallExpression)((Expression<Action>)(() => new C().M())).Body).Method;
        var m2 = new Action(new C().M).Method;
        Console.WriteLine(m1.MetadataToken == m2.MetadataToken); // 'true'
        Console.WriteLine(m1.DeclaringType == m2.DeclaringType); // 'true'
        Console.WriteLine(m1.GetHashCode() == m2.GetHashCode()); // 'true' if C is class, 'false' if C is struct
        Console.WriteLine(m1.Equals(m2)); // 'true' if C is class, 'false' if C is struct
    }
}

struct C { internal void M() {} }

Expected behavior

I expected MethodInfo objects referring to the same method to compare equal.

Actual behavior

In some cases, MethodInfo objects that refer to the same method but are obtained in different ways (reflection vs delegate creation, i.e. ldftn IL instruction) do not compare equal, and have different hash codes.

Regression?

No response

Known Workarounds

No response

Configuration

Windows 10 21H2 x64
Microsoft (R) Visual C# Compiler version 3.11.0-4.21403.6 (ae1fff34)
This behavior is the same in net461 (4.8.4470.0), netcoreapp3.1 (3.1.23), net5.0 (5.0.12) and net6.0 (6.0.1).

Other information

No response

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

In some cases, MethodInfo objects that refer to the same method but are obtained in different ways (reflection vs delegate creation, i.e. ldftn IL instruction) do not compare equal, and have different hash codes. DeclaringType and MetadataToken properties of the two MethodInfos are the same, but their MethodHandles are different. This happens when the method is declared in a value type, but not if it is declared in a reference type. This is very inconvenient because this means that MethodInfos cannot be used as dictionary keys.

Reproduction Steps

MWE:

using System;
using System.Linq.Expressions;
static class Program
{
    static void Main()
    {
        var m1 = ((MethodCallExpression)((Expression<Action>)(() => new C().M())).Body).Method;
        var m2 = new Action(new C().M).Method;
        Console.WriteLine(m1.MetadataToken == m2.MetadataToken); // 'true'
        Console.WriteLine(m1.DeclaringType == m2.DeclaringType); // 'true'
        Console.WriteLine(m1.GetHashCode() == m2.GetHashCode()); // 'true' if C is class, 'false' if C is struct
        Console.WriteLine(m1.Equals(m2)); // 'true' if C is class, 'false' if C is struct
    }
}

struct C { internal void M() {} }

Expected behavior

I expected MethodInfo objects referring to the same method to compare equal.

Actual behavior

In some cases, MethodInfo objects that refer to the same method but are obtained in different ways (reflection vs delegate creation, i.e. ldftn IL instruction) do not compare equal, and have different hash codes.

Regression?

No response

Known Workarounds

No response

Configuration

Windows 10 21H2 x64
Microsoft (R) Visual C# Compiler version 3.11.0-4.21403.6 (ae1fff34)
This behavior is the same in net461 (4.8.4470.0), netcoreapp3.1 (3.1.23), net5.0 (5.0.12) and net6.0 (6.0.1).

Other information

No response

Author:	atykhyy
Assignees:	-
Labels:	area-System.Reflection, untriaged
Milestone:	-

[steveharter]

Verified the issue; depending on how a MethodInfo is obtained from value type, the same method can have different MethodInfo instances:

    Console.WriteLine(object.ReferenceEquals(m1, m2)); // 'false'

@buyaa-n per research into #46272 is there a single cache for MethodInfo no matter how it was obtained? If so, it seems like this issue can be fixed with that or if we are changing the caching mechanism. In either case, we should also ensure that results are deterministic when obtaining MethodInfo and friends in different ways.

[atykhyy]

FWIW having only a single MethodInfo instance for each method does not matter for many applications. As long as different instances which refer to the same method all compare equal (with Equals and GetHashCode), dictionaries, hashsets and ==/!= will behave in the expected manner.

[steveharter]

FWIW having only a single MethodInfo instance for each method does not matter for many applications. As long as different instances which refer to the same method all compare equal (with Equals and GetHashCode), dictionaries, hashsets and ==/!= will behave in the expected manner.

Yes based on this hot reload at #69427 preserving instances is probably not feasible. Supporting a proper Equals() and stable GetHashCode() seems reasonable assuming perf is not crippled.

[buyaa-n]

@buyaa-n per research into #46272 is there a single cache for MethodInfo no matter how it was obtained?

Yes there is a single cache for MethodInfo, but as the cache also checks method handles for equality in this case 2 method will be added to the cache

runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs

Lines 87 to 88 in 810a7f9

	internal override bool CacheEquals(object? o) =>
	o is RuntimeMethodInfo m && m.m_handle == m_handle;

In either case, we should also ensure that results are deterministic when obtaining MethodInfo and friends in different ways.

I will check how those methods are created/obtained

[jkotas]

I do not think that this issue and #69427 have the same root cause.

Without hot-reload, we are not supposed to ever end up with multiple MethodInfos pointing to same (non-generic) method.

Internally, the runtime can have multiple MethodDesc* for the same method. These MethodDescs are unboxing and/instantiating stubs. This is runtime implementation detail that is not supposed to leak through the reflection APIs.

The MethodDesc* should be always normalized using MethodDesc::FindOrCreateAssociatedMethodDescForReflection before MethodInfos are created for them. This normalization seems to be missing in Delegate.Method implementation and that's how we end up with two different MethodInfos. We should fix this problem by adding the normalization for Delegate.Method.

I agree that changing Equals as discussed in #69427 would make the repro in this issue work. However, there are likely other reflection APIs that will choke on the non-normalized MethodInfos, so changing Equals would not be a proper fix.