davidwhitney / System.Configuration.Abstractions

Injectable, mockable, extensible, configuration for .NET
MIT License
42 stars 7 forks source link

Conversion to enum seems to fail after each redeploy + JITer warmup #13

Closed roncanepa closed 6 years ago

roncanepa commented 7 years ago

Hello, I'm using custom converters to map from a string value in the config to an enum. For the most part, everything has been working great, but I noticed via logs that every once in a while, I'd get an InvalidCastException. This is in a *.ashx web handler acting as a web service (old code base), so I'd hit it once, it'd throw the exception, I'd hit it again right away without changes, and it'd work perfectly.

I've narrowed this down: the first time I deploy the webservice and hit it, the JITer has to spin everything up, and I get a delay and then a InvalidCastException on my enum. If I immediately send another request via Postman, it works perfectly and immediately. I've hammered the webservice and the only time it fails is the first time after a deploy.

Is there some kind of timeout used by the library or you can you think of any reasons / fixes for this behavior? In case it's relevant, code snippets follow. Thank you.

    public enum OurPlatforms
    {
        Dev,
        Production
    }
    public class OurPlatformEnumConverter : IConvertType
    {
        public object Convert(string configurationValue)
        {
            if (configurationValue == OurPlatforms.Dev.ToString())
            {
                return OurPlatforms.Dev;
            }
            else if (configurationValue == OurPlatforms.Production.ToString())
            {
                return OurPlatforms.Production;
            }
        }
        public Type TargetType => typeof(OurPlatforms);

I'm using them in a config factory, which gets wired up like this in the factory constructor:

        public ProcessorConfigFactory()
        {
            var ourPlatformEnumConverter = new OurPlatformEnumConverter();
            ConfigurationManager.RegisterTypeConverters(ourPlatformEnumConverter);
        }

and then it reads out the config value that should map out to an enum when a Get method is called:

        private OurPlatforms GetOurPlatform(IConfigurationManager manager)
        {
            return manager.AppSettings.AppSettingConvert<OurPlatforms>("SomeNamespace.OurPlatform");
        }
roncanepa commented 7 years ago

Update: it works just fine in a webform, as expected, so I'm wondering if there's something to do with the lifecycle of an http handler.

roncanepa commented 7 years ago

I pasted ALL my code into said webform, and I'm experiencing the same behavior of throwing the exception after a deploy but being fine after that. I wonder if it's because of how I'm registering the converters in the constructor of the class that's trying to use them.

Is there a recommended way to call ConfigurationManager.RegisterTypeConverters(converter); such that it's picked up by all instances of the Configuration Manager? I'll experiment with this. I was trying to avoid having a separate "setup" step in each spot that CM will be used, as there isn't a convenient place to wire this up in a webforms project...

davidwhitney commented 7 years ago

This is super interesting - I'll take a look at this in the next day or two.

roncanepa commented 7 years ago

I was just coming back to make another comment. I think it has to do with static versus instance stuff. I've reproduced a minimally working example to highlight the issue. I could very well be doing something silly in the way I'm using things.

https://dl.dropboxusercontent.com/u/34032461/MinimalWorkingExampleConfigAbstraction.zip

If you run the attached solution by loading up default.aspx, it'll automatically break on return manager.AppSettings.AppSettingConvert<Platforms>("SomeNamespace.Platform");

If you look at the following in the debugger:

you'll see that on the first run, TypeConverters holds the custom one, and _typeConverters has the default URI converter. You'll get a "InvalidCast" exception because CM defaults to the PrimitiveConverter due to not finding a match.

If you click "continue" (in VS), let it show you the exception page, and immediately reload default.aspx, it'll break again, and you'll notice that TypeConverters now has 2x of PlatformConverter and _typeConverters has 1x PlatformConverter and 1x UriConverter. If you click continue, everything works as expected.

If you immediately reload the page again, TypeConverters now has 3x PlatformConverter and _typeConverters has 1x UriConverter and 2x PlatformConverter. On each load, it'll add another PlatformConverter.

As I said, I could be doing something inadvisable, but I'm using this in an old legacy webforms app and AFAIK I don't have a convenient place to wire this up "once and only once" like in a startup.cs. And I'm trying to avoid having to have a separate call to wire up the converters before using it in each of the various places.

roncanepa commented 7 years ago

Theories: because the call to ConfigurationManager.RegisterTypeConverters() (via RegisterConverter() in the "factory" class) happens inside a class library, the JITer / whatever doesn't "see" it at first and only loads it when it runs/loads the class library code.

Then there's some static/instance stuff going on such that the first custom converter doesn't get registered the first time around.

(Separate issue is it continuing to stack another one each time, but that's likely related to the static nature.)

I'm pretty sure that other areas where I'm using this don't have an issue the first time around because "factory" class stuff gets called into right in the code-behind of a webform, so it gets picked up and pre-warmed.

I started playing around with adding an instance version of RegisterTypeConverters to see if that would help, but it's too late in the day so I'll pick it up again tomorrow.

Edit: Existing code that calls GetConfig() directly in a codebehind also experiences the same problem.

roncanepa commented 7 years ago

It looks like throwing the custom converter registration stuff into the global.asax file may be the way I need to go on this.

davidwhitney commented 6 years ago

Have you got that repro hanging around? Dropbox link has expired...

davidwhitney commented 6 years ago

A few idle thoughts - the correct pattern of usage here is that this stuff should all be configured on application startup in a global.asmx which deals with the static configuration of the library - then each subsequent instance of the configuration manager that is constructed it copies a snapshot of the currently configured Converters into the private _typeConverters list for use with that instance of the ConfigurationManager.

The only way I can see the thing you're demo-ing up there happening is if the ConfigurationManager instance is created before those static registrations happen.

davidwhitney commented 6 years ago

While kind of tangential - I think I'm just going to bake Enum conversion into the library - it's two lines of code and makes this whole thing go away :)

davidwhitney commented 6 years ago

Version 2.2.0.2.42 adds Enum support by default, which should stop you needing to worry about any of this :)

roncanepa commented 6 years ago

Sorry I dropped off and was unavailable the rest of the weekend. I appreciate you adding enum support and will try it out this week. Thank you!