castleproject / Core

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

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. #654

Open jakubs79 opened 1 year ago

jakubs79 commented 1 year ago

Hi, I think there is a bug in DictionaryAdapper - particulary in DictionaryAdapterInstance class (or ComponentAttribute class depending on approach :)).

Extended properties is instantianted as Dictionary wich is not thread safe.

public IDictionary ExtendedProperties
        {
            get
            {
                if (extendedProperties == null)
                {
                    extendedProperties = new Dictionary<object, object>();
                }
                return extendedProperties;
            }
        }

My interface (some code omitted for clarity) is defined as

public interface IAppSettings
 {

        [Component(Prefix = "AppPath:")]
        IAppPath AppPath { get; }
}

and is initialized on startup and used as singleton

In ComponentAttribute class there is such code:

if (component == null)
                {
                    var descriptor = new PropertyDescriptor(property.Property, null);
                    descriptor.AddBehavior(new KeyPrefixAttribute(key));
                    component = dictionaryAdapter.This.Factory.GetAdapter(
                        property.Property.PropertyType, dictionaryAdapter.This.Dictionary, descriptor);
                    dictionaryAdapter.This.ExtendedProperties[property.PropertyName] = component;
                }

which writes to Dictionary and it causes occasionally a exception in multihread environment (web page in my case). Stack trase from my app:

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
   at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
   at System.Collections.Generic.Dictionary`2.System.Collections.IDictionary.get_Item(Object key)
   at Castle.Components.DictionaryAdapter.ComponentAttribute.Castle.Components.DictionaryAdapter.IDictionaryPropertyGetter.GetPropertyValue(IDictionaryAdapter dictionaryAdapter, String key, Object storedValue, PropertyDescriptor property, Boolean ifExists)
   at Castle.Components.DictionaryAdapter.PropertyDescriptor.GetPropertyValue(IDictionaryAdapter dictionaryAdapter, String key, Object storedValue, PropertyDescriptor descriptor, Boolean ifExists)
   at Castle.Components.DictionaryAdapter.DictionaryAdapterBase.GetProperty(String propertyName, Boolean ifExists)
   at CastleDictionaryAdapterType.get_AppPath()

The code should be fixed to be thread safe in that place.

stakx commented 1 year ago

Hi @jakubs79, I'm not overly familiar with DictionaryAdapter (I've never used it myself) and I'm not even 100 % sure whether we're still maintaining that thing (see #394 about that), but just in case we do: Could you please add a minimal but complete code example that reproduces this exception? Thanks.