Sometimes there's an exception thrown on first page request

[heikof]

(placeholder - detail to be added)

[benmcevoy]

I don't have the project running anywhere to test. But bear with me.

Our stack trace looks like:

51612 2017:03:29 03:27:02 ERROR System.NullReferenceException: Object reference not set to an instance of an object.
at DeloitteDigital.Atlas.Mapping.ItemMapper.MapModelProperties[TModel](TModel model, Item item, ModelMeta modelMeta)
at DeloitteDigital.Atlas.Mapping.ItemMapper.Map[TModel](TModel model, Item item)
at DeloitteDigital.Atlas.Mvc.RenderingModel1.Map[T](T to, Item from) at MyProject.Web.Components.KnowIt.Header.HeaderRenderingModel.InitialiseViewModel(Rendering rendering, DataSource dataSource) in E:\agent\_work\7\s\src\MyProject.Web\Components\MyProject\Header\HeaderRenderingModel.cs:line 15 at DeloitteDigital.Atlas.Mvc.RenderingModel1.Initialize(Rendering rendering)

https://github.com/DeloitteDigitalAPAC/Atlas/blob/master/src/DeloitteDigital.Atlas/Mapping/ItemMapper.cs

The exception is thrown in MapModelProperties, seems to be as it tries to use the modelMeta.PropertyMap

When the modelMeta is created I think there is a race.

       private ModelMeta EnsureModelDictionaryEntryExists<TModel>(IDictionary<string, ModelMeta> metadataDictionary)
        {
            var modelType = typeof(TModel);
            var modelMetaKey = modelType.AssemblyQualifiedName;
        
        *** HERE IT IS NULL - the default ***
            var modelMeta = default(ModelMeta);

            *** 1st request this is false
             *** 2nd request this is still false, 1st request is still busy in the lock below
            if (metadataDictionary.ContainsKey(modelMetaKey))
            {
                modelMeta = metadataDictionary[modelMetaKey];
            }
            else
            {
                lock (MetaSyncRoot)
                {
                    *** 1st request this is true
                    *** 2nd request this is now false, as we had to wait for the lock
                    if (!metadataDictionary.ContainsKey(modelMetaKey))
                    {
                        modelMeta = new ModelMeta();
                        var propertyMetaBuilder = new PropertyMetaBuilder();
                        modelMeta.PropertyMap = propertyMetaBuilder.BuildPropertyMetaMap<TModel>();
                        metadataDictionary.Add(modelMetaKey, modelMeta);
                    }
                }
            }
            *** 1st request is OK
            *** 2nd request is null :disappointed:
            return modelMeta;
        }

If we move the declarations closer to where they are used:

       private ModelMeta EnsureModelDictionaryEntryExists<TModel>(IDictionary<string, ModelMeta> metadataDictionary)
        {
        // check first to avoid locking, can you use TryGetValue instead? Avoid having to lookup twice.
        // https://msdn.microsoft.com/en-us/library/bb347013(v=vs.110).aspx
        if (metadataDictionary.ContainsKey(modelMetaKey))
            {
                return metadataDictionary[modelMetaKey];
            }


               lock (MetaSyncRoot)
                {
                    if (!metadataDictionary.ContainsKey(modelMetaKey))
                    {
                var modelType = typeof(TModel);
                var modelMetaKey = modelType.AssemblyQualifiedName;
                        var modelMeta = new ModelMeta();
                        var propertyMetaBuilder = new PropertyMetaBuilder();

                       modelMeta.PropertyMap = propertyMetaBuilder.BuildPropertyMetaMap<TModel>();
                        metadataDictionary.Add(modelMetaKey, modelMeta);

            return modelMeta;
                    }
                }

           return metadataDictionary[modelMetaKey];            
       }

If/When I get time I'll try actually test this and get a PR together

[benmcevoy]

I think I can reproduce this.

In the EnsureModelDictionaryEntryExists method, add a Thread.Sleep(1000) inside the lock.
Open two browsers.
Restart the app pool.
In both browsers make a request for the home page.

I pretty reliably get a null reference exception then.

[benmcevoy]

        private ModelMeta EnsureModelDictionaryEntryExists<TModel>(IDictionary<string, ModelMeta> metadataDictionary)
        {
            var modelType = typeof(TModel);
            var modelMetaKey = modelType.AssemblyQualifiedName;
            var modelMeta = default(ModelMeta);

            if (metadataDictionary.ContainsKey(modelMetaKey))
            {
                modelMeta = metadataDictionary[modelMetaKey];
            }
            else
            {
                lock (MetaSyncRoot)
                {
// here
System.Threading.Thread.Sleep(1000);

                    if (!metadataDictionary.ContainsKey(modelMetaKey))
                    {

[davidnguyen]

You are a living legend @benmcevoy. Let me know if you want to patch it yourself, or send me the fix I can create a PR from my fork :P

[geekrocket]

Fix looks great, thanks @benmcevoy! PR has been merged in and I no longer get the exception.