CODEWITHSUNDEEP
c#multithreading
RotatingWheel
RotatingWheel

Reputation: 1041

Excessive usage of lock in C#

I have inherited some C# code that I need to do fine-tuning. Since the dictionary (in the following code) is created on stack meaning individual instance (created by different threads) will be used for each call and it is not necessary to use the lock in this case, is that correct? Looks to me, it is not necessary. 

private object textLock = new object();
private Dictionary<string, string> GetMyTexts(Language language)
{
    Dictionary<string, string> texts = new Dictionary<string, string>();
    foreach (KeyValuePair<string, DisplayText> pair in Repository.DisplayTextCollection.Texts)
    {
        string value = pair.Value.Get(language);
        //other code ....
        lock(textLock)
        {
            texts.Add(pair.Key, value);
        }
    }
    return texts;
}

Upvotes: 3

Views: 231

Answers (3)

caesay
caesay

Reputation: 17233

For clarification, the dictionary is created on the heap - it's only the reference to the dictionary which lives on the stack.

Since no other thread or context has access to the reference until the method returns, no other code can simultaneously modify the dictionary, so the lock where it is currently is useless.

On the other hand, if the lock were outside the foreach loop, it might have been used to make sure only one of these methods is executing at any one time (for example, if Language or Repository was not thread safe)

Upvotes: 4

Gonzo345
Gonzo345

Reputation: 1343

Since there is just one thread supposed to be accessing that dictionary, according to the MSDN that lock statement wouldn't be necessary.

The lock statement acquires the mutual-exclusion lock for a given object, executes a statement block, and then releases the lock. While a lock is held, the thread that holds the lock can again acquire and release the lock. Any other thread is blocked from acquiring the lock and waits until the lock is released.

Upvotes: 0

meJustAndrew
meJustAndrew

Reputation: 6621

Most likely you are correct and the lock is useless, but to remove any doubt that someone may have, let me ask you a question:

Isn't this part of

//other code ....

containing a call to a method or some code that can acquire both the textLock and the texts?

This would be the only case when some other thread could insert into texts while locking also the textLock. If that is not the case, then you can safely remove the textLock.

Upvotes: 1

Related Questions