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

Upgrade to Castle Core 4.3.0 #623

Closed
stakx opened this issue Jun 6, 2018 · 13 comments · Fixed by #624
Closed

Upgrade to Castle Core 4.3.0 #623

stakx opened this issue Jun 6, 2018 · 13 comments · Fixed by #624

Comments

@stakx
Copy link
Contributor

stakx commented Jun 6, 2018

Castle Core 4.3.0 has been published earlier today (see castleproject/Core#354 (comment)). Moq should upgrade its dependency because that'll enable:

  • support for in parameter modifiers on .NET Core
  • better reproduction of default parameter values in mock types
@zplan
Copy link

zplan commented Jun 7, 2018

Yes, please upgrate.

In my case it is not even working with Castle Core 4.3.0

I get an exception in "Moq.CastleProxyFactory.CreateProxy" when using CC 4.3.0, With CC 4.2.1 it was working.

@stakx
Copy link
Contributor Author

stakx commented Jun 7, 2018

I've neglected Moq 4 a little during the last few weeks, a new release is long overdue. I'll try to make this happen during the next few days.

@stakx
Copy link
Contributor Author

stakx commented Jun 7, 2018

@zplan:

In my case it is not even working with Castle Core 4.3.0. I get an exception [...]

It would be great if you could provide a short but complete code example that reproduces the issue you're having.

That way, I could validate the upgraded version of Moq even better before I push it to NuGet.

@stakx
Copy link
Contributor Author

stakx commented Jun 9, 2018

@zplan - I've just pushed Moq 4.8.3 to NuGet. It's configured to depend on Castle.Core 4.3.0.

@zplan
Copy link

zplan commented Jun 12, 2018

@stakx : Thanks for the release.
But it looks that this not solve the issue I have, when I use the new Castle Core version.

Do you have any idea when you see this stack trace:
The exception is thrown when i call:
var t = this.wrapperMock.Object;

System.ArgumentException: Constant does not match the defined type..
at System.Reflection.Emit.TypeBuilder.SetConstantValue(ModuleBuilder module, Int32 tk, Type destType, Object value)
at System.Reflection.Emit.ParameterBuilder.SetConstant(Object defaultValue)
at Castle.DynamicProxy.Generators.Emitters.MethodEmitter.CopyDefaultValueConstant(ParameterInfo from, ParameterBuilder to)
at Castle.DynamicProxy.Generators.Emitters.MethodEmitter.DefineParameters(ParameterInfo[] parameters)
at Castle.DynamicProxy.Generators.Emitters.MethodEmitter..ctor(AbstractTypeEmitter owner, String name, MethodAttributes attributes, MethodInfo methodToUseAsATemplate)
at Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.CreateMethod(String name, MethodAttributes attributes, MethodInfo methodToUseAsATemplate)
at Castle.DynamicProxy.Generators.MethodGenerator.Generate(ClassEmitter class, ProxyGenerationOptions options, INamingScope namingScope)
at Castle.DynamicProxy.Contributors.CompositeTypeContributor.ImplementMethod(MetaMethod method, ClassEmitter class, ProxyGenerationOptions options, OverrideMethodDelegate overrideMethod)
at Castle.DynamicProxy.Contributors.CompositeTypeContributor.Generate(ClassEmitter class, ProxyGenerationOptions options)
at Castle.DynamicProxy.Generators.InterfaceProxyWithoutTargetGenerator.GenerateType(String typeName, Type proxyTargetType, Type[] interfaces, INamingScope namingScope)
at Castle.DynamicProxy.Generators.InterfaceProxyWithTargetGenerator.<>c__DisplayClass6_0.b__0(String n, INamingScope s)
at Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(CacheKey cacheKey, Func3 factory) at Castle.DynamicProxy.Generators.InterfaceProxyWithTargetGenerator.GenerateCode(Type proxyTargetType, Type[] interfaces, ProxyGenerationOptions options) at Castle.DynamicProxy.DefaultProxyBuilder.CreateInterfaceProxyTypeWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options) at Castle.DynamicProxy.ProxyGenerator.CreateInterfaceProxyWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, IInterceptor[] interceptors) at Moq.CastleProxyFactory.CreateProxy(Type mockType, IInterceptor interceptor, Type[] interfaces, Object[] arguments) in C:\projects\moq4\Source\ProxyFactories\CastleProxyFactory.cs:line 103 at Moq.Mock1.InitializeInstancePexProtected() in C:\projects\moq4\Source\Mock.Generic.cs:line 295
at Moq.PexProtector.Invoke(Action action) in C:\projects\moq4\Source\PexProtector.cs:line 57
at Moq.Mock1.InitializeInstance() in C:\projects\moq4\Source\Mock.Generic.cs:line 258 at Moq.Mock1.OnGetObject() in C:\projects\moq4\Source\Mock.Generic.cs:line 320
at Moq.Mock`1.get_Object() in C:\projects\moq4\Source\Mock.Generic.cs:line 234

Update:
It looks like there was a change in the MethodEmitter.cs of castle core which leads now to this issue:
castleproject/Core@d75e5b3#diff-d8c107b19efb77444c39c422ff125610

@stakx
Copy link
Contributor Author

stakx commented Jun 12, 2018

@zplan: Please provide a code example. Either here or over in your issue at Castle.Core. I know the code in question that triggers the exception (I wrote it) but to understand why, I'd need to see what type you're trying to mock and what .nET runtime you're running on.

@zplan
Copy link

zplan commented Jun 12, 2018

@stakx : Thanks for your fast reply. It is not so easy to provide code to reproduce the issue but what I see when I debug into CastleCore is:

Framework: .net461

I have an interface like:
public interface IMyInterface
{
ShutdownResponse Shutdown(long timeoutICM = 60);
}
In MethodEmitter.CopyDefaultValueConstant the
'from' value is 'Int64 timeoutICM'
'to' value is

  •   to	{System.Reflection.Emit.ParameterBuilder}	System.Reflection.Emit.ParameterBuilder
      Attributes	16	int
      IsIn	false	bool
      IsOptional	true	bool
      IsOut	false	bool
      MetadataTokenInternal	134217729	int
      Name	"timeoutICM"	string
      Position	1	int
      m_attributes	Optional	System.Reflection.ParameterAttributes
      m_iPosition	1	int
    
  •   m_methodBuilder	{Name: Shutdown 
    

Attributes: 454
Method Signature: Length: 5
Arguments: 0
Signature:
32 0 18 65 10 0

} System.Reflection.Emit.MethodBuilder

  •   m_pdToken	{System.Reflection.Emit.ParameterToken}	System.Reflection.Emit.ParameterToken
      m_strParamName	"timeoutICM"	string
    

to.SetConstant(defaultValue);
--> throws exception (defaultValue was setting to 60)

@stakx
Copy link
Contributor Author

stakx commented Jun 12, 2018

It is not so easy to provide code to reproduce the issue

@zplan, unfortunately, this is an absolutely essential step. There's nothing much I can do to help otherwise.

@stakx
Copy link
Contributor Author

stakx commented Jun 13, 2018

@zplan, two simple questions:

  1. Do you actually define that interface (IMyInterfaceType) yourself, in C# source code? Or is it defined in another assembly that you are referencing? If the latter, do you know how that assembly was compiled (source language, approximate age of the assembly)?

  2. Can you please step into Castle.Core once more and tell me the type of defaultValue? We know the value is 60, but is that an int or a long?

@zplan
Copy link

zplan commented Jun 14, 2018

@stakx : Thanks for your feedback.
To 1: The IMyInterfaceType is just an example, and when I debug this it works and the long is handled as a long. The real interface where I have the issue is in another assembly which was generated by a code generator.
The assembly has
Target Runtime: v4.0.30319
Platform Target: Any

To 2:
Debugging with the IMyInterfaceType shows "DefaultValue 60 object {long}" --> this works
Debugging with the real interface shows "DefaultValue 60 object {int}" --> this crashes, but the reflector shows that the interface contains a long. But somehow it is represented as an int during runtime and this leads to an ArgumentException

@stakx
Copy link
Contributor Author

stakx commented Jun 14, 2018

@zplan: awesome, thanks. Then I think I know what the problem is. I'm already working on a fix, you can follow progress over at the Castle.Core repo. A new release is being discussed currently.

@zplan
Copy link

zplan commented Jun 14, 2018

@stakx Great!!! Looking forward for the new release :)
Since I had now looked into the internals of castle core, I am curious about the root cause. Can you explain the cause? Only if it is an easy explanation otherwise I will check the commit.
Thanks again

@stakx
Copy link
Contributor Author

stakx commented Jun 14, 2018

@zplan - It's pretty involved, but I'll try to summarise. Basically, in IL metadata, there are two mechanisms that can be used to record an optional parameter's default value.

  • Record an entry in the Metadata constant table (see ECMA-335, section II.22.9) and link that entry to the optional parameter. This works only for primitive / built-in types because you cannot encode arbitrary objects in metadata. It's important to note here that the specification does not require that recorded constant value has the same type as the parameter it's linked to!

  • Attaching a custom attribute to the parameter that holds the default value. This is done for System.DateTime and System.Decimal because they are not primitive types.

System.Reflection and System.Reflection.Emit tries to hide that distinction from you, but does a pretty poor job. When you query ParameterInfo.[Raw]DefaultValue, the default value might originate in a custom attribute, or in the Metadata table. When you call ParameterBuilder.SetConstant, however, you're targeting the Metadata table exclusively.

Reflection also converts values automatically to the right target type, but only in some instances. In others, querying the default value will give you the value as recorded, and you can observe that it doesn't match the actual parameter type.

So basically, Reflection is pretty broken in that regard. DynamicProxy tries hard to work around all the limitations in Reflection, which have been documented and even reported to the Mono & CLR team recently.

What must be going wrong in your case is that there's a metadata entry of IL type int32 for your parameter which has type int64. Both the reflector tool as well as VS IntelliSense / Roslyn are smart enough to simply coerce the recorded default value from int32 to int64. But reflection and DynamicProxy aren't. I'm about to change the latter limitation.

If you want to know more, I'd like to direct you to this section of code—read on from there:

https://github.com/castleproject/Core/blob/8012a37e1aa74b3aeda729c4f895c71fdae085f2/src/Castle.Core/DynamicProxy/Generators/Emitters/MethodEmitter.cs#L164-L177

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants