DynamicProxy doesn't support generic method declarations? (Includes temporary fix.)

[Joseph Riesen]

Working with the 7/29 nightly build (or the latest release, for that matter), I was unable to proxy generic method declarations, receiving a TypeLoadException (method signature and body don't match). FYI, when I refer to a 'generic method declaration', I mean something like the following:


public interface MyInterface {
void SomeMethod<T>();
}

public class MyClass : MyInterface {
public void SomeMethod<T>() {
// Do something T-specific.
}
}


(According to Microsoft, a 'generic method' is a method with a generic type as a parameter. But a 'generic method definition' is one with actual type arguments after the method name, like SomeMethod<T> above. If I say 'generic method' later in this post, I really meant 'generic method declaration.')

Attempting to run the following throws a TypeLoadException:


ProxyFactory factory = new ProxyFactory(new MyClass());
MyInterface obj = (MyInterface)factory.GetProxy();


After digging around in the code for a little bit, I think I found the problem - the DefineMethod overrides in TargetProxyMethodBuilder and IntroductionProxyMethodBuilder are not generic-aware. Now I have to apologize in advance; I had a bit of trouble getting JIRA to deal with Spring.NET instead of Spring (Java), so I'm just dumping my fix here. More information on this later.

For example, in TargetProxyMethodBuilder.DefineMethod, I changed it from:


return _typeBuilder.DefineMethod(method.Name, MethodAttributes.Public | CalculateMethodAttributes(), method.CallingConvention,
method.ReturnType, ReflectionUtils.GetParameterTypes(method.GetParame ters()));


...to...


MethodBuilder m = _typeBuilder.DefineMethod(method.Name, MethodAttributes.Public | CalculateMethodAttributes(), method.CallingConvention,
method.ReturnType, ReflectionUtils.GetParameterTypes(method.GetParame ters()));
if (method.IsGenericMethodDefinition) {
m.DefineGenericParameters(ReflectionUtils.GetGener icParameterNames(method));
}
return m;


...where ReflectionUtils.GetGenericParameterNames (which I created) is defined as such:


public static string[] GetGenericParameterNames(MethodInfo method) {
AssertUtils.ArgumentNotNull(method, "method");
return GetGenericParameterNames(method.GetGenericArgument s());
}

public static string[] GetGenericParameterNames(Type[] args)
{
AssertUtils.ArgumentNotNull(args, "args");
string[] names = new string[args.Length];
for (int i = 0; i < args.Length; i++)
{
names[i] = args[i].Name;
}
return names;
}


In other words, every time we use a MethodBuilder to 'clone' the methods on a class (as described by an interface), we must make sure to define the generic types if method.IsGenericMethodDefinition. Otherwise, the interface will describe a generic method definition, but the MethodBuilder will add a non-generic method definition to the TypeBuilder and blow up when the type is generated.

FYI, after making this change to TargetMethodProxyBuilder, I could get my sample class (e.g. from the beginning of this post) to proxy correctly, but the SaoExporter would fail. This is because the SaoExporter uses other proxying functionality in Spring.Core. In the 7/29 build, I noticed MethodBuilder functionality like I'm describing in the following locations:


Spring.Core\Objects\Factory\Support\MethodInjectin gInstantiationStrategy.cs(438)
Spring.Core\Proxy\AbstractProxyTypeBuilder.cs(362)
Spring.Aop\Aop\Framework\DynamicProxy\Introduction ProxyMethodBuilder.cs(56)
Spring.Aop\Aop\Framework\DynamicProxy\TargetProxyM ethodBuilder.cs(58)


(Sorry if the line numbers are a little bit off - I use different brace formatting and I don't know if VS.NET reformatted anything.)

After adding the IsGenericMethodDefinition conditional in these four locations, the SaoExporter also began proxying classes with generic method definitions correctly.

...

So that's that. This isn't a universal fix, because those methods don't exist in .NET 1.1 or 1.0 - I'm sure you guys use conditional comments for that. However, I'm right in the middle of a project and had to expose a generic method in a SAO (via the SaoExporter), so I got it to work all quick-and-dirty.

Fortunately, there shouldn't be any problems with proxying generic type declarations - to instantiate such a type you must provide all of the type arguments, so I think the problem with proxying classes with generic method declarations is all we have to worry about.

I feel really bad about summing up my fixes and just dumping them on you guys, because you get to write the tests, do the conditional comments, etc. I'm afraid I just don't have the time to put this into a formalized diff right now. Hope it helps you guys out, though! Thanks for a wonderful DI container!

[Bruno Baia]

Hi Joseph,

thanks for reporting, describing and fixing this issue :D

I'm currently working on dynamic proxies, one of the goals was to merge all dynamic proxy code in Spring.Proxy (now Spring.Aop.Framework.DynamicProxy is using Spring.Proxy) to remove the duplicated code you have been confronted.

I already fixed it in my sandbox (code + tests) and i hope to commit everything tonight or tomorrow.

Btw, I added the issue in JIRA (http://opensource.atlassian.com/projects/spring/browse/SPRNET-350).


-Bruno

[Joseph Riesen]

Wow, hey, thanks for the rapid response! Good to know about consolidating the code in Spring.Proxy too - seemed like a bit of a code smell there. ;) I shouldn't have to pull down another nightly version for a while, but I'm glad to know the update will be there soon. Oh, and thanks for squaring that stuff away in JIRA - someday I'll figure out how to use it properly... someday, when I have free time again. :)

[Bruno Baia]

back,

It's working nice in Spring.Proxy and custom proxy related to it (like in SaoExporter), but i can't intercept a generic method in AOP :
System.InvalidOperationException : Late bound operations cannot be performed on types or methods for which ContainsGenericParameters is true.

Have you tried to call the proxied method ?

ProxyFactory factory = new ProxyFactory(new MyClass());
MyInterface obj = (MyInterface)factory.GetProxy();
obj.SomeMethod<string>();


The Aop proxy uses a cached MethodInfo generated with the proxy.
I'm investigating to generate some il code to call MethodInfo.MakeGenericMethod on the cached method.


-Bruno

[Joseph Riesen]

Ooh, oops. Yeah, I just threw together a quick test and found that once you apply interception (IMethodBeforeAdvice, IAfterReturningAdvice and IMethodInterceptor individually all fail, at least - I haven't checked them all) it'll blow out with the error you described.

Sorry about that - I didn't notice it because my biggest priority was just to get the SaoExporter up and exposing my class with a generic method definition. I haven't needed to add any interceptors yet, so I didn't test that.

As I understand it, the problem is that the interceptor logic is trying to invoke the intercepted method... but it hasn't told the method its type parameters. So you call SomeMethod<int>(), but the interceptor is trying to invoke SomeMethod<T>() basically. As per the SDK page for MethodInfo.ContainsGenericParameters, the cached (intercepted) method is, at this point, an 'open constructed method' and cannot be called until it's been closed with Type.MakeGenericType().

(Forgive me if I'm lecturing and you know all of this already, heh. I'm guessing you do. I just like to type out problems like this so I can look back and go, 'oh yeah, that's what I was thinking.')

After digging for a bit, it seems like the problem is that when we actually enter Spring.Aop.Framework.AbstractMethodInvocation.Invo keJoinpoint() and it calls Spring.Objects.ObjectUtils.InvokeMethod to invoke the proxied method, the underlying cached method is still an open constructed method.

But here's the problem - and I suspect you already know this - we don't know what the type arguments were to the original method call (i.e. did they call SomeMethod<int> or SomeMethod<string> or whatever), so we won't know what type arguments to pass to Type.MakeGenericType.

I'm afraid I don't have time to hit the code (and I'm a novice when it comes to MSIL work, though it's close enough to x86 assembly language that I can limp along), but it seems to me the gameplan is something like this:


Call System.Reflection.MethodInfo.GetCurrentMethod() in the IL generated in BaseProxyMethodBuilder.DeclareLocals(), saving it to a new local variable (in the generated method).
Add a 'MethodInfo executingMethod' parameter to BaseCompositionProxy.Invoke() and the AbstractMethodInvocation constructor (I'd recommend against creating an overload because we want to make sure all callers are generic-aware).
In AbstractMethodInvocation.InvokeJoinpoint() we'll check and see if the intercepted method IsGenericMethodDefinition. If there are type parameters to be assigned, we pull them from _executingMethod.GetGenericArguments and then return ObjectUtils.InvokeMethod(Method.GetType().MakeGene ricType(typeArguments), target, arguments); or somesuch. (Alternatively, you may want to change ObjectUtils.InvokeMethod - whose only caller is InvokeJoinpoint at the moment - to have an optional type argument array or accept the executing method or somesuch. Not sure what to do there.)


At least, it sounds like a good plan. Given the sheer amount of warnings and exceptions on the Type.MakeGenericType page in the SDK, I'm not sure it'll be as simple as this, and we'll probably want test cases for arrays of generic types, nested generic types (with and without shared type parameters), and all that fun stuff. Also, I'm not sure if IsGenericMethodDefinition is adequate, or if ContainsGenericParameters might be required. (I think ContainsGenericParameters will only differ from IsGenericMethodDefinition if you're dealing with a method on an open class definition, not a method on an actual, real, concrete object, but I don't know for sure.)

Anyway, I may have some free time in a few days to take a stab at some of this. I'll let you know if I get anywhere, if-and-when I get a chance. Hope this helps!

[Bruno Baia]

Hi Joseph,

I resolved it by calling MakeGenericMethod in the proxied method.
Here is how looks like the proxy generated with your sample (used for tests) :


public class CompositionAopProxy_8f8e2f2788ea4608bffd71491e2a4b 48 : BaseCompositionProxy, AbstractAopProxyTests.InterfaceWithGenericMethod
{
// Methods
public CompositionAopProxy_8f8e2f2788ea4608bffd71491e2a4b 48(IAdvised advised1) : base(advised1)
{
}

public void SomeMethod<T, L>()
{
CompositionAopProxy_8f8e2f2788ea4608bffd71491e2a4b 48._m5670cab586994a04a5c61500701a8dc7 = CompositionAopProxy_8f8e2f2788ea4608bffd71491e2a4b 48._m5670cab586994a04a5c61500701a8dc7.MakeGenericM ethod(new Type[] { typeof(T), typeof(L) });
if (base.m_advised.ExposeProxy)
{
AopContext.PushProxy(this);
}
Type type1 = base.m_targetType;
IList list1 = base.GetInterceptors(type1, "_m5670cab586994a04a5c61500701a8dc7", CompositionAopProxy_8f8e2f2788ea4608bffd71491e2a4b 48._m5670cab586994a04a5c61500701a8dc7);
using (ITargetSourceWrapper wrapper1 = base.m_targetSourceWrapper)
{
if (list1.Count > 0)
{
base.Invoke(this, base.m_targetSourceWrapper.GetTarget(), type1, CompositionAopProxy_8f8e2f2788ea4608bffd71491e2a4b 48._m5670cab586994a04a5c61500701a8dc7, null, list1);
}
else
{
((AbstractAopProxyTests.InterfaceWithGenericMethod ) base.m_targetSourceWrapper.GetTarget()).SomeMethod<T, L>();
}
}
if (base.m_advised.ExposeProxy)
{
AopContext.PopProxy();
}
}

// Cached MethodInfo Fields
private static MethodInfo _m5670cab586994a04a5c61500701a8dc7;
}


What's changing, when we have a generic method, is the first line in the generic method that update the MethodInfo and the field is not readonly.

Now we can write poincuts that check generic type arguments, and so apply some aspects according to them.

I planned to commit everything tonight, i'll ping back here.

Sorry for not having updated this topic after fixed the bug, that would prevent you from writing this amazing post :D

-Bruno

[Joseph Riesen]

Haha... that's quite okay. I rather enjoyed taking a break and going through the proxy generation and MSIL stuff. I'm impressed. :)

Wow... that's a much simpler approach than I would have thought possible. Guess I overthought the problem, heh.

Upon first reading it I realized that you could just replace _targetMethod with the retval from MakeGenericType entirely in the MSIL and not worry about sending it in later parameters. I don't think in MSIL yet. :P However, I didn't get how you could avoid calling MethodInfo.GetExecutingMethod()... since a caller could hand you any sort of type argument, there's no way to predict a type and lock it in during proxy generation.

And then... I noticed the typeof directives. (However you do that in MSIL... Refanytype?). I'd always figured typeof was a compiler-level thing that just stored the static type reference in the binary. Thinking about it, though, it's pretty obvious that if I can do typeof(T) in my generic class or method, typeof() does evaluation at runtime as well.

Next time I want to do something funky in MSIL, I'll have to remember to plan it out in C#, then do the MSIL, then decompile it (I'm guessing you use Reflector) and compare. The last thing I expected to see was a disassembly of the AOP proxy, good stuff!

Well hats off to you! All that programming experience of yours pays off. :) I look forward to taking a look at your changes to the code whenever I can get a chance.

[Bruno Baia]

Hi Joseph,

I commited the fix, download the latest nightly build and give it a try.

-Bruno

[Joseph Riesen]

Bruno:

Finally had a chance to get some more work in on the project which instigated this thread. Not sure whether to go to JIRA with this (I'm bad with it anyway, haha). And I like the pretty forum editor - sorry if it means extra work for you.

Anyway, I just ran into a very interesting problem. Took me a while to figure it out, since my unit tests were (mostly) working correctly. Ah... try calling the proxied method twice.


SomeMethod<object>();
SomeMethod<object>();


...upon executing the second line of code, we get:


Void SomeMethod[Object]() is not a GenericMethodDefinition.
MakeGenericMethod may only be called on a method for which
MethodBase.IsGenericMethodDefinition is true.


...and what's funny is that you'll get the exact same exception if you switch up the type arguments, e.g. SomeMethod<int>(); for the second call - it still says 'Void SomeMethod[Object]()'.

Looking at the decompiled proxy MSIL (from your previous post), I think setting the 'original method' instance variable to the output of MakeGenericMethod is the problem, that is:


CompositionAopProxy_8f8e2f2788ea4608bffd71491e2a4b 48._m5670cab586994a04a5c61500701a8dc7 = CompositionAopProxy_8f8e2f2788ea4608bffd71491e2a4b 48._m5670cab586994a04a5c61500701a8dc7.MakeGenericM ethod(new Type[] { typeof(T), typeof(L) });


...this permanently binds the method into the type arguments provided by the first call made to the method. Instead, the output of MakeGenericMethod should be stored in a local variable for that method call only... that way you can, say, call with different type arguments later and other goodness. :)

I'd throw together a patch, but I'm not so good with MSIL and seem chronically short on time nowadays, heh. But let me know if you have any questions or trouble reproducing the problem and I'll be glad to help further.

Thanks in advance!

Joseph

[Bruno Baia]

Hi Joseph,

You are right, stupid me :)
I'll investigate this... The local variable can be the solution.


-Bruno

[Bruno Baia]

Hi Joseph,

I commited a fix.
I used a local variable to store the output of MakeGenericMethod as you propposed.

Give a try to the latest nightly build.

Thanks,
Bruno

[Joseph Riesen]

Bruno:

So far, so good! Works like a charm with different type arguments and the like. I'll let you know if I run into any more problems, but I think I'm good for now.

Thanks again!

Joseph

[Joseph Riesen]

Haha... the problem of Generics is like an onion, with many undiscovered layers beneath.

Tried applying a type constraint, and got a delicious error. I'd post it, but I already forget what exactly it was (I believe it was a System.Security.ValidationException of "the type argument 'T' violates the constraint on type parameter 'T' or somesuch.)

Anyway, this one was more within my ability to solve, so here's a fix.

Mind you, I'm working off the 9/16 nightly build, so my line numbers might be a little off.

First, at Spring.Aop.Tests/App/Framework/DynamicProxy/AbstractAopProxyTests.cs:192 and 197, where we define SomeMethod<T, L>(), I figured I'd add a constraint that wouldn't require any more changes. String and Int32 both implement IComparable, so it becomes:


public interface InterfaceWithGenericMethod
{
void SomeMethod<T, L>() where T : IComparable;
}

public class ClassWithGenericMethod : InterfaceWithGenericMethod
{
public void SomeMethod<T, L>() where T : IComparable
{
// Do something T-specific.
}
}


...run the tests now and you'll see the problem.

Anyway, after a little puzzling, the problem is that the proxied method doesn't get any constraints! When the method is called, T and L are types that demand to be IComparable, but SomeMethod<T, L>() is all the emitted method declaration states.

So, in Spring.Core/Proxy/AbstractProxyMethodBuilder.cs:192, we add a bit more to the .NET 2.0-specific code here. Now mind you, I've been up for almost 24 hours and I'm in a hurry, so... this is just a really, really quick hack, but it gets the job done (and illustrates what needs to be added):


if (method.IsGenericMethodDefinition)
{
GenericTypeParameterBuilder[] genericParamBuilders = methodBuilder.DefineGenericParameters(ReflectionUt ils.GetGenericParameterNames(method));

Type[] genericArguments = method.GetGenericArguments();
for (int i = 0; i < genericArguments.Length; i++) {
genericParamBuilders[i].SetGenericParameterAttributes(genericArguments[i].GenericParameterAttributes);

Type[] constraints = genericArguments[i].GetGenericParameterConstraints();
System.Collections.Generic.List<Type> interfaces = new System.Collections.Generic.List<Type>(constraints.Length);
foreach (Type constraint in constraints) {
if (constraint.IsClass)
genericParamBuilders[i].SetBaseTypeConstraint(constraint);
else
interfaces.Add(constraint);
}
genericParamBuilders[i].SetInterfaceConstraints(interfaces.ToArray());
}
}


...sorry my indent style is all messed up there. You can tell I'm a fan of the new generic List, heh.


Anyway, what this code basically does is add the type parameter constraints onto the proxied method declaration. These come in three flavors: First, an assortment of flags (for stuff like where T : new() or where T : class) is stuffed into the GetGenericParameterAttributes bitfield. Second, the none-or-one base class for the constraint. (Which is why I use that clumsy IsClass - because there should only ever be a maximum of one of those.) Third, none-to-many interface implementations. This code (kludgily) handles all three.

FYI, I puzzled most of this out at http://msdn2.microsoft.com/en-us/library/system.reflection.emit.generictypeparameterbuilder .aspx before realizing there's a full rundown at http://msdn2.microsoft.com/en-us/library/b8ytshk6.aspx.

Anyway, it works for me, even in my extended tests, e.g for interface methods like:


void WithConstraint<Z>() where Z : IComparable;
IList<Z> OtherConstraint<Z>() where Z : class, new();
void OtherConstraint2<X, Y, Z>(IList<X> x, Y y)
where X : Y
where Z : struct, IComparable<Z>;


...and I don't have any problems with it. It might be worth extending the complexity of the Spring.Aop.Tests related to generics, at least with a few more methods on the mock object.

Anyway, just thought I would share. Let me know if you have any questions. Have fun with the refactoring, heh. On the plus side, this appears to be the only place in all of Spring.NET that calls DefineGenericParameters, so this fix -ought- to work everywhere, but I dunno.

Thanks again!

[Bruno Baia]

Hi Joseph,

What can i say ? You are the perfect forum user :D

I've integrated your fix. ( try the latest nightly build (http://www.springframework.net/downloads/nightly/) )

the code is not so kludgly... but one thing I will maybe do is to move some code related with System.Reflection.Emit to a ProxyBuilderUtils class or ReflectionEmitUtils.
Then it will look less kludgy :p


Thanks,
Bruno

[manuj]

I am still having a problem with generic types. Here is the interface:

public interface IDataAccessObject
{
IList FindAll( Type t);

IList FindList( ICriteria criteria );

IList FindList( DetachedCriteria detachedCriteria );

IList<T> FindAll<T>( );
}


and here is the xml configuration:

<!-- This is a generic data access object supplied to all business objects -->
<object id="baseDao" type="Spider.Framework.Data.DataAccess.BaseDataAccessObj ect, Spider.Framework">
<property name="SessionFactory" ref="SessionFactory"/>
</object>

<object id="baseDaoProxy" type="Spring.Aop.Framework.ProxyFactoryObject, Spring.Aop">
<property name="Target" ref="baseDao"/>
</object>


BaseDataAccessObject is a default implementation of the interface.

and I get this exception when spring tries to generate a proxy:

Error creating object with name 'baseDaoProxy' defined in '...' : Initialization of object failed : Signature of the body and declaration in a method implementation do not match. Type: 'Spring.Proxy.CompositionAopProxy_8da90bd16dd14cc3 b176bbfa7aad55ac'. Assembly: 'Spring.Proxy, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.


Any help will be much appreciated.

[Bruno Baia]

Hi,

This have been resolved recently.
which version are you using ?

Bruno

[manuj]

I am using the latest version from CVS. I looked at the code and the method generators are generics aware. But for some reason the exception still occurs.

[Bruno Baia]

hmmmm...

I'm not able to reproduce it.

Have you tried to comment all methods except the generic one ?

I also need to know if the error occurs when generating the proxy or while trying to call a proxied method.
The stacktrace can help in this case.


Bruno

[manuj]

Yes if I remove the generic method - everything works ok.
Here is the stack trace (pretty much all coming from the IL emitter)

at System.Reflection.Emit.TypeBuilder.TermCreateClass (Int32 handle, Module module)
at System.Reflection.Emit.TypeBuilder.CreateTypeNoLoc k()
at System.Reflection.Emit.TypeBuilder.CreateType()
at Spring.Aop.Framework.DynamicProxy.CompositionAopPr oxyTypeBuilder.BuildProxyType() in F:\Spring.Aop\Aop\Framework\DynamicProxy\Compositi onAopProxyTypeBuilder.cs:line 109

[Bruno Baia]

Oki, I got it.

Problem occurs when you have 2 methods with the same name :
1 generic and the other non generic :

IList FindAll( Type t);
IList<T> FindAll<T>( );

You can rename FindAll<T> to FindAllGeneric<T> (or something) to test.

I'm on it.

Thanks for the infos,
Bruno

[manuj]

Awesome!!

that did the trick. now off to integrate generics support in hibernatetemplate.

[Bruno Baia]

Hi,

I fixed this.
I will commit the modifications as soon as SF.net CVS will back online...
It should be in the next nightly build.

Bruno