castleproject / Core

Castle Core, including Castle DynamicProxy, Logging Services and DictionaryAdapter
http://www.castleproject.org/
Other
2.22k stars 470 forks source link

Null is not a valid constant value for this type while trying to copy cancellation token parameters #617

Closed TimLovellSmith closed 2 years ago

TimLovellSmith commented 2 years ago

[Sorry if this is an obsolete bug, haven't tried to repro with latest yet.]

Packages.config: <package id="Castle.Core" version="4.3.1" targetFramework="net472" />

Observed: managed to get this first-chance exception {"Null is not a valid constant value for this type."} | System.ArgumentException as a first-chance exception.

Expected behavior: this seems like a bit of unnecessary exception-oriented programming where conditional tests, or appropriate method overloading, would be better and faster.

Local variable from refers to the cancellation token parameter in CopyDefaultValueConstant().

{Name = "CancellationToken" FullName = "System.Threading.CancellationToken"}

mscorlib.dll!System.Reflection.Emit.TypeBuilder.SetConstantValue(System.Reflection.Emit.ModuleBuilder module, int tk, System.Type destType, object value) Unknown mscorlib.dll!System.Reflection.Emit.ParameterBuilder.SetConstant(object defaultValue) Unknown Castle.Core.dll!Castle.DynamicProxy.Generators.Emitters.MethodEmitter.CopyDefaultValueConstant(System.Reflection.ParameterInfo from, System.Reflection.Emit.ParameterBuilder to) Unknown Castle.Core.dll!Castle.DynamicProxy.Generators.Emitters.MethodEmitter.DefineParameters(System.Reflection.ParameterInfo[] parameters) Unknown Castle.Core.dll!Castle.DynamicProxy.Generators.Emitters.MethodEmitter.MethodEmitter(Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter owner, string name, System.Reflection.MethodAttributes attributes, System.Reflection.MethodInfo methodToUseAsATemplate) Unknown Castle.Core.dll!Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.CreateMethod(string name, System.Reflection.MethodAttributes attributes, System.Reflection.MethodInfo methodToUseAsATemplate) Unknown Castle.Core.dll!Castle.DynamicProxy.Generators.MethodGenerator.Generate(Castle.DynamicProxy.Generators.Emitters.ClassEmitter class, Castle.DynamicProxy.ProxyGenerationOptions options, Castle.DynamicProxy.Generators.INamingScope namingScope) Unknown Castle.Core.dll!Castle.DynamicProxy.Contributors.CompositeTypeContributor.ImplementMethod(Castle.DynamicProxy.Generators.MetaMethod method, Castle.DynamicProxy.Generators.Emitters.ClassEmitter class, Castle.DynamicProxy.ProxyGenerationOptions options, Castle.DynamicProxy.Contributors.OverrideMethodDelegate overrideMethod) Unknown Castle.Core.dll!Castle.DynamicProxy.Contributors.CompositeTypeContributor.Generate(Castle.DynamicProxy.Generators.Emitters.ClassEmitter class, Castle.DynamicProxy.ProxyGenerationOptions options) Unknown Castle.Core.dll!Castle.DynamicProxy.Generators.ClassProxyGenerator.GenerateType(string name, System.Type[] interfaces, Castle.DynamicProxy.Generators.INamingScope namingScope) Unknown Castle.Core.dll!Castle.DynamicProxy.Generators.ClassProxyGenerator.GenerateCode.AnonymousMethod__0(string n, Castle.DynamicProxy.Generators.INamingScope s) Unknown Castle.Core.dll!Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(Castle.DynamicProxy.Generators.CacheKey cacheKey, System.Func<string, Castle.DynamicProxy.Generators.INamingScope, System.Type> factory) Unknown Castle.Core.dll!Castle.DynamicProxy.Generators.ClassProxyGenerator.GenerateCode(System.Type[] interfaces, Castle.DynamicProxy.ProxyGenerationOptions options) Unknown Castle.Core.dll!Castle.DynamicProxy.DefaultProxyBuilder.CreateClassProxyType(System.Type classToProxy, System.Type[] additionalInterfacesToProxy, Castle.DynamicProxy.ProxyGenerationOptions options) Unknown Castle.Core.dll!Castle.DynamicProxy.ProxyGenerator.CreateClassProxy(System.Type classToProxy, System.Type[] additionalInterfacesToProxy, Castle.DynamicProxy.ProxyGenerationOptions options, object[] constructorArguments, Castle.DynamicProxy.IInterceptor[] interceptors) Unknown FakeItEasy.dll!FakeItEasy.Creation.CastleDynamicProxy.CastleDynamicProxyGenerator.GenerateInterfaceProxy(System.Type typeOfProxy, System.Collections.ObjectModel.ReadOnlyCollection additionalInterfacesToImplement, System.Collections.Generic.IEnumerable<System.Linq.Expressions.Expression<System.Func>> attributes, FakeItEasy.Core.IFakeCallProcessorProvider fakeCallProcessorProvider) Line 43 C# FakeItEasy.dll!FakeItEasy.Creation.FakeObjectCreator.DefaultCreationStrategy.CreateFakeInterface(System.Type typeOfFake, FakeItEasy.Creation.IProxyOptions proxyOptions) Line 151 C# FakeItEasy.dll!FakeItEasy.Creation.FakeObjectCreator.CreateFakeWithoutLoopDetection(System.Type typeOfFake, FakeItEasy.Creation.IProxyOptions proxyOptions, FakeItEasy.Creation.IDummyValueResolver resolver, FakeItEasy.Creation.LoopDetectingResolutionContext resolutionContext) Line 63 C# FakeItEasy.dll!FakeItEasy.Creation.FakeObjectCreator.CreateFake(System.Type typeOfFake, FakeItEasy.Creation.IProxyOptions proxyOptions, FakeItEasy.Creation.IDummyValueResolver resolver, FakeItEasy.Creation.LoopDetectingResolutionContext resolutionContext) Line 50 C# FakeItEasy.dll!FakeItEasy.Creation.FakeAndDummyManager.CreateFake(System.Type typeOfFake, FakeItEasy.Creation.LoopDetectingResolutionContext resolutionContext) Line 36 C# FakeItEasy.dll!FakeItEasy.A.Fake() Line 32 C#

TimLovellSmith commented 2 years ago

Quick research into CopyDefaultValueConstant makes it look like this might because of this kind of edge case:

https://stackoverflow.com/questions/9977530/default-parameters-and-reflection-if-parameterinfo-isoptional-then-is-defaultva

stakx commented 2 years ago

Could you please provide a minimal code example that reproduces the reported exception? (And yes, please do test it against the latest published version.)

stakx commented 2 years ago

/ping @TimLovellSmith. The information you gave so far is insufficient for us to reproduce the reported problem. Please provide a minimal but complete repro code example; otherwise, I'm going to close this issue in a few days' time.

stakx commented 2 years ago

Closing due to missing repro code & inactivity. @TimLovellSmith, we can reopen once we have something that allows us to reproduce this error.

TimLovellSmith commented 1 year ago

Here, is the kind of code I was trying to fake. I produced it with Castle 4.3.1, looks like it may be fixed since then.

using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;

namespace RedisRP.Handlers
{
    public interface IAuthorizeRequests
    {
        Task<(bool, HttpResponseMessage)> IsAllowedAsync(HttpRequestMessage request, CancellationToken token = default);
    }
}
TimLovellSmith commented 1 year ago

Ha... with decompile to source code, I see I can still 'repro' it, but it wasn't really a bug at all. Unless you are trying to avoid first-chance exceptions.

private void CopyDefaultValueConstant(ParameterInfo from, ParameterBuilder to)
        {
            object obj;
            try
            {
                obj = from.DefaultValue;
            }
            catch (FormatException) when (from.ParameterType == typeof(DateTime))
            {
                obj = null;
            }
            catch (FormatException) when (from.ParameterType.IsEnum)
            {
                obj = null;
            }
            if (obj is Missing)
            {
                return; 
            }
            try
            {
                to.SetConstant(obj); // this line throws
TimLovellSmith commented 1 year ago

So yes, this was always good to close.

stakx commented 1 year ago

Ah, yes. Thanks for letting us know.

In case you're wondering why we need to catch exceptions there, check out the code comments in MethodEmitter.cs. (In short, it is to work around various bugs and limitations in the CLR.)