Thread Safe: Generic Dictionary

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.

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?

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.

> you should not need a lock.

means: you do not need a lock.

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?

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

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;
                    }
               
            }
        }

                                              

1:
2:
3:
4:
5:
6:
7:
8:
9:
10:
11:
12:
13:
14:
15:
16:
17:
18:
19:
20:
21:
22:
23:
24:
25:
26:
27:
28:
29:
30:
31:

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?

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

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.

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

no i'm lockihn on a object

 _lockStateMotionStoppingSTorLimSW is a object not a bool

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.

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!