Why does comparing 2 .NET framework classes with eachother result in a stackoverflow exception?

Question

Problem

I'm currently working on creating an application. In this application I was working with serializing a Func. This somehow crashed my application without an exception.

Crashing without an exception made me curious on wtf is going on so I did some deep diving and after some digging finally found out that somewhere within Newtonsoft.Json a List.Contains is happening which is then executing an equals check on 2 properties.

Apparently in this equals check results in an infinite loop which causes a stackoverflow exception.

Reproducing the issue with just C#

Expression<Func<string, int>> expr = (t) => t.Length;
Func<string, int> exprCompiled = expr.Compile();

var aa = exprCompiled.Method.Module;
var bb = exprCompiled.Method.Module.Assembly;

//This code results in either an infinite loop or a Stackoverflow Exception
var tempresult = aa.Equals(bb);

Console.WriteLine("This code is never executed");

Reproduce the issue with Newtonsoft.Json

Expression<Func<string, int>> expr = (t) => t.Length;
Func<string, int> exprCompiled = expr.Compile();

//This code results in either an infinite loop or a Stackoverflow Exception
var res = JsonConvert.SerializeObject(exprCompiled);

Console.WriteLine("This code is never executed");

Actual underlying issue

Doing a bit more digging into how the .NET framework works I think the issue is with the implementation of the InternalAssemblyBuilder internal class and the InternalModuleBuilder internal classes. Both of them have an Equals method override like this:

public override bool Equals(object obj)
{
    if (obj == null)
    {
        return false;
    }
    if (obj is InternalAssemblyBuilder)
    {
        return this == obj;
    }
    return obj.Equals(this);
}

I think it should be this:

public override bool Equals(object obj)
{
    if (obj == null)
    {
        return false;
    }
    if (obj is InternalAssemblyBuilder)
    {
        return this == obj;
    }
    return base.Equals(this); //changed obj to base
}

Answer 1

As stated in your question's Actual underlying issue as well as by NineBerry in comments, Microsoft's implementations of InternalAssemblyBuilder.Equals(object) and InternalModuleBuilder.Equals(object) appear to be broken. Specifically, in the case of checking equality between an object of type InternalAssemblyBuilder and an object of type InternalModuleBuilder, an infinite recursion will occur.

To work around this issue you can set a custom IEqualityComparer on JsonSerializer.SettingsEqualityComparer that substitutes plausible implementations of Equals() for these types. The following is one such example using reference equality:

public class CustomJsonEqualityComparer : IEqualityComparer
{
    public static readonly CustomJsonEqualityComparer Instance = new CustomJsonEqualityComparer();

    // Use ImmutableHashSet in later .net versions
    static readonly HashSet<string> naughtyTypes = new HashSet<string>
    {
        "System.Reflection.Emit.InternalAssemblyBuilder",
        "System.Reflection.Emit.InternalModuleBuilder"
    };

    static readonly IEqualityComparer baseComparer = EqualityComparer<object>.Default;

    static bool HasBrokenEquals(Type type)
    {
        return naughtyTypes.Contains(type.FullName);
    }

    #region IEqualityComparer Members

    public bool Equals(object x, object y)
    {
        // Check reference equality
        if ((object)x == y)
            return true;
        // Check null
        else if ((object)x == null || (object)y == null)
            return false;

        var xType = x.GetType();
        if (xType != y.GetType())
            // Types should be identical.
            // Note this check alone might be sufficient to fix the problem.
            return false;

        if (xType.IsClass && !xType.IsPrimitive) // IsPrimitive check for performance
        {
            if (HasBrokenEquals(xType))
            {
                // These naughty types should ONLY be compared via reference equality -- which we have already done.
                // So return false
                return false;
            }
        }
        return baseComparer.Equals(x, y);
    }

    public int GetHashCode(object obj)
    {
        return baseComparer.GetHashCode(obj);
    }

    #endregion
}

Then you would use it as follows:

var settings = new JsonSerializerSettings
{
    EqualityComparer = CustomJsonEqualityComparer.Instance,
};
var json = JsonConvert.SerializeObject(exprCompiled, settings);

Notes:

• You may need to tweak CustomJsonEqualityComparer.HasBrokenEquals() if other System.Reflection.Emit types have similarly broken implementations of Equals().

• It might be sufficient to ensure that the two incoming objects have the same System.Type value for GetType(), as the broken Equals() methods discovered so far only overflow the stack in the event that two different types are compared, and both have the same bug.

• While I was able to reproduce the infinite recursion and verify it has been fixed using some mockup objects, I was unable to confirm that Json.NET can actually serialize your Func<string, int> exprCompiled.