Possible Duplicate:
Is using an existing object rather than creating a specific lock object safe?
What do you think: is it good practice to lock on private static objects that are in use inside my class? For example, I have Dictionary that contains elements and on modifying it I do lock(myList). I prefer to use special private System.Object field that is used just as a lock, but my colleague thinks that it's ok to use private static field, because it's already private.
Either way will work; however, I'd prefer a separate readonly, static object for synchronizing. That way its purpose is obvious and distinct for the type of lock you're performing.
If you're locking in order to safely work with a dictionary or list, have you considered using the System.Generic.Collections.Concurrent objects?
Also, have you considered a ReaderWriterLock? That way you can allow multiple reads, but safely lock to allow only one write at a time.
More Info
There's no difference locking against a separate object versus locking on the dictionary itself. The CLR will create or assign a SyncBlock struct and manage the lock count without regard to the object being locked. More info about that here: http://msdn.microsoft.com/en-us/magazine/cc188793.aspx
What I think:
A dictionary's responsibility is to store data, not synchronize threads. Use a separate object that is meant for synchronization, mark it as private readonly, and name it appropriately so that you know that the member is meant for locking.
You and your teammates are less likely to run into confusion about who is locking what and when. If the dictionary is pulling double duty, as a synchronization object and a dictionary, then it might not be obvious to new teammates that there is locking, it's on the dictionary, and they should not create a new synchronization object.
A new teammate might not know that the dictionary reference has dual responsibility as a synchronization object as well as a dictionary. That teammate might be tempted to pass a reference to the dictionary outside the class where it might be locked in an unexpected way that causes a deadlock. A separate synchronization object could cause your other teammates to stop and reconsider passing its reference around.
If the member is marked as readonly and initialized before a lock is called, then you're mostly guaranteed that the member will never be null or swapped out for another reference.
There are objects in the CLR that are meant specifically for thread synchronization. A SyncLock and Monitor.Enter are fine in most cases, but you can get much better performance by switching to a ReaderWriterLock. Not only can you allow multiple threads to read at the same time, but you can safely block all the thread to allow synchronous writes. Quick example here: http://www.codekicks.com/2008/03/readerwriterlock-cnet-systemthreading.html
