Proper class definition and usage - thread safe - ASP.net

Question

I wonder how to define a class properly and use it safely. I mean thread safely when thousands of concurrent calls are being made by every website visitor.

I made myself something like below but i wonder is it properly built

public static class csPublicFunctions
{
    private static Dictionary<string, clsUserTitles> dicAuthorities;

    static csPublicFunctions()
    {
        dicAuthorities = new Dictionary<string, clsUserTitles>();
        using (DataTable dtTemp = DbConnection.db_Select_DataTable("select * from myTable"))
        {
            foreach (DataRow drw in dtTemp.Rows)
            {
                clsUserTitles tempCLS = new clsUserTitles();
                tempCLS.irAuthorityLevel = Int32.Parse(drw["Level"].ToString());
                tempCLS.srTitle_tr = drw["Title_tr"].ToString();
                tempCLS.srTitle_en = drw["Title_en"].ToString();
                dicAuthorities.Add(drw["authorityLevel"].ToString(), tempCLS);
            }
        }
    }

    public class clsUserTitles
    {
        private string Title_tr;
        public string srTitle_tr
        {
            get { return Title_tr; }
            set { Title_tr = value; }
        }

        private string Title_en;
        public string srTitle_en
        {
            get { return Title_en; }
            set { Title_en = value; }
        }

        private int AuthorityLevel;
        public int irAuthorityLevel
        {
            get { return AuthorityLevel; }
            set { AuthorityLevel = value; }
        }
    }

    public static clsUserTitles returnUserTitles(string srUserAuthority)
    {
        return dicAuthorities[srUserAuthority];
    }
}

Dictionary will be initialized only 1 time. No add remove update later.

Answer 1

A quick look through your code and it seems to me that your first problem will be the publicly available dictionary dicAuthorities. Dictionaries are not thread safe. Depending on what you want to do with that Dictionary, you'll need to implement something that regulates access to it. See this related question:

Making dictionary access thread-safe?

Answer 2

Dictionary supports thread safe reading. Here is the proof from MSDN:

A Dictionary can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

So, if you are planning to only read data from it, it should work. However, I do not believe that your dictionary is filled only once and won't be modified during your application work. in this case, all other guys in this thread are correct, it is necessary to synchronize access to this dictionary and it is best to use the ConcurrentDictionary object.

Now, I want to say a couple of words about the design itself. If you want to store a shared data between users, use ASP.NET Cache instead which was designed for such purposes.

Answer 3

As the others have said, Dictionary<TKey,TValue> is not inherently thread-safe. However, if your usage scenario is:

1. Fill the dictionary on startup
2. Use that dictionary as lookup while the application is running
3. Never add or remove values after startup

than you should be fine.

However, if you use .net 4.5, I would recommend making #3 explict, by using a ReadOnlyDictionary

So, your implementation might look like this (changed the coding style to more C# friendly)

private static readonly ReadOnlyDictionary<string, UserTitles> authorities;

static PublicFunctions()
{
    Dictionary<string, UserTitles> authoritiesFill = new Dictionary<string, clsUserTitles>();
    using (DataTable dtTemp = DbConnection.db_Select_DataTable("select * from myTable"))
    {
        foreach (DataRow drw in dtTemp.Rows)
        {
            UserTitles userTitle = new UserTitles
            {
              AuthorityLevel = Int32.Parse(drw["Level"].ToString()),
              TitleTurkish = drw["Title_tr"].ToString();
              TitleEnglish = drw["Title_en"].ToString();
            }
            authoritiesFill.Add(drw["authorityLevel"].ToString(), userTitle);
        }
    }
    authorities = new ReadOnlyDictionary<string, UserTitles>(authoritiesFill);
}

I've also added a readonly modifier to the declaration itself, because this way you can be sure that it won't be replaced at runtime by another dictionary.

Answer 4

No you code is not thread safe.

• [EDIT does not apply - set/created inside static constructor] Dictionary (as pointed by System Down answer) is not thread safe while being updated. Dictionary is not read only - hence no way to guarantee that it is not modified over time.
• [EDIT does not apply - set/created inside static constructor] Initialization is not protected by any locks so you end-up with multiple initializations at the same time
• Your entries are mutable - so it is very hard to reason if you get consistent value of each entry
• [EDIT does not apply - only modified in static constructor] Field that holds dictionary not read-only - depending on code you may end-up with inconsistent data if not caching pointer to dictionary itself.

Side note: try to follow coding guidelines for C# and call classes starting with upper case MySpecialClass and have names that reflect purpose of the class (or clearly sample names).

EDIT: most of my points do not apply as the only initialization of the dictionary is inside static constructor. Which makes initialization safe from thread-safety point of view. Note that initialization inside static constructor will happen at non-deterministic moment "before first use". It can lead to unexpected behavior - i.e. when access to DB may use wrong "current" user account.

Answer 5

The answer to your question is no, it's not thread safe. Dictionary is not a thread-safe collection. If you want to use a thread-safe dictionary then use ConcurrentDictionary.

Besides that, it's difficult to say whether your csPublicFunctions is thread-safe or not because it depends on how you handle your database connections inside the call to DbConnection.db_Select_DataTable

Answer 6

There is not thread-safe problem only with public Dictionary. Yes, dictionary filling is thread-safe. But another modification of this dictionary is not thread safe. As was wrote above - ConcurrentDictionary could help.

Another problem that your class clsUserTitles is not thread-safe too. If clsUserTitles is using only for reading you could make each property setter of clsUserTitles private. And initialize these properties from clsUserTitles constructor.