I have a thread #1 which is looping continuously with foreach through a ConcurrentBag list and modifying the items, but at some time another thread #2 needs to modify an item from the list. Is it safe if I take out all the objects from thread #2 (while the other one is looping with foreach) and modify the object then add all items back in thread #2 ? if it's not then is there a workaround ?
ConcurrentBag<MyObject> list;
while (1) // continuous loop
{
foreach(MyObject obj in list)
{
obj.RunMethod();
Thread.Sleep(500);
}
Thread.Sleep(1000);
}
(at some point a callback is called on thread #2 and need to modify an object)
List<MyObject> temp;
while (list.TryTake(out obj)
{
MyObject obj2=obj.Clone(); // make a copy of the object
if (obj2.Id==4) obj2.variable=10;
temp.Add(obj2);
}
// Add objects back to concurrentbag
foreach(MyObject obj in temp) list.Add(obj);
No, this isn't safe because the thread doing foreach may have retrieved a reference to an object and be working with that object while the other thread removes it from the bag, modifies it and puts it back.
This could result in the object's state changing while the foreach thread is using it. You can avoid this by creating a copy of the object to be mutated, and then change the copy and put the copy back into the bag.
Have you considered making your object immutable? Doing so will really simplify things, and actually makes it impossible for two threads to mutate it at the same time.
NOTE: Copying a reference to an object does NOT copy the contents of the object, so when you do list.TakeOut(out obj) you are receiving a reference to an existing object, not creating a new copy of it.
[EDIT] So you edited your question to use obj.Clone(). If that does a proper deep clone of your object, then the code should be OK, but that's only if you're careful to copy the object everywhere it is changed. If you make the class immutable, then you GUARANTEE this behaviour,so that's the best solution if you can use it.