CODEWITHSUNDEEP
.netmultithreadingdictionary
mark
mark

Reputation: 62712

Accessing .NET dictionary in a multithreaded environment

I would like to emply the if-lock-if pattern for checking if an object is present in the dictionary in a multithreaded environment. So, the code I am considering looks like so:

private IDictionary<string, SomeType> m_dic = new Dictionary<string, SomeType>();

private SomeType GetSomeObject(string key)
{
  SomeType obj;
  if (!m_dic.TryGetValue(key, out obj))
  {
    lock(m_dic)
    {
      if (!m_dic.TryGetValue(key, out obj))
      {
        m_dic[key] = obj = CreateSomeObject(key);
      }
    }
  }
  return obj;
}

I act on the assumption that even if another thread is inserting the object at the same key right now, the TryGetValue will not return a partially set reference (such thing does not exist in .NET, does it?), rather it will return null and so we enter the protected section and repeat the check.

My question is my assumption correct and the code is right?

Thanks.

EDIT

Let me throw in a restriction. The dictionary is actually a dictionary of singleton objects. So, once an entry is occupied, it is never changed. Just like the Instance property of a singleton - once it is set, it is never changed. Given that constraint, can we use the if-lock-if pattern?

Upvotes: 1

Views: 2636

Answers (5)

Alex Yakunin
Alex Yakunin

Reputation: 6658

See my comments to the correct reply. Important part is: if you replace Dictionary to Hashtable in your example, this approach will work.

Upvotes: 2

Henk Holterman
Henk Holterman

Reputation: 273169

Edited for clarity:

No this is a very bad idea. You can play an if-lock game on something simple and atomic (an int) but a Dictionary is a class with multiple moving parts. Reading and Writing must be synchronized at all times, See the ThreadSafety section on this MSDN page.

Upvotes: 5

m3kh
m3kh

Reputation: 7941

I think ReaderWriterLock is better than approach in this senario. furthermore, you have multiple readers, but only one writer.

http://msdn.microsoft.com/en-us/library/system.threading.readerwriterlock.aspx

Upvotes: 0

Ian Kemp
Ian Kemp

Reputation: 29839

From http://www.yoda.arachsys.com/csharp/singleton.html:

Locking on objects which other classes can access and lock on (such as the type) risks performance issues and even deadlocks. This is a general style preference of mine - wherever possible, only lock on objects specifically created for the purpose of locking, or which document that they are to be locked on for specific purposes (e.g. for waiting/pulsing a queue). Usually such objects should be private to the class they are used in. This helps to make writing thread-safe applications significantly easier.

Furthermore, double-checked locking is almost always a bad idea. Hence, I would use code similar to the following:

private static readonly object m_padlock = new object();
private IDictionary<string, SomeType> m_dic = new Dictionary<string, SomeType>();

private SomeType GetSomeObject(string key)
{
  SomeType obj;

  lock (m_padlock)
  {
    if (!m_dic.TryGetValue(key, out obj))
    {
      m_dic[key] = obj = CreateSomeObject(key);
    }
  }

  return obj;
}

Upvotes: 3

Vitaliy Liptchinsky
Vitaliy Liptchinsky

Reputation: 5299

In .NET it is not possible to get partially set reference. All read/write reference operations are atomic.

Code looks fine :) But don't forget to add synchronization to the other operations, like INSERT and UPDATE.

Upvotes: 0

Related Questions