Proxy created with CreateClassProxyWithTarget returns false for Equals on itself

[Gonyoda]

ProxyGenerator.CreateClassProxyWithTarget returns a proxy instance p where p.Equals(p) returns false.

I can't find any documentation or code to indicate why this happens. We recently updated from 4.4.1 to 5.1.1 and noticed this new behavior/issue with proxy equality. Below is a simple test to demonstrate the error.

Why does p.Equals(p) return false?

public class SomeClass
{
    public virtual string Value { get; set; }
}

[Fact]
public void ProxyClassEqualsBug()
{
    var entity = new SomeClass();
    var entityType = entity.GetType();
        ProxyGenerator proxyGen = new();
    var proxy = proxyGen.CreateClassProxyWithTarget(entityType, entity);

    Assert.True(entity == entity);
    Assert.True(entity.Equals(entity));

    Assert.True(proxy == proxy);
    Assert.True(proxy.Equals(proxy)); // fails
}

[Gonyoda]

I discovered is has to do with ForwardingMethodGenerator - it is replacing proxy with the proxy's target.
It's like the generated code is re-writing my code from: proxy.Equals(proxy); to: entity.Equals(proxy)

Replacing this with the target makes sense for most cases, but sure is weird with Equals. Is this expected? Why did earlier versions of Castle not have this behavior for Equals?

We are throwing the proxies into a Dictionary - we can avoid this problem using a ReferenceEquals IEqualityComparer implementation. But since our case is library code, I'm not sure how downstream code uses the proxied instances, they could have their own dependency on Equals to return true for proxy.Equals(proxy) in their own Dictionaries. Seems worthy of either documenting this behavior, or fixing it. But I'm not sure which is best.

[stakx]

Hi @Gonyoda, and sorry for the slow reply.

What you're observing is caused by #571, which got merged and first released in version 5.0.0 of this library. Because your proxy does not intercept Equals, the call gets forwarded to the target (entity). Because SomeClass is a class (reference type) and does not override Equals, the default reference equality semantics hold, and therefore proxy.Equals(proxy) yields false (just like entity.Equals(proxy) would return that vaue, too); proxy.Equals(entity) OTOH would yield true, because only entity (reference-) equals itself.

I don't think that short of reverting the change we made in #571, there's not really anything we can do here. (But it's unlikely we want to reverse this, it'll just re-cause other issues that have been resolved by it.)

What you could consider doing is to intercept the Equals method:

public class EqualsTargetInterceptor : IInterceptor
{
    private static MethodInfo equalsMethod = typeof(object).GetMethod("Equals", BindingFlags.Public | BindingFlags.Instance);

    public void Intercept(IInvocation invocation)
    {
        if (invocation.Method == equalsMethod)
        {
            invocation.ReturnValue = object.ReferenceEquals(invocation.Proxy, invocation.Arguments[0]);
                                                         // ^^^^^^^^^^^^^^^^
                                                         // instead of invocation.InvocationTarget
        }
        else
        {
            invocation.Proceed();
        }
    }
}

You'd also need to use a custom IProxyGenerationHook because object members are by default excluded from interception:

[Serializable]
public class InterceptEqualsHook : AllMethodsHook
{
    private static MethodInfo equalsMethod = typeof(object).GetMethod("Equals", BindingFlags.Public | BindingFlags.Instance);

    public override bool ShouldInterceptMethod(Type type, MethodInfo method)
    {
        return base.ShouldInterceptMethod(type, method) || method == equalsMethod;
    }
}

Then use these components during proxy generation:

-var proxy = proxyGen.CreateClassProxyWithTarget(entityType, entity);
+var proxy = proxyGen.CreateClassProxyWithTarget(entityType, entity, new ProxyGenerationOptions { Hook = new InterceptEqualsHook() }, new EqualsTargetInterceptor());

Please let me know whether this is helpful, otherwise I'll close this issue in a two weeks' time or thereabouts.

[Gonyoda]

Thank you for the detailed reply explaining why this is happening! I agree that changing it back is not a good solution.

But I will leave this here as to our solution for storing proxies in a Dictionary. First was to make an IEqualityComparer<object> and then use this in the Dictionary<object, T> constructor:

sealed class ReferenceEquality : IEqualityComparer<object>
{
  bool IEqualityComparer<object>.Equals(object? x, object? y) =>
    ReferenceEquals(x, y);

  int IEqualityComparer<object>.GetHashCode(object obj) =>
    obj.GetHashCode();
}

Dictionary declaration:

Dictionary<object, T> _proxyMap = new (new ReferenceEquality());

Then when you create a proxy instance you can store it in the dictionary:

object proxy = proxy = ProxyGen.CreateInterfaceProxyWithTarget( ... );
_proxyMap.Add(proxy, T);

And pull it out safely:

Assert.True(_proxyMap.TryGetValue(proxy, out var tInstance));

I believe the Equals interceptor solution in @stakx's comment is better, but our approach solved our problem as well.