xcguy87 Posted July 2, 2009 Share Posted July 2, 2009 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); } } Link to comment Share on other sites More sharing options...
0 Andre S. Veteran Posted July 2, 2009 Veteran Share Posted July 2, 2009 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. Link to comment Share on other sites More sharing options...
0 Eric Veteran Posted July 3, 2009 Veteran Share Posted July 3, 2009 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 } Link to comment Share on other sites More sharing options...
0 xcguy87 Posted July 6, 2009 Author Share Posted July 6, 2009 Why not use .NET's built-in lock mechanism? In C#, that is lock(object). Doing something likewhile (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. Link to comment Share on other sites More sharing options...
0 Andre S. Veteran Posted July 6, 2009 Veteran Share Posted July 6, 2009 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. Link to comment Share on other sites More sharing options...
0 antareus Posted July 6, 2009 Share Posted July 6, 2009 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 Link to comment Share on other sites More sharing options...
0 Eric Veteran Posted July 12, 2009 Veteran Share Posted July 12, 2009 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." Link to comment Share on other sites More sharing options...
0 amrinders87 Posted July 15, 2009 Share Posted July 15, 2009 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. Link to comment Share on other sites More sharing options...
Question
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); } }Link to comment
Share on other sites
7 answers to this question
Recommended Posts