views:

147

answers:

9
lock(dictionaryX)
{
   dictionaryX.TryGetValue(key, out value);
}

is locking necessary while doing lookups to a Dictionary ?

THe program is multithreaded, and while adding key/value to dict. dict is being locked.

+2  A: 

Locking is only needed when you are synchronizing access to a resource between threads. As long as there are not mulitple threads involved then locking is not needed here.

In the context of updating and reading the value from multiple threads, yes a lock is absolutely necessary. In fact if you're using 4.0 you should consider switching to one of the collections specifically designed for concurrent access.

JaredPar
even for lookups ? while writing, it s already being locked.
@user177883 no operation on `Dictionary` requires a lock unless there are multiple threads involved
JaredPar
I also mentioned that the dictionary is being locked while adding data to it. but why would i lock it again, while doing lookup? does that mean, i can still do lookup while dict is locked for writes?
@user177883 because if you don't lock during lookup as well then you could start doing a lookup while a write was in progress and hence see the Dictionary an an intermediate state.
JaredPar
"lock ensures that one thread does not enter a critical section while another thread is in the critical section of code. If another thread attempts to enter a locked code, it will wait (block) until the object is released." if i m locking it while writing, i doubt that i need to lock it while doing lookup ??
@user177883: Locking takes the cooperation of the reader AND the writer. If the reader doesn't lock then it's as if the writer never did any locking at all. "Locking" only means taking ownership of a mutex, it doesn't stop execution of code that doesn't attempt to take the mutex.
Nathan Hughes
i see. thanks for the explanation and i ll take that as the answer.
A: 

Yes, you should lock if this dictionary is a shared resource between more than one thread. This ensures that you get the correct value and other thread doesn't happen to change value mid-way during your Lookup call.

Aamir
+7  A: 

As mentioned here:

Using TryGetValue() without locking is not safe. The dictionary is temporarily in a state that makes it unsuitable for reading while another thread is writing the dictionary. A dictionary will reorganize itself from time to time as the number of entries it contains grows. When you read at the exact time this re-organization takes place, you'll run the risk of finding the wrong value for the key when the buckets got updated but not yet the value entries.

UPDATE: take a look at "Thread Safety" part of this page too.

Kamyar
This is only true when threading is involved. Without threading no lock is needed.
JaredPar
OP mentioned explicitly that this operation is taking place in a multithreaded environment.
Jesse C. Slicer
I agree. But the question author means using lock in a multi-threaded environment.
Kamyar
Well, I haven't been in such a situation. But according to JaredPar, no locking is needed unless you are in a mutil-threaded environment. Looking at his reputation points, I'm pretty sure he knows what he's talking about!
Kamyar
Hmm, the copy of the answer is getting a lot more upvotes then I ever got when posting it. Not sure if attribution is required here, but it ought to be.
Hans Passant
A: 

Yes you need to lock the dictionary for access in a multithreaded environment. The writing to a dictionary is not atomic, so it could add the key, but not the value. In that case when you access it, you could get an exception.

Yuriy Faktorovich
not for writing, for reading.
@user177 yes, I was talking about reading. When I mentioned writing, it is an example of what could happen during the time you're trying to read.
Yuriy Faktorovich
+1  A: 

As with many subtle questions in programming, the answer is: Not necessarily.

If you only add values as an initialization, then the subsequent reading does not need to be synchronized. But, on the other hand, if you're going to be reading and writing at all times, then absolutely you need to protect that resource.

However, a full-blown lock may not be the best way, depending on the amount of traffic your Dictionary gets. Try a ReaderWriterLockSlim if you are using .NET 3.5 or greater.

Jesse C. Slicer
+1: While it is worth a shot it is *very* likely that `ReaderWriterLockSlim` will be slower than a plain old `lock` in this situation...a lot slower in fact. In my own personal tests I have discovered that `ReaderWriterLockSlim` has about 5x the overhead and the old `ReaderWriterLock` is about 15x. A RW lock has a much narrower range of application than does a normal `lock` and as such it will be slower in most situations. RW locks really shine when the lock is held for a long duration and the number of writers outnumber the readers by a wide margin. It is worth benchmarking though.
Brian Gideon
+2  A: 

If you have multiple threads accessing the Dictionary, then you do need to lock on updates and on lookup. The reason you need to lock on lookup is that there could be an update taking place at the same time you're doing the lookup, and the Dictionary could be in an inconsistent state during the update. For example, imagine that your have one thread doing this:

if (myDictionary.TryGetValue(key, out value))
{
}

and a separate thread is doing this:

myDictionary.Remove(key);

What could happen is that the thread doing the TryGetValue determines that the item is in the dictionary, but before it can retrieve the item, the other thread removes it. The result would be that the thread doing the lookup would either throw an exception or TryGetValue would return true but the value would be null or possibly an object that does not match the key.

That's only one thing that can happen. Something similarly disastrous can happen if you're doing a lookup on one thread and another thread does an add of the value that you're trying to look up.

Jim Mischel
+1  A: 

If you are on .Net 4, you can replace with ConcurrentDictionary to do this safely. There are other similar collections, preferred when you need multithreaded access, in the System.Collection.Concurrent namespace.

Don't use roll-your-own locking if this is an option for you.

Steve Townsend
+2  A: 

Use the new ConcurrentDictionary<TKey, TValue> object and you can forget about having to do any locks.

SnickersAreMyFave
A: 

Yes, you have to lock if you have multithreaded updates on that dictionary. Check this great post for details: “Thread safe” Dictionary(TKey,TValue)

But since ConcurrentDictionary<> introduced you can use it either via .NET 4 or by using Rx in 3.5 (it contains System.Threading.dll with implementation for new thread-safe collections)

Nick Martyshchenko