CODEWITHSUNDEEP
c#multithreadingconcurrencylocking
Hosea146
Hosea146

Reputation: 7702

Locking / Concurrency Issue

I have the following C# code:

1.    List<BandEdge> bandEdgeList;
2.    
3.    bandEdgeList = CicApplication.BandEdgeCache.Where(row => row.Coater == coater).ToList();
4.    foreach (BandEdge bandEdge in bandEdgeList)
5.       {
6.          ...
7.          ...
8.       }

My question is this. Once 'bandEdgeList' is populated on line 3, if another thread modifies the contents of CicApplication.BandEdgeCache, would the contents of 'bandEdgeList' be invalidated? I have a lock in the CicApplication.BandEdgeCache getter / setter. But I'm wondering if I should put a lock around this block of code so that the contents of CicApplication.BandEdgeCache don't change while I'm working with 'bandEdgeList'.

Upvotes: 3

Views: 155

Answers (3)

Russell Troywest
Russell Troywest

Reputation: 8776

Putting a lock in the getter of CicApplication.BandEdgeCache does not help you if it is returning a reference to the collection.

CicApplication.BandEdgeCache{
  get{lock(_myCollection){return _myCollection;}}
}

returns the reference but has EXITED the lock once it returns so using the Where() function on the reference to the collection returned by the getter is done OUTSIDE the lock and is not threadsafe. Another thread can quite happily alter the collection while the Where is iterating as the lock is not held - Daniel is correct, an InvalidOperationException will be thrown if another thread alters the collection while you are generating the list.

Once the list has been generated the original collection can be changed without it harming access to the new list.

Upvotes: 1

Oded
Oded

Reputation: 499002

bandEdgeList would be an independent copy (since you are using ToList()), so you don't need to lock.

However, as @Daniel A. White commented, you need to lock around the LINQ statement that creates that copy.

Upvotes: 3

Daniel A. White
Daniel A. White

Reputation: 190945

Not automatically, but this is still not thread-safe. It could throw a InvalidOperationException.

Once ToList is called, its saves a copy of those references. But if another thread modifies BandEdgeCache while that is happening, bad things happen.

So, you should lock all references to BandEdgeCache.

But along the lines of the saved list, that would be safe, but modifying any BandEdge isn't thread-safe without some locking.

Upvotes: 5

Related Questions