DynamicProxy does not support reference semantics with value types (C# 7.2) on .NET Core

[TAGC]

(Cross-posting some details from nsubstitute/NSubstitute#378)

C# 7.2 introduced the possibility of using reference semantics for value types - this is done by applying the in modifier for value-type parameters in method signatures.

Castle.Core does not seem to support generating mocks for interfaces that use in modifiers for value types. This has been reproduced using Castle.Core indirectly through the latest stable versions of Moq and NSubstitute:

namespace SomeProject.Tests
{
    public readonly struct Struct
    {
    }

    public interface IStructByRefConsumer
    {
        void Consume(in Struct message);
    }

    public interface IStructByValueConsumer
    {
        void Consume(Struct message);
    }

    public class TempSpec
    {
        // Fails.
        [Fact]
        internal void Struct_ByRef_Moq_Test()
        {
            _ = Mock.Of<IStructByRefConsumer>();
        }

        // Fails.
        [Fact]
        internal void Struct_ByRef_NSubstitute_Test()
        {
            _ = Substitute.For<IStructByRefConsumer>();
        }

        // Passes.
        [Fact]
        internal void Struct_ByValue_Moq_Test()
        {
            _ = Mock.Of<IStructByValueConsumer>();
        }

        // Passes.
        [Fact]
        internal void Struct_ByValue_NSubstitute_Test()
        {
            _ = Substitute.For<IStructByValueConsumer>();
        }
    }
}

Posted below is the part of the stracktrace that's common between the failing Moq and NSubstitute tests:

Message: System.TypeLoadException : Signature of the body and declaration in a method implementation do not match.  Type: 'Castle.Proxies.IStructByRefConsumerProxy'.  Assembly: 'DynamicProxyGenAssembly2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.

Result StackTrace:	
at System.Reflection.Emit.TypeBuilder.TermCreateClass(RuntimeModule module, Int32 tk, ObjectHandleOnStack type)
   at System.Reflection.Emit.TypeBuilder.CreateTypeNoLock()
   at System.Reflection.Emit.TypeBuilder.CreateTypeInfo()
   at Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.CreateType(TypeBuilder type)
   at Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.BuildType()
   at Castle.DynamicProxy.Generators.InterfaceProxyWithoutTargetGenerator.GenerateType(String typeName, Type proxyTargetType, Type[] interfaces, INamingScope namingScope)
   at Castle.DynamicProxy.Generators.InterfaceProxyWithTargetGenerator.<>c__DisplayClass6_0.<GenerateCode>b__0(String n, INamingScope s)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(CacheKey cacheKey, Func`3 factory)
   at Castle.DynamicProxy.ProxyGenerator.CreateInterfaceProxyWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, IInterceptor[] interceptors)

[TAGC]

For what it's worth, here's the generated IL for each interface:

.class interface public abstract auto ansi 
  SomeProject.Tests.IStructByRefConsumer
{

  .method public hidebysig virtual newslot abstract instance void 
    Consume(
      valuetype SomeProject.Tests.Struct& modreq ([System.Runtime.InteropServices]System.Runtime.InteropServices.InAttribute) message
    ) cil managed 
  {
    .param [1] 
    .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() 
      = (01 00 00 00 )
    // Can't find a body
  } // end of method IStructByRefConsumer::Consume
} // end of class SomeProject.Tests.IStructByRefConsumer

.class interface public abstract auto ansi 
  SomeProject.Tests.IStructByValueConsumer
{

  .method public hidebysig virtual newslot abstract instance void 
    Consume(
      valuetype SomeProject.Tests.Struct message
    ) cil managed 
  {
    // Can't find a body
  } // end of method IStructByValueConsumer::Consume
} // end of class SomeProject.Tests.IStructByValueConsumer

[TAGC]

And also, the source code for the last line of that stacktrace.

[stakx]

@TAGC: I cannot reproduce this problem. All of your tests pass just fine for me with the following NuGet package versions:

• Castle.Core 4.2.1
• Moq 4.8.2 / NSubstitute 3.1.0
• xunit 2.3.1
• xunit.runner.visualstudio 2.3.1

Some notes:

• I have run the Moq and NSubstitute tests separately since installing both Moq and NSubstitute at the same time appears to cause an assembly version conflict that I couldn't bother to fix.
• I have .NET 4.7.1 installed (IIRC).
• I haven't tried whether x86 / x64 makes a difference.
• I took a quick look at the dynamic assembly generated by DynamicProxy, it reproduces the method signature just fine (including the modreq).

[TAGC]

Try .NET Core instead of .NET Framework. I've got a suspicion it might be a problem with corefx (or coreclr).

[stakx]

Yes, the tests start failing when run under .NET Core.

If I am putting facts together correctly, then the problem is caused by DynamicProxy not emitting custom modifiers in its .NET Standard 1.3 target (see the FEATURE_EMIT_CUSTOMMODIFIERS compilation symbol in common.props):

Core/buildscripts/common.props

Lines 43 to 44 in c6f75ff

<NetStandardConstants>TRACE;FEATURE_NETCORE_REFLECTION_API;FEATURE_TEST_SERILOGINTEGRATION</NetStandardConstants>

<CommonDesktopClrConstants>TRACE;FEATURE_APPDOMAIN;FEATURE_ASSEMBLYBUILDER_SAVE;FEATURE_BINDINGLIST;FEATURE_DICTIONARYADAPTER_XML;FEATURE_EMIT_CUSTOMMODIFIERS;FEATURE_EVENTLOG;FEATURE_GAC;FEATURE_GET_REFERENCED_ASSEMBLIES;FEATURE_IDATAERRORINFO;FEATURE_ISUPPORTINITIALIZE;FEATURE_LISTSORT;FEATURE_REMOTING;FEATURE_SECURITY_PERMISSIONS;FEATURE_SERIALIZATION;FEATURE_SMTP;FEATURE_SYSTEM_CONFIGURATION;FEATURE_TARGETEXCEPTION;FEATURE_TEST_COM;FEATURE_TEST_SERILOGINTEGRATION</CommonDesktopClrConstants>

Core/buildscripts/common.props

Lines 85 to 87 in c6f75ff

	<PropertyGroup Condition="'$(TargetFramework)|$(Configuration)'=='netstandard1.3|Release'">
	<DefineConstants>$(NetStandardConstants)</DefineConstants>
	</PropertyGroup>

Core/src/Castle.Core/DynamicProxy/Generators/Emitters/MethodEmitter.cs

Lines 179 to 202 in c6f75ff

	#if FEATURE_EMIT_CUSTOMMODIFIERS
	returnRequiredCustomModifiers = returnParameter.GetRequiredCustomModifiers();
	Array.Reverse(returnRequiredCustomModifiers);
	returnOptionalCustomModifiers = returnParameter.GetOptionalCustomModifiers();
	Array.Reverse(returnOptionalCustomModifiers);
	int parameterCount = baseMethodParameters.Length;
	parametersRequiredCustomModifiers = new Type[parameterCount][];
	parametersOptionalCustomModifiers = new Type[parameterCount][];
	for (int i = 0; i < parameterCount; ++i)
	{
	parametersRequiredCustomModifiers[i] = baseMethodParameters[i].GetRequiredCustomModifiers();
	Array.Reverse(parametersRequiredCustomModifiers[i]);
	parametersOptionalCustomModifiers[i] = baseMethodParameters[i].GetOptionalCustomModifiers();
	Array.Reverse(parametersOptionalCustomModifiers[i]);
	}
	#else
	returnRequiredCustomModifiers = null;
	returnOptionalCustomModifiers = null;
	parametersRequiredCustomModifiers = null;
	parametersOptionalCustomModifiers = null;
	#endif

IIRC this is because support for emitting custom modifiers is not included in that version of .NET Standard.

The new C# in keyword gets emitted as a custom modifier (modreq), so if DynamicProxy can't emit that modreq, then the generated proxy method signature won't match your interface's method signature, thus the error under .NET Core (which uses the .NET Standard 1.3 build of DynamicProxy).

One solution might be to have DynamicProxy support a new framework target, e.g. .NET Standard 2.0, which quite possibly supports emitting custom modifiers (but I haven't checked at this point).

[TAGC]

I've found that Castle.Core fails to create a proxy for interfaces like this:

public interface IGenericStructByRefConsumer<T>
{
    T Consume(in Struct message);
}

These tests fail under both .NET Framework 4.6.1 and .NET Core 2.0:

[Fact]
internal void Struct_ByRef_Generic_Moq_Test()
{
    _ = Mock.Of<IGenericStructByRefConsumer<int>>();
}

[Fact]
internal void Struct_ByRef_Generic_NSubstitute_Test()
{
    _ = Substitute.For<IGenericStructByRefConsumer<int>>();
}

In the case of .NET Core, I get a similar sort of error to the other tests:

Signature of the body and declaration in a method implementation do not match.  Type: 'Castle.Proxies.IGenericStructByRefConsumer`1Proxy'.  Assembly: 'DynamicProxyGenAssembly2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.

Under .NET Framework, I get a stacktrace like this:

Castle.DynamicProxy.ProxyGenerationException : There was an error in static constructor on type Castle.Proxies.IGenericStructByRefConsumer`1Proxy. This is likely a bug in DynamicProxy. Please report it.
---- System.Reflection.TargetInvocationException : Exception has been thrown by the target of an invocation.
-------- System.TypeInitializationException : The type initializer for 'Castle.Proxies.IGenericStructByRefConsumer`1Proxy' threw an exception.
------------ System.MissingMethodException : Method not found: '!0 SomeProject.Tests.IGenericStructByRefConsumer`1.Consume(SomeProject.Tests.Struct ByRef)'.
   at Castle.DynamicProxy.Internal.TypeUtil.SetStaticField(Type type, String fieldName, BindingFlags additionalFlags, Object value)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.InitializeStaticFields(Type builtType)
   at Castle.DynamicProxy.Generators.InterfaceProxyWithoutTargetGenerator.GenerateType(String typeName, Type proxyTargetType, Type[] interfaces, INamingScope namingScope)
   at Castle.DynamicProxy.Generators.InterfaceProxyWithTargetGenerator.<>c__DisplayClass6_0.<GenerateCode>b__0(String n, INamingScope s)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(CacheKey cacheKey, Func`3 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.CreateInterfaceProxyTypeWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
   at Castle.DynamicProxy.ProxyGenerator.CreateInterfaceProxyWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, IInterceptor[] interceptors)

[stakx]

@TAGC: .NET 4.6.1 probably prefers the .NET Standard 1.3 build to the .NET Framework builds (a problem that has cropped up since .NET 4.6.1 was retroactively changed to support .NET standard 2.0), explaining why it is affected by this as well.

[TAGC]

How hard would it be to fix this? I'd need to manually mock all my interfaces that use in <valuetype> methods otherwise, and I guess now that C# 7.2 is out a bunch of other people will be running into similar problems.

[jonorossi]

@TAGC I've renamed the issue title, no mocking happens here that is a mocking library concern. Thanks for the issue report, I'd usually start by asking for a test case against this library not another layer up your stack, but in this case it'll be easy to make that step.

Thanks @stakx, yes it definitely sounds like this is caused by custom modifiers not emitted when using the .NET Standard 1.3 build, I disliked writing that code during the DNX port.

It looks like ParameterInfo.GetRequiredCustomModifiers (and friends) are available in .NET Standard 2.0, 1.6 and 1.5, and the coreclr has code for it.

I guess its time to also add a .NET Standard 2.0 build. However I don't think that will fix Moq (.NET Standard 1.3), NSubstitute (.NET Standard 1.3) or FakeItEasy (.NET Standard 1.6) without each project also releasing a .NET Standard 2.0 build which they do not do today, as NuGet will continue to use the .NET Standard 1.3 build. I don't think we really have any other options.

/cc @dtchepak @blairconrad @thomaslevesque

[stakx]

@jonorossi: Thanks for the hint regarding Moq. I'll take care of upgrading Moq once an updated Castle.Core package is available. (I think I'll skip over .NET Standard 1.5 and 1.6 as framework targets and go straight to .NET Standard 2.0.)

[thomaslevesque]

I guess its time to also add a .NET Standard 2.0 build

Why 2.0? If the necessary APIs are supported in 1.5, why not target the lowest possible version to support the largest number of consumers? Is there a compelling reason to target 2.0?

.NET 4.6.1 probably prefers the .NET Standard 1.3 build to the .NET Framework builds (a problem that has cropped up since .NET 4.6.1 was retroactively changed to support .NET standard 2.0), explaining why it is affected by this as well.

I don't think that's the reason; I just tried it, making sure to reference the net45 version of Castle.Core, and the problem with the generic interface still occurs. I think this is a related but different bug.

[zvirja]

guess its time to also add a .NET Standard 2.0 build

The same question from the NSubstitute side. If you can fix the issue in .NET Standard 1.5, it worth it to do that. The Castle.Core is the fundamental library and other libraries are based on it. If you don't select the minimal required version, you force all the downstream dependencies to be also unavailable for the earlier versions of .NET.

Probably that isn't so critical for the testing libraries like Moq or NSubstitute, however there might be other dependencies which offer the production functionality and it might hurt them.

[jonorossi]

Why 2.0? If the necessary APIs are supported in 1.5, why not target the lowest possible version to support the largest number of consumers? Is there a compelling reason to target 2.0?

Thanks @thomaslevesque and @zvirja for the feedback, exactly why I CCed. I've just been seeing a lot of other libraries skip over or drop .NET Standard 1.x, I thought it was just the done thing now with .NET Standard 2.0 out (but obviously not). Also on the 2.0 side gives us full access to the .NET Standard 2.0 APIs, but I guess we aren't ready to let go of .NET Standard 1.x yet so there isn't much point. No problem, .NET Standard 1.5 it will be and we'll drop .NET Standard 1.3 in the next major version (that change only drops .NET Framework 4.6 in favour of 4.6.1).

I don't think that's the reason; I just tried it, making sure to reference the net45 version of Castle.Core, and the problem with the generic interface still occurs. I think this is a related but different bug.

Thanks for the heads up, there might be a defect relating just to generics.

We need to get a full set of unit tests added for these cases. If someone wants to contribute them please do, just let everyone know, I won't have time to look at it tonight.

[thomaslevesque]

.NET Standard 1.5 it will be and we'll drop .NET Standard 1.3 in the next major version (that change only drops .NET Framework 4.6 in favour of 4.6.1).

👍
But if it's not too much trouble, it might be a good idea to keep 1.3 around (even if it doesn't support in parameters) in addition to 1.5, because Moq and NSubstitute still target it (not an issue for FakeItEasy, since we target 1.6)

[TAGC]

@thomaslevesque Both the Moq and NSubstitute guys have commented above that they're prepared to upgrade to .NET Standard 1.5+ when Castle.Core vNext gets released, so I guess it's not necessary to keep 1.3 around for their sake (although it might be for other, non-mocking related libraries).

[TAGC]

Hi, any update on how this is progressing?

[jonorossi]

Hi, any update on how this is progressing?

@TAGC I'm not aware of anyone working on a pull request to add a .NET Standard 1.5 build defining FEATURE_EMIT_CUSTOMMODIFIERS, and/or investigating if there is an additional defect you were talking about in #339 (comment).

[stakx]

I had some free time today, so I took a stab at this and submitted a PR for part of this issue. I'm however not quite sure how to resolve the issues that popped up with the Travis CI build.

and/or investigating if there is an additional defect you were talking about in #339 (comment)

There indeed seems to be a second defect here. Apparently, when DynamicProxy caches a method token in a generated proxy class' type initializer (.cctor), it doesn't take into account the in modifier modreq.

I'll try to take a look at that, as well, but I'll probably submit a separate PR if I find a solution here. For now, suffice it to say that #344 won't be quite enough to close this present issue.

Update: This is likely a problem in the framework, not in DynamicProxy. See the dotnet/corefx issue linked to below.

[jonorossi]

Many thanks @stakx, great work!

I've responded on your pull request to get things going on .NET Core, I guess we'll wait to hopefully get a response from someone more versed in the CLR implementation as the work week starts.

[TAGC]

Thanks @stakx, I appreciate it!

[jonorossi]

I've merged #344 which gets things going, and is enough to ship.

I think it would be a great idea to add a failing unit test (marked ignored referencing the corefx issue) for the defect reported in https://github.com/dotnet/corefx/issues/29254 before we close out this defect.

[zvirja]

I feel that I started to be a bit confused, as previously you said that issue is at .NET Runtime side 😕 Given that you merged the PR and tests passed, does that mean that no issue is present there?

If issue is still present at .NET Runtime side, could you clarify which scenarios work and which ones are affected?

Thanks. Excuse me if I'm missing something obvious.

[jonorossi]

I feel that I started to be a bit confused, as previously you said that issue is at .NET Runtime side 😕 Given that you merged the PR and tests passed, does that mean that no issue is present there?

If issue is still present at .NET Runtime side, could you clarify which scenarios work and which ones are affected?

@zvirja this issue isn't yet closed which is why my last comment is requesting we add ignored failing unit tests for the .NET runtime defect so it is clear what we expect not to work. in parameters didn't work at all on .NET Core because the .NET Standard 1.3 API doesn't expose the required reflection methods for us to emit custom modifiers, which is why we've added a .NET Standard 1.5 target which does expose these reflection methods, those methods have always been used on the .NET Framework.

You'll see @TAGC's #339 (comment) and https://github.com/dotnet/corefx/issues/29254 both have interfaces with a generic argument, the remaining defect is with generic types, however there might be other scenarios which we haven't yet discovered.

[stakx]

@jonorossi, I'll submit a PR with the skipped failing unit test.

[jonorossi]

I'll submit a PR with the skipped failing unit test.

Thanks merged.

@zvirja see #353, if either the type or method has a generic argument it will fail, didn't realise it also broke with a method having a generic argument.

[jonorossi]

I'm going to close this issue as fixed. The corefx issue is assigned to .NET Core 2.2.0, I can't see any way we could work around this defect, so subscribe to that issue to track the fix there. If the Microsoft team post a workaround we can look at doing something our side.

[zvirja]

@jonorossi Thanks for the explanation. Do you have also any ETA the already merged fix is released in a new version of the Castle.Core library?

[jonorossi]

@zvirja I just created #354 for the next release.