I have List<string> collection called List<string> list.
I have two threads. One thread is enumerating through all list elements and adding to collection. Second thread is enumerating through all list elements and removing from it.
How can make it thread safe? I tried creating global Object "MyLock" and using lock(MyLock) block in each thread function but it didn't work.
Can you help me?
If you have access to .NET 4.0 you can use the class ConcurrentQueue or a BlockingCollection with a ConcurrentQueue backing it. It does exactly what you are trying to do and does not require any locking. The BlockingCollection will make your thread wait if there is no items available in the list.
A example of removing from the ConcurrentQueue you do something like
ConcurrentQueue<MyClass> cq = new ConcurrentQueue<MyClass>();
void GetStuff()
{
MyClass item;
if(cq.TryDeqeue(out item))
{
//Work with item
}
}
This will try to remove a item, but if there are none available it does nothing.
BlockingCollection<MyClass> bc = BlockingCollection<MyClass>(new ConcurrentQueue<MyClass>());
void GetStuff()
{
if(!bc.IsCompleated) //check to see if CompleatedAdding() was called and the list is empty.
{
try
{
MyClass item = bc.Take();
//Work with item
}
catch (InvalidOpperationExecption)
{
//Take is marked as completed and is empty so there will be nothing to take
}
}
}
This will block and wait on the Take till there is something available to take from the list. Once you are done you can call CompleteAdding() and Take will throw a execption when the list becomes empty instead of blocking.
Without knowing more about your program and requirements, I'm going say that this is a "Bad Idea". Altering a List<> while iterating through it's contents will most likely throw an exception.
You're better off using a Queue<> instead of a List<>, as a Queue<> was designed with synchronization in mind.
You should be able to lock directly on your list:
lock(list) {
//work with list here
}
However adding/removing from the list while enumerating it will likely cause an exception...
Lock on the SyncRoot of your List<T>:
lock(list.SyncRoot)
{
}
More information on how to use it properly can be found here
You could implement your own version of IList<T> that wraps the underlying List<T> to provide locking on every method call.
public class LockingList<T> : IList<T>
{
public LockingList(IList<T> inner)
{
this.Inner = inner;
}
private readonly object gate = new object();
public IList<T> Inner { get; private set; }
public int IndexOf(T item)
{
lock (gate)
{
return this.Inner.IndexOf(item);
}
}
public void Insert(int index, T item)
{
lock (gate)
{
this.Inner.Insert(index, item);
}
}
public void RemoveAt(int index)
{
lock (gate)
{
this.Inner.RemoveAt(index);
}
}
public T this[int index]
{
get
{
lock (gate)
{
return this.Inner[index];
}
}
set
{
lock (gate)
{
this.Inner[index] = value;
}
}
}
public void Add(T item)
{
lock (gate)
{
this.Inner.Add(item);
}
}
public void Clear()
{
lock (gate)
{
this.Inner.Clear();
}
}
public bool Contains(T item)
{
lock (gate)
{
return this.Inner.Contains(item);
}
}
public void CopyTo(T[] array, int arrayIndex)
{
lock (gate)
{
this.Inner.CopyTo(array, arrayIndex);
}
}
public int Count
{
get
{
lock (gate)
{
return this.Inner.Count;
}
}
}
public bool IsReadOnly
{
get
{
lock (gate)
{
return this.Inner.IsReadOnly;
}
}
}
public bool Remove(T item)
{
lock (gate)
{
return this.Inner.Remove(item);
}
}
public IEnumerator<T> GetEnumerator()
{
lock (gate)
{
return this.Inner.ToArray().AsEnumerable().GetEnumerator();
}
}
IEnumerator IEnumerable.GetEnumerator()
{
lock (gate)
{
return this.Inner.ToArray().GetEnumerator();
}
}
}
You would use this code like this:
var list = new LockingList<int>(new List<int>());
If you're using large lists and/or performance is an issue then this kind of locking may not be terribly performant, but in most cases it should be fine.
It is very important to notice that the two GetEnumerator methods call .ToArray(). This forces the evaluation of the enumerator before the lock is released thus ensuring that any modifications to the list don't affect the actual enumeration.
Using code like lock (list) { ... } or lock (list.SyncRoot) { ... } do not cover you against list changes occurring during enumerations. These solutions only cover against concurrent modifications to the list - and that's only if all callers do so within a lock. Also these solutions can cause your code to die if some nasty bit of code takes a lock and doesn't release it.
In my solution you'll notice I have a object gate that is a private variable internal to the class that I lock on. Nothing outside the class can lock on this so it is safe.
I hope this helps.
As others already said, you can use concurrent collections from the System.Collections.Concurrent namespace. If you can use one of those, this is preferred.
But if you really want a list which is just synchronized, you could look at the SynchronizedCollection<T>-Class in System.Collections.Generic.
Note that you had to include the System.ServiceModel assembly, which is also the reason why I don't like it so much. But sometimes I use it.
