[C#] Thread Safe Dictionary .NET 2.0

[xcguy87]

Will this code make the dictionary thread-safe?

Each function hangs until the locked boolean clears (a write operation finishes).

Will it work? Thanks.

 public class SafeDictionary<TKey, TValue>
	{
		private bool						_bLocked = false;
		private Dictionary<TKey, TValue>	_dDictionary;

		public SafeDictionary()
		{
			_dDictionary = new Dictionary<TKey, TValue>();
		}

		private void WaitForUnlock()
		{
			while (_bLocked);
		}

		private void WaitForUnlock(int timeout)
		{
			while (_bLocked)
				Thread.Sleep(timeout);
		}

		public void Lock()
		{
			_bLocked = true;
		}

		public void Unlock()
		{
			_bLocked = false;
		}

		public void Add(TKey key, TValue value)
		{
			WaitForUnlock();

			Lock();
			_dDictionary.Add(key, value);
			Unlock();
		}

		public virtual ICollection<TValue> Values
		{
			get
			{
				WaitForUnlock();

				return _dDictionary.Values;
			}
		}

		public virtual ICollection<TKey> Keys
		{
			get
			{
				WaitForUnlock();

				return _dDictionary.Keys;
			}
		}

		public int Count
		{
			get
			{
				WaitForUnlock();

				return _dDictionary.Count;
			}
		}

		public void Clear()
		{
			WaitForUnlock();

			Lock();
			_dDictionary.Clear();
			Unlock();
		}

		public void Remove(TKey key)
		{
			WaitForUnlock();

			Lock();
			_dDictionary.Remove(key);
			Unlock();
		}

		public virtual TValue this[TKey key]
		{
			get
			{
				WaitForUnlock();

				return _dDictionary[key];
			}
			set
			{
				WaitForUnlock();

				Lock();
				_dDictionary[key] = value;
				Unlock();
			}
		}

		public virtual bool ContainsKey(TKey key)
		{
			WaitForUnlock();

			return _dDictionary.ContainsKey(key);
		}


		public virtual bool TryGetValue(TKey key, out TValue value)
		{
			WaitForUnlock();

			return _dDictionary.TryGetValue(key, out value);
		}
	}

[Andre S. Veteran]

Why not use .NET's built-in lock mechanism? In C#, that is lock(object). Doing something like

while (locked);

keeps the CPU busy for nothing, while

lock(privateVariable)

achieves the same effect while not using any unnecessary CPU cycles.

See http://msdn.microsoft.com/en-us/library/ms173179.aspx if you don't know what I'm talking about.

[Eric Veteran]

Just use Dr_Asik's method. It's the quickest (and makes it easy to see what you're doing)

...
lock(dictionaryClass.SyncRoot)
{
   // access dictionaryClass
}

[xcguy87]

  Dr_Asik said:

Why not use .NET's built-in lock mechanism? In C#, that is lock(object). Doing something like

while (locked);

keeps the CPU busy for nothing, while

lock(privateVariable)

achieves the same effect while not using any unnecessary CPU cycles.

See http://msdn.microsoft.com/en-us/library/ms173179.aspx if you don't know what I'm talking about.

Thanks for the replies...i started implementing the lock already. Do you have to lock for both read and write access or just when modifying the collection?

Thanks again.

[Andre S. Veteran]

Several threads can read at the same time, but only one can write. So you don't need to lock for read access.

Not sure what you mean by "implementing the lock"... it is a built-in function in C#. You just need to provide it with a private class member, typically just an object.

[antareus]

Consider using a private Object instance to serve as your lock instead of SyncRoot. The problem with SyncRoot is that anybody else could be holding that lock in an irresponsible fashion.

More discussion is available at: http://blogs.msdn.com/brada/archive/2003/09/28/50391.aspx

[Eric Veteran]

  antareus said:

Consider using a private Object instance to serve as your lock instead of SyncRoot. The problem with SyncRoot is that anybody else could be holding that lock in an irresponsible fashion.

More discussion is available at: http://blogs.msdn.com/brada/archive/2003/09/28/50391.aspx

That's from 2003. :) At the bottom it reads "Rest assured we will not make the same mistake as we build the generic versions of these collections."

[amrinders87]

  GreyWolfSC said:

That's from 2003. :) At the bottom it reads "Rest assured we will not make the same mistake as we build the generic versions of these collections."

Well by the mistake he means that they will try to deprecate the SyncRoot propery, which is the case now. The ICollection<T> interface does not contain SynRoot property and "lock(dictionaryClass.SyncRoot)" will not compile. You can cast the dictionaryClass to ICollection and then get SynRoot since the old non-genric ICollection interface has SyncRoot property, but the Dictionary<TKey, TValue> class implements it explicitly.

So as antareus said, use a private readonly object for locking.