I have a multi-threading app where several threads manipulate a list (Of T), add, remove, and query the list. Here one of the threads query the list and then need to iterate the result set. Of course it's expected to get an exception indicates a collection was changed during the iteration, for some reason related the nature of the query and the iteration behavior I can't lock the list here, so I tried to get a copy of the result using the .ToList() function, but it was strange that I still receive the same error. I thought .ToList() function get separated list of the result, but it not looks to be. Is there any alternative way to get the query result in a new list?
1 Answer
You cannot access an instance of List<T> from multiple threads without synchronization, as the resulting behavior is undefined and in most cases leads to problems. You found one already.
Create some kind of ConcurrentCollection and synchronize all the operations you need:
public class ConcurrentCollection<T> : IEnumerable<T>
{
private readonly List<T> innerList = new List<T>();
public void Add(T item)
{
lock (innerList)
{
innerList.Add(item);
}
}
public bool TryRemove(T item)
{
lock (innerList)
{
return innerList.Remove(item);
}
}
public int Count
{
get
{
lock (innerList)
{
return this.innerList.Count;
}
}
}
public IEnumerator<T> GetEnumerator()
{
lock (innerList)
{
return (IEnumerator<T>) this.innerList.ToArray().GetEnumerator();
}
}
IEnumerator IEnumerable.GetEnumerator()
{
return this.GetEnumerator();
}
}
The critical point here is that the returned enumerator enumerates over a snapshot of the list (ToArray) so it doesn't matter if it is modified in between.
Keep in mind, that you have to create "atomic" operations for your calling code. Let's assume from one thread you want to add an item only if the list is empty. This would be wrong:
if (collection.Count == 0)
{
collection.Add(item);
}
Instead, add another member to the ConcurrentCollection...
public bool AddIfEmpty(T item)
{
lock (innerList)
{
if (innerList.Count == 0)
{
innerList.Add(item);
return true;
}
return false;
}
}
... and use it like this:
var itemWasAdded = collection.AddIfEmpty(item);
10 Comments
Mephy
I guess that is a rough implementation of
System.Collections.Concurrent.ConcurrentBag<T>. This code kind of reinvents the wheel.
Drunken Code Monkey
Agreed with Mephy here, what you should be using is a ConcurrentBag<T>. It is the concurrent equivalent of a List<T>. It's not called a list because no sorting can be done on a bag.
Abed
@Frank - thanks but all your functions force locking the list, and as I said I can't do this, all what I need is to get the result of the query in a new list or a new array, even if this result is only a snapshot and without locking the list.
Abed
@Mephy - Thanks. so far our system is running on framework 3.5, and this class is available only on 4.5+. But if I didn't find another solution then I will consider changing the target framework and will try it. But the question is: if ConcurrentBag is managing adding and removing items and force implemented lock, then will it lock the entire list while executing the query as well?! I still don't know how this will help me getting the result of the query and iterating the result snapshot while I still allowing other threads to keep working on the list.
Abed
Just to make it clear again, I need to iterate the result of the query, as iterating the snapshot at the moment of the query execution, and I don't care about what will happen to the list later by other threads.
|
List<T>is not thread-safe. You can't do that.ToList, or when you're iterating over the result?