• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 889
  • Last Modified:

Thread Safe: Generic Dictionary

Hi!

I need to make my generic dictionary  thread-safe

Is this enuff for doing that?
private object _lockServolist = new object();
 
        private Dictionary<string, ServoMotor> Servolist
        {
            get 
            {
                lock (_lockServolist)
                {
                    return this._servoList;
                }
            } 
        }

Open in new window

0
AWestEng
Asked:
AWestEng
  • 6
  • 6
  • 5
2 Solutions
 
abelCommented:
This is not a trivial question to answer. What you are showing is a lock during the get operation. This is not necessary, as when you only need read-access there's no need to lock, unless you want read/write access from multiple threads. Here's what MS has to say about Dictionaries, maybe you need not do anything:

A Dictionary<(Of  <(TKey, TValue>)>) 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.
0
 
gregoryyoungCommented:
I can tell you that code is 100% not what you want. Can you put up the code that is calling that code? You generally need locks when you are *using* the dictionary ... all you have done is locked around returning it to the thing that will use it.
0
 
AWestEngAuthor Commented:
I read data from the dictionary  in several diffrent methods, but I do not modify the it when multiple threads are accessing the dictionary .
So then it should be safe?
 I do this in about 10 diffrent methods

ServoMotor servo = this._servoList[name];
thre is one time it can create a problem, I have a remove method thats removes a a servo from the list.
 
can i "safe" this up in any way, can I lock with the object "this" in this methd?
0
[Webinar] Database Backup and Recovery

Does your company store data on premises, off site, in the cloud, or a combination of these? If you answered “yes”, you need a data backup recovery plan that fits each and every platform. Watch now as as Percona teaches us how to build agile data backup recovery plan.

 
abelCommented:
If you do not change the Dictionary while reading from it, you should not need a lock. If you want to change the Dictionary and expect other threads to read at the same time, you will need a lock, but on the whole dictionary. If you do not want to block the other threads while removing elements from the Dictionary, you will have to create your own synchronization scheme for the Dictionary.
0
 
abelCommented:
> you should not need a lock.
means: you do not need a lock.
0
 
AWestEngAuthor Commented:
one general thing, so if i have a class object and modify values in this object it does not help to lock the "hole" object in a get property,  it needs to be protected inside the object it self to get the thread-safe ability
 is that right?
0
 
abelCommented:
since this is not a trivial discussion and not trivial to answer, I think a little example says more then the well-known 1000 words: http://stackoverflow.com/questions/157933/whats-the-best-way-of-implementing-a-thread-safe-dictionary-in-net
0
 
AWestEngAuthor Commented:
One other strange this.
In this property I can't lock all in the set part then then hole application stops
and I get a System.ExecutionEngineException
Thre is one thread that is updating but several threads that are reading, so can I do that then?

        public bool Status_StateMotionStoppingSTorLimSW
        {
            get
            {
                lock (_lockStateMotionStoppingSTorLimSW)
                {
                    return this._stateMotionStoppingSTorLimSW;
                }
            }
            set
            {
                lock (_lockStateMotionStoppingSTorLimSW)
                {
                    this._stateMotionStoppingSTorLimSW = value;
                }
                    if (this._previousStateMotionStoppingSTorLimSW != value)
                    {
                        this._previousStateMotionStoppingSTorLimSW = value;
 
                        if (ServoUpdated != null)
                        {
                            ServoUpdated(this);
                        }
                    }
                    else
                    {
                        this._previousStateMotionStoppingSTorLimSW = value;
                    }
               
            }
        }

Open in new window

0
 
AWestEngAuthor Commented:
abel:

 I'm looking at the page you posted, but one post says that the dictionary should be locked

lock (mySafeDictionary)
{
    if (!mySafeDictionary.ContainsKey(someKey))
    {
        mySafeDictionary.Add(someKey, someValue);
    }
}

but what I can remember locking the object itself is not a good idea?
0
 
abelCommented:
That post apparently contained some inconsistencies and I should've read it through better before posting it here. Here's a more elaborate example that I believe you can just takeover and use for your own: http://devplanet.com/blogs/brianr/archive/2008/09/26/thread-safe-dictionary-in-net.aspx
0
 
gregoryyoungCommented:
For your latter code of

        public bool Status_StateMotionStoppingSTorLimSW
        {
            get
            {
                lock (_lockStateMotionStoppingSTorLimSW)
                {
                    return this._stateMotionStoppingSTorLimSW;
                }
            }
            set
            {
                lock (_lockStateMotionStoppingSTorLimSW)
                {
                    this._stateMotionStoppingSTorLimSW = value;
                }
                    if (this._previousStateMotionStoppingSTorLimSW != value)
                    {
                        this._previousStateMotionStoppingSTorLimSW = value;
 
                        if (ServoUpdated != null)
                        {
                            ServoUpdated(this);
                        }
                    }


You are locking on a boolean ... a boolean is a value object... As such it will be boxed and you will be locking on the boxed version (which will be different in both calls...) Create a separate object private object stateLockObject = new object(); and lock on that in both places instead.


0
 
gregoryyoungCommented:
How often are you changing items in the dictionary? is performance a concern?


The easiest way to handle this would be to stop returning the dictionary (generally not considered good practice anyways) and instead put methods on your object for the find and remove. Then just lock in both the find and the remove and you are done.

Greg
0
 
AWestEngAuthor Commented:
no i'm lockihn on a object
 _lockStateMotionStoppingSTorLimSW is a object not a bool
0
 
gregoryyoungCommented:
sorry misread with all of the similar variable names ... I also didn't see ExecutionEngineException ... this is when the CLR runs into an internal problem http://msdn.microsoft.com/en-us/library/system.executionengineexception.aspx

this is probably not only caused by your bad locking code ... are you using unmanaged code as well (or integrating with an unmanaged dll?)
0
 
AWestEngAuthor Commented:
your right there, the ExecutionEngineException  does not seem to have something to do with the locking stuff, I did a clean on the project and the it worked again.
 
0
 
abelCommented:
Ah, glad to see that gregoryyoung is back on this thread, he's a bit better equipped when it comes to multi threading then I am ;-). You're in good hands there!
0
 
gregoryyoungCommented:
The rest of this I think is solved? not sure?
0

Featured Post

Get your problem seen by more experts

Be seen. Boost your question’s priority for more expert views and faster solutions

  • 6
  • 6
  • 5
Tackle projects and never again get stuck behind a technical roadblock.
Join Now