c# – Collection was modified; enumeration operation may not execute
c# – Collection was modified; enumeration operation may not execute
Whats likely happening is that SignalData
is indirectly changing the subscribers dictionary under the hood during the loop and leading to that message. You can verify this by changing
foreach(Subscriber s in subscribers.Values)
To
foreach(Subscriber s in subscribers.Values.ToList())
If Im right, the problem will disappear.
Calling subscribers.Values.ToList()
copies the values of subscribers.Values
to a separate list at the start of the foreach
. Nothing else has access to this list (it doesnt even have a variable name!), so nothing can modify it inside the loop.
When a subscriber unsubscribes you are changing contents of the collection of Subscribers during enumeration.
There are several ways to fix this, one being changing the for loop to use an explicit .ToList()
:
public void NotifySubscribers(DataRecord sr)
{
foreach(Subscriber s in subscribers.Values.ToList())
{
^^^^^^^^^
...
c# – Collection was modified; enumeration operation may not execute
A more efficient way, in my opinion, is to have another list that you declare that you put anything that is to be removed into. Then after you finish your main loop (without the .ToList()), you do another loop over the to be removed list, removing each entry as it happens. So in your class you add:
private List<Guid> toBeRemoved = new List<Guid>();
Then you change it to:
public void NotifySubscribers(DataRecord sr)
{
toBeRemoved.Clear();
...your unchanged code skipped...
foreach ( Guid clientId in toBeRemoved )
{
try
{
subscribers.Remove(clientId);
}
catch(Exception e)
{
System.Diagnostics.Debug.WriteLine(Unsubscribe Error +
e.Message);
}
}
}
...your unchanged code skipped...
public void UnsubscribeEvent(Guid clientId)
{
toBeRemoved.Add( clientId );
}
This will not only solve your problem, it will prevent you from having to keep creating a list from your dictionary, which is expensive if there are a lot of subscribers in there. Assuming the list of subscribers to be removed on any given iteration is lower than the total number in the list, this should be faster. But of course feel free to profile it to be sure thats the case if theres any doubt in your specific usage situation.