Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ArgumentException: "Could not find method overriding method" with overridden class method having generic by-ref parameter #655

Closed
rbeurskens opened this issue Jun 15, 2023 · 3 comments · Fixed by #657
Assignees
Labels
Milestone

Comments

@rbeurskens
Copy link

rbeurskens commented Jun 15, 2023

Note that I encountered this using NSubstitute and I'm not using Castle.Core directly, but the stack trace comes from Castle.Core and tells me it is likely a bug and asks me to report it. (NSubstitute issue: nsubstitute/NSubstitute#716 )
Version used is 5.0.0, running on .NET Framework 4.8

System.ArgumentException : Could not find method overriding Boolean TryCreateControl[TResult](IBaseControl, IHasGridLinesDefined, TResult ByRef) on type MyControlFactory. This is most likely a bug. Please report it.
   at Castle.DynamicProxy.Internal.InvocationHelper.ObtainMethod(MethodInfo proxiedMethod, Type type)
   at Castle.Core.Internal.SynchronizedDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Castle.DynamicProxy.Internal.InvocationHelper.GetMethodOnType(Type type, MethodInfo proxiedMethod)
   at NSubstitute.Proxies.CastleDynamicProxy.CastleInvocationMapper.Map(IInvocation castleInvocation)
   at NSubstitute.Proxies.CastleDynamicProxy.CastleForwardingInterceptor.Intercept(IInvocation invocation)
   at Castle.DynamicProxy.AbstractInvocation.Proceed()
   at Castle.DynamicProxy.AbstractInvocation.Proceed()
   at Castle.Proxies.MyControlFactoryProxy.TryCreateControl[TResult](IBaseControl control, IHasGridLinesDefined parentGrid, TResult& result)

This occurs when trying to configure an interception for a (generic) method (with an out argument) that is overridden but not defined on the class being proxied.

public interface IControlFactory
{
    bool TryCreateControl<TResult>(IBaseControl control, IHasGridLinesDefined? parentGrid, [MaybeNullWhen(false)] out TResult result);
}
public class ControlFactory: IControlFactory
{
    public virtual bool TryCreateControl<TResult>(IBaseControl control, IHasGridLinesDefined? parentGrid, [MaybeNullWhen(false)] out TResult result) { /* ... */ }
}
public class MyControlFactory: ControlFactory
{
    // If this override is removed, code runs as expected
    public override bool TryCreateControl<TResult>(IBaseControl control, IHasGridLinesDefined? parentGrid, [MaybeNullWhen(false)] out TResult result) { /* ... */ }
}

[Test]
void MyTest()
{
    // sorry, this is nsubstitute syntax. I don't know how this translates to Castle.Core calls, but I hope it provides enough information on what I am trying to do
    var control = Substitute.For<IStringTextBoxControl>();
    var controlFactory = Substitute.For<MyControlFactory>();
    controlFactory.Configure().TryCreateControl<object>(default, default, out var _)
          .ReturnsForAnyArgs(ci => { ci[2] = control; return true; }); // <== exception here
}
@stakx
Copy link
Member

stakx commented Jun 17, 2023

Thanks for reporting this @rbeurskens. Don't worry about your repro code example making use of NSubstitute for now, I expect it shouldn't be too difficult figuring out the underlying calls and deriving DynamicProxy-only repro code.

@stakx
Copy link
Member

stakx commented Aug 22, 2023

OK, it's taken me a while, but here is a corresponding DynamicProxy-only repro code example:

var generator = new ProxyGenerator();
var factory = generator.CreateClassProxy<DerivedFactory>(new QueryMethodInvocationTargetInterceptor());
factory.Create<object>(out _);

class QueryMethodInvocationTargetInterceptor : IInterceptor
{
    public void Intercept(IInvocation invocation)
    {
        _ = invocation.MethodInvocationTarget;
    }
}

public interface IFactory  // NOTE: the issue is also reproducible without this interface
{
    void Create<T>(out T result);
}

public class Factory : IFactory
{
    public virtual void Create<T>(out T result) => result = default;
}

public class DerivedFactory : Factory
{
    public override void Create<T>(out T result) => result = default;
}

It seems that the error is caused in this method:

private bool EqualSignatureTypes(Type x, Type y)
{
if (x.IsGenericParameter != y.IsGenericParameter)
{
return false;
}
else if (x.IsGenericType != y.IsGenericType)
{
return false;
}
if (x.IsGenericParameter)
{
if (x.GenericParameterPosition != y.GenericParameterPosition)
{
return false;
}
}
else if (x.IsGenericType)
{
var xGenericTypeDef = x.GetGenericTypeDefinition();
var yGenericTypeDef = y.GetGenericTypeDefinition();
if (xGenericTypeDef != yGenericTypeDef)
{
return false;
}
var xArgs = x.GetGenericArguments();
var yArgs = y.GetGenericArguments();
if (xArgs.Length != yArgs.Length)
{
return false;
}
for (var i = 0; i < xArgs.Length; ++i)
{
if(!EqualSignatureTypes(xArgs[i], yArgs[i])) return false;
}
}
else
{
if (!x.Equals(y))
{
return false;
}
}
return true;
}

When given two matching generic types (or type parameters) by-ref (in our repro code: the out T result parameters' types), this method claims that they do not match. This could possibly be fixed by introducing some new logic that erases by-ref-ness for further comparisons:

 private bool EqualSignatureTypes(Type x, Type y)
 {
     if (x.IsGenericParameter != y.IsGenericParameter)
     {
         return false;
     }
     else if (x.IsGenericType != y.IsGenericType)
     {
         return false;
     }
 
+    if (x.IsByRef != y.IsByRef)
+    {
+        return false;
+    }
+
+    if (x.IsByRef)
+    {
+        x = x.GetElementType();
+        y = y.GetElementType();
+    }
+
     if (x.IsGenericParameter)
     ...

But I haven't checked yet whether this code breaks any existing test cases.

The suggested code addition does not appear to break any existing test cases, but we should still double-check its validity, and whether it can possibly be added in a more optimal place (from a run-time performance standpoint).

@stakx stakx added the bug label Aug 22, 2023
@stakx stakx self-assigned this Aug 22, 2023
@stakx
Copy link
Member

stakx commented Aug 22, 2023

Scratch the code fix suggested above, the following would be more correct (because it doesn't skip the first two checks):

 private bool EqualSignatureTypes(Type x, Type y)
 {
+    if (x.IsByRef != y.IsByRef)
+    {
+        return false;
+    }
+    else if (x.IsByRef)
+    {
+        return EqualSignatureTypes(x.GetElementType(), y.GetElementType());
+    }
+
     if (x.IsGenericParameter != y.IsGenericParameter)
     ...

@stakx stakx changed the title ArgumentException: Could not find method overriding method with overridden generic method. ArgumentException: "Could not find method overriding method" with overridden class method having generic by-ref parameter Aug 23, 2023
@stakx stakx added this to the vNext milestone Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants