Is there any issue when accessing a globa/static dictionary using a static class?

Question

I've created a simple validation process for a small project. The validation rules are created as attributes to properties of a class.

I have a static class that requires to pass the class type. The method will first check if the set of rules for the given class is already in the dictionary, otherwise, it will use reflection to go through each property.

Will there be any issues when using this kind of approach? I'm worrying that it may cause some issues when being accessed in multi-threaded environments.

public static class Validator
{
    private static Dictionary<Type, ValidationRulesCollection> _validationRules = new Dictionary<Type, ValidationRulesCollection>();

    public static ValidationRulesCollection GetValidationRules(object obj)
    {
        return GetValidationRules(obj.GetType());
    }

    public static ValidationRulesCollection GetValidationRules(Type t)
    {
        ValidationRulesCollection rules = null;

        /* Check if the centralized rules repository already contains the rules for this class type */
        if (_validationRules.ContainsKey(t) == true)
        {
            rules = _validationRules[t];
        }
        else
        {
            /* Using reflection, get the list of properties for the class type */
            PropertyInfo[] properties = t.GetProperties();

            if (properties != null)
            {
                /* Iterate through each property info and check if it contains a validation rule */
                ValidationAttribute[] attribs = null;
                foreach (PropertyInfo property in properties)
                {
                    /* For each property, check if it contains a validation rule */
                    attribs = (ValidationAttribute[])property.GetCustomAttributes(typeof(ValidationAttribute), true);

                    if (attribs != null)
                    {
                        foreach (ValidationAttribute attrib in attribs)
                        {
                            if (rules == null)
                                rules = new ValidationRulesCollection();

                            /* Add the validation rule to the collection */
                            rules.Add(new ValidationRule(property, attrib));
                        }
                    }
                }
            }

            /* Add the rules collection to the centralized rules repository */
            if (rules.Count > 0)
                _validationRules.Add(t, rules);
            else
                throw new ArgumentNullException("The type " + t.ToString() + " does not have any ValidationAttributes");
        }

        return rules;
    }

    public static ValidationRulesCollection Validate(object obj)
    {
        /* Get the Validation Rules */
        ValidationRulesCollection rules = GetValidationRules(obj);

        /* Validate the rules */
        foreach (ValidationRule rule in rules)
        {
            rule.Validate(obj);
        }

        return rules;
    }
}

See [C# : What if a static method is called from multiple threads?](http://stackoverflow.com/questions/3037637/c-sharp-what-if-a-static-method-is-called-from-multiple-threads) — SwDevMan81, Mar 29 '13 at 18:17

score 3 · Answer 1 · answered Mar 29 '13 at 18:17

3

Dictionary is not thread-safe. And your method GetValidationRules is reading and writing it. If multiple threads call this method, dictionary data may get corrupt. To avoid this, put a lock(...){ ... } that covers all dictionary method calls.

answered Mar 29 '13 at 18:17

alex

12,464
3
46
67

driis · Accepted Answer · 2013-03-29T18:24:18.177

3

This could get you in trouble in a multi threaded environment. Generally, the Dictionary<K,V> class is not thread safe. Beyound data corruption or correctness issues, in rare circumstances, if one thread hits the dictionary while it is being resized on another thread, you could get a situation where the thread gets stuck in a loop, eating 100% CPU. This happened to me in a production web app once - not a fun thing to debug.

You could either put a lock around the method, or use a ConcurrentDictionary<K,V> instead.

Note that we could define "thread safety" in different ways. If you want the whole operation of finding the properties and attributes to be atomic, you should put a lock around the method. But beaware that this would introduce quite a lot of locking in the app, potentially hurting performance. If you are OK with potentially doing the work twice in a race condition, and only need to make sure the dictionary behaves correctly, ConcurrentDictionary is the way to go.

edited Mar 29 '13 at 18:24

answered Mar 29 '13 at 18:18

driis

161,458
45
265
341

Thanks for pointing out the thread-safety issue. If im not mistaken, whenever an object in the dictionary is used by one thread, the 2nd thread will have to wait? Is this correct and if so, is there any significant hit to performance? – codex10 Mar 29 '13 at 18:25
You'll even get (sporadic) errors: you cannot use code like `if (!Contains) Add` without locking: your call to `Add` can throw duplicate key exceptions when right between the `Contains` and the `Add` call on thread 1, thread 2 calls `Add` using the same key. This is also true for `ConcurrentDictionary`. Using the indexer will 'fix' this though: `_validationRules[t] = rules;` because the last thread will silently overwrite any value set by a previous thread. – Ruben Mar 29 '13 at 18:27

Is there any issue when accessing a globa/static dictionary using a static class?

2 Answers2