In C# would it be better to use Queue.Synchronized or lock() for thread safety?
I have a Queue object that I need to ensure is thread-safe. Would it be better to use a lock object like this:
lock(myLockObject)
{
//do stuff with the queue
}
Or is it recommended to use Queue.Synchronized like this:
Queue.Synchronized(myQueue).whatever_i_want_to_do();
From reading the MSDN docs it says I should use Queue.Synchronized to make it thread-safe, but then it gives an example using a lock object. From the MSDN article:
To guarantee the thread safety of the Queue, all operations must be done through this wrapper only.
Enumerating through a collection is intrinsically not a thread-safe procedure. Even when a collection is synchronized, other threads can still modify the collection, which causes the enumerator to throw an exception. To guarantee thread safety during enumeration, you can either lock the collection during the entire enumeration or catch the exceptions resulting from changes made by other threads.
If calling Synchronized() doesn't ensure thread-safety what's the point of it? Am I missing something here?
Answer
Personally I always prefer locking. It means that you get to decide the granularity. If you just rely on the Synchronized wrapper, each individual operation is synchronized but if you ever need to do more than one thing (e.g. iterating over the whole collection) you need to lock anyway. In the interests of simplicity, I prefer to just have one thing to remember - lock appropriately!
EDIT: As noted in comments, if you can use higher level abstractions, that's great. And if you do use locking, be careful with it - document what you expect to be locked where, and acquire/release locks for as short a period as possible (more for correctness than performance). Avoid calling into unknown code while holding a lock, avoid nested locks etc.
In .NET 4 there's a lot more support for higher-level abstractions (including lock-free code). Either way, I still wouldn't recommend using the synchronized wrappers.