Proxies using Records with a base class broken using .NET 6 compiler

[ajcvickers]

See original issue here: dotnet/efcore#26602

Attempt is made to proxy this:

public record MyRecord : MyBaseRecord
{
    public int MyInt { get; init; }
}

public record MyBaseRecord
{
}

Comment from Jared Parsons on the C# compiler team:

The C# compiler changed the emit strategy for records in .NET 6 to take advantage of covariant returns. The Clone method now always has a return type that matches the containing type. Covariant returns were added to the runtime and language in .NET 5 but records didn't take advantage of them (just ran out of time). In .NET 6 though we finished off that feature and added it to records.

Full repro: https://github.com/Jejuni/Net6RecordEfCoreProxies

Stack trace:

Unhandled exception. System.TypeLoadException: Return type in method 'Castle.Proxies.MyRecordProxy.<Clone>$()' on type 'Castle.Proxies.MyRecordProxy' from assembly 'DynamicProxyGenAssembly2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' is not compatible with base type method 'Net6RecordEfCoreProxies.M
yRecord.<Clone>$()'.
   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.ClassProxyGenerator.GenerateType(String name, Type[] interfaces, INamingScope namingScope)
   at Castle.DynamicProxy.Generators.ClassProxyGenerator.<>c__DisplayClass1_0.<GenerateCode>b__0(String n, INamingScope s)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.<>c__DisplayClass33_0.<ObtainProxyType>b__0(CacheKey _)
   at Castle.Core.Internal.SynchronizedDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(CacheKey cacheKey, Func`3 factory)
   at Castle.DynamicProxy.Generators.ClassProxyGenerator.GenerateCode(Type[] interfaces, ProxyGenerationOptions options)
   at Castle.DynamicProxy.DefaultProxyBuilder.CreateClassProxyType(Type classToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Proxies.Internal.ProxyFactory.CreateProxyType(ProxiesOptionsExtension options, IEntityType entityType)
   at Microsoft.EntityFrameworkCore.Proxies.Internal.ProxyBindingRewriter.ProcessModelFinalizing(IConventionModelBuilder modelBuilder, IConventionContext`1 context)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.ImmediateConventionScope.OnModelFinalizing(IConventionModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.ConventionDispatcher.OnModelFinalizing(IConventionModelBuilder modelBuilder)
   at Microsoft.EntityFrameworkCore.Metadata.Internal.Model.FinalizeModel()
   at Microsoft.EntityFrameworkCore.ModelBuilder.FinalizeModel()
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, ModelDependencies modelDependencies)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, IConventionSetBuilder conventionSetBuilder, ModelDependencies modelDependencies)
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
   at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__7_3(IServiceProvider p)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite factoryCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScopeCache(ServiceCallSite singletonCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite constructorCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScopeCache(ServiceCallSite singletonCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.Resolve(ServiceCallSite callSite, ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine.<>c__DisplayClass1_0.<RealizeService>b__0(ServiceProviderEngineScope scope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngine.GetService(Type serviceType, ServiceProviderEngineScope serviceProviderEngineScope)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.GetService(Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
   at Microsoft.EntityFrameworkCore.DbContext.get_DbContextDependencies()
   at Microsoft.EntityFrameworkCore.DbContext.get_InternalServiceProvider()
   at Microsoft.EntityFrameworkCore.DbContext.Microsoft.EntityFrameworkCore.Infrastructure.IInfrastructure<System.IServiceProvider>.get_Instance()
   at Microsoft.EntityFrameworkCore.Infrastructure.Internal.InfrastructureExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Infrastructure.AccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.get_Dependencies()
   at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.EnsureDeleted()
   at Program.<Main>$(String[] args) in C:\local\code\repros\Net6RecordEfCoreProxies-master\Net6RecordEfCoreProxies-master\Net6RecordEfCoreProxies\Program.cs:line 6

[ajcvickers]

/cc @jaredpar @jcouv @Jejuni

[Jejuni]

This will most likely affect proxying any hierarchy that uses covariant returns.
It's the most apparent with records since they use covariant returns out of the box with the Clone method when you derive from one.

[jcouv]

@ajcvickers I see that multiple issues have been file in different repos. What is the purpose of having a roslyn issue? How did roslyn misbehave (expected versus actual)?

[ajcvickers]

@stakx Any chance this can get looked at soon? It's blocking people using dynamic proxies for lazy-load in EF Core on .NET 6.

[jonorossi]

@ajcvickers Am I missing something, are you discussing this with Dom via private channels? I can't find any discussion in the linked issues across various projects. I had ignored this issue as you directly mentioned him.

[ajcvickers]

@jonorossi No, I just pinged him because he seemed to be most active recently on the project.

[qwertie]

Note that this is an issue involving covariant return types rather than .NET 6 per se. I got this error yesterday in .NET 5 after defining a custom Clone() method with covariant return type.

[ajcvickers]

Note: we were able to patch EF with a workaround for the common case (by excluding the method), but this doesn't work for abstract record base types, so this fix is still important for EF. See dotnet/efcore#27491

[jonorossi]

Unassigning from v5.0 milestone. No one was contributed or is discussing a possible fix.

[ian-g-holm-intel]

So am I correct that this prevents making proxies of any record that inherits from an abstract base record?

[jonorossi]

So am I correct that this prevents making proxies of any record that inherits from an abstract base record?

I don't think this just impacts abstract records. No one has put together a set of tests using just DP, so I'm not across the full impacts.

[CesarD]

Bump? 🤔
It's impacting Moq as well, since they also consume Castle.
Can't mock stuff that inherits from abstract records as well.

[stakx]

@CesarD, this hasn't been forgotten. I'd like to fix a few things in DynamicProxy (this issue included, i.e. support for covariant return types; if time permits), but have been awaiting the now-imminent DynamicProxy v5 release. I'm planning to start working on open issues in about 2-3 weeks' time.

[stakx]

I've taken a first brief look. The problem appears to be that MethodFinder.GetAllInstanceMethods reports the <Clone>$ method twice on .NET 6+ (one from the base record class, and one from the derived record class), vs. just once on earlier runtimes. If we're lucky, all we need to do to get covariant returns working is to account for them in MethodSignatureComparer.Equals, and make sure that the method with the more derived return type gets proxied.

[stakx]

I've put together a little something that may resolve this. See PR linked above.

I am admittedly a little out of touch with modern C# (as I haven't spent all that much time in .NET world during the past couple of years). So I am open to suggestions for further important covariant method return scenarios not currently covered by the newly added CovariantReturnsTestCase fixture. (More tests in the RecordsTestCase might be a good idea, too, but given the PR's title, those are perhaps secondary and could also be added in a later, separate PR.)

[CesarD]

Any idea when this is going to be released?

[stakx]

All merge requests that make code changes are currently merged, think we could do a minor/patch release at this point @jonorossi?

(I'm working on new interface capabilities support, but that's going to take some more time, so there's no need to wait for it at this time.)

[jonorossi]

@stakx sorry for the silence on this end. Could you create a release issue with a proposed version number.

[stakx]

@ajcvickers @CesarD I've just pushed a release including a bugfix for this to NuGet. Hope this helps!

[ajcvickers]

@stakx Thanks! Much appreciated.

[CesarD]

Thank you so much!! Cheers!!

[CesarD]

One question: should this also work for a generic abstract record?
ie: public abstract record MyBaseRecord<T>

I'm having issues with a case like that, but not really sure so far...

[stakx]

@CesarD, yes, I'd say this should work as well. Are you using T as a return type somewhere?

Please open a new issue for this once you're sure / have some repro code.

[CesarD]

Yes, I am using it as part of the return type of a Command I'm using with MediatR... And when mocking (with Moq) commands inheriting from this base (generic) abstract command, it throws the exception when accessing the mock.Object property. Commands that are deriving from a non generic abstract command work perfectly and they can be mocked without issues.

I'll try to get some repro code for tomorrow.

[CesarD]

Hi @stakx! I've just tried some repro code using Moq, which is my use case, and it turns out to effectively occur with only generic abstract records.
Now, I know you told me to open a new issue, but the exception thrown is the exact same that originated this one.

Here goes the code:

var mock1 = new Mock<MyDerivedRecord>();
var mock2 = new Mock<MyDerivedGenericRecord>();

var a = mock1.Object;
var b = mock2.Object;

public abstract record MyBaseRecord
{
}

public abstract record MyBaseGenericRecord<T>
{
	public T Prop { get; set; }
}

public record MyDerivedRecord : MyBaseRecord
{
}

public record MyDerivedGenericRecord : MyBaseGenericRecord<int>
{
}

Variable a holds the mocked object as expected, but when initializing b, the following is thrown:

System.ArgumentException
  HResult=0x80070057
  Message=Type to mock (MyDerivedGenericRecord) must be an interface, a delegate, or a non-sealed, non-static class.
  Source=Moq
  StackTrace:
   at Moq.CastleProxyFactory.CreateProxy(Type mockType, IInterceptor interceptor, Type[] interfaces, Object[] arguments) in C:\projects\moq4\src\Moq\Interception\CastleProxyFactory.cs:line 66
   at Moq.Mock`1.InitializeInstance() in C:\projects\moq4\src\Moq\Mock`1.cs:line 307
   at Moq.Mock`1.OnGetObject() in C:\projects\moq4\src\Moq\Mock`1.cs:line 326
   at Moq.Mock`1.get_Object() in C:\projects\moq4\src\Moq\Mock`1.cs:line 281
   at Program.<Main>$(String[] args) in C:\Users\Cesar\source\repos\ConsoleApp1\Program.cs:line 11

  This exception was originally thrown at this call stack:
    System.Reflection.Emit.TypeBuilder.CreateTypeNoLock()
    System.Reflection.Emit.TypeBuilder.CreateTypeInfo()
    Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.BuildType()
    Castle.DynamicProxy.Generators.BaseClassProxyGenerator.GenerateType(string, Castle.DynamicProxy.Generators.INamingScope)
    Castle.Core.Internal.SynchronizedDictionary<TKey, TValue>.GetOrAdd(TKey, System.Func<TKey, TValue>)
    Castle.DynamicProxy.Generators.BaseProxyGenerator.GetProxyType()
    Castle.DynamicProxy.ProxyGenerator.CreateClassProxy(System.Type, System.Type[], Castle.DynamicProxy.ProxyGenerationOptions, object[], Castle.DynamicProxy.IInterceptor[])
    Moq.CastleProxyFactory.CreateProxy(System.Type, Moq.IInterceptor, System.Type[], object[]) in CastleProxyFactory.cs

Inner Exception 1:
TypeLoadException: Return type in method 'Castle.Proxies.MyDerivedGenericRecordProxy_2.<Clone>$()' on type 'Castle.Proxies.MyDerivedGenericRecordProxy_2' from assembly 'DynamicProxyGenAssembly2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' is not compatible with base type method 'MyDerivedGenericRecord.<Clone>$()'.

If you still wish, I can open a new issue based on this. Please let me know.
Thanks!

[stakx]

I can open a new issue based on this.

Yes, please open a new issue. Thanks!