Problem
I can’t figure out what’s causing this problem because it doesn’t seem to happen when the debugger is attached.
The code can be found below.
This is a Windows service with a WCF server. When a data event occurs, the service calls the method NotifySubscribers() (at random intervals, but not very often – about 800 times per day).
The subscriber ID is added to the subscribers dictionary when a Windows Forms client subscribes, and it is removed when the client unsubscribes. When (or after) a client unsubscribes, the error occurs. The foreach() loop appears to fail with the error in the subject line the next time the NotifySubscribers() method is performed. As demonstrated in the code below, the method logs the error in the application log. The code runs fine when a debugger is attached and a client unsubscribes.
Is there anything wrong with this code? Is it necessary for the dictionary to be thread-safe?
[ServiceBehavior(InstanceContextMode=InstanceContextMode.Single)]
public class SubscriptionServer : ISubscriptionServer
{
private static IDictionary<Guid, Subscriber> subscribers;
public SubscriptionServer()
{
subscribers = new Dictionary<Guid, Subscriber>();
}
public void NotifySubscribers(DataRecord sr)
{
foreach(Subscriber s in subscribers.Values)
{
try
{
s.Callback.SignalData(sr);
}
catch (Exception e)
{
DCS.WriteToApplicationLog(e.Message,
System.Diagnostics.EventLogEntryType.Error);
UnsubscribeEvent(s.ClientId);
}
}
}
public Guid SubscribeEvent(string clientDescription)
{
Subscriber subscriber = new Subscriber();
subscriber.Callback = OperationContext.Current.
GetCallbackChannel<IDCSCallback>();
subscribers.Add(subscriber.ClientId, subscriber);
return subscriber.ClientId;
}
public void UnsubscribeEvent(Guid clientId)
{
try
{
subscribers.Remove(clientId);
}
catch(Exception e)
{
System.Diagnostics.Debug.WriteLine("Unsubscribe Error " +
e.Message);
}
}
}
Asked by cdonner
Solution #1
SignalData is most likely modifying the subscribers dictionary invisibly under the hood during the loop, resulting in that message. You can check this by making a change.
foreach(Subscriber s in subscribers.Values)
To
foreach(Subscriber s in subscribers.Values.ToList())
If I’m correct, the problem will vanish.
Subscribers are being called. Values. ToList() duplicates the subscribers’ values. At the start of the foreach, assign values to a separate list. Nothing outside the loop has access to this list (it doesn’t even have a variable name! ), thus nothing can change it.
Answered by JaredPar
Solution #2
When a subscriber unsubscribes, the contents of the Subscribers collection are changed during enumeration.
There are a few solutions, one of which is to change the for loop to use an explicit. ToList():
public void NotifySubscribers(DataRecord sr)
{
foreach(Subscriber s in subscribers.Values.ToList())
{
^^^^^^^^^
...
Answered by Mitch Wheat
Solution #3
A more efficient method, in my opinion, is to create a separate list where you designate everything that needs to be “removed.” Then, after finishing your main loop (without the.ToList()), you cycle over the “to be removed” list again, eliminating each entry one by one. As a result, you include the following in your class:
private List<Guid> toBeRemoved = new List<Guid>();
After that, you alter 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 fix your problem, but it will also save you the time and money of having to continuously compiling a list from your dictionary, which may be costly if you have a large number of subscribers. This should be faster if the list of subscribers to be eliminated on each iteration is smaller than the total number of subscribers in the list. But of course feel free to profile it to be sure that’s the case if there’s any doubt in your specific usage situation.
Answered by Chris McElligott Park
Solution #4
Generally speaking. Enumeration and modification of net collections are not possible at the same time. It throws an error if you try to change the collection list during enumeration. The problem with this mistake is that we can’t change the list/dictionary while looping through it.
We can edit the dictionary object while iterating through a list of its keys, because we are iterating through the key-collection rather than the dictionary itself (and iterating its key collection).
//get key collection from dictionary into a list to loop through
List<int> keys = new List<int>(Dictionary.Keys);
// iterating key collection using a simple for-each loop
foreach (int key in keys)
{
// Now we can perform any modification with values of the dictionary.
Dictionary[key] = Dictionary[key] - 1;
}
A blog post on this solution can be found here.
For a more in-depth look at StackOverflow: What causes this error?
Answered by Shri
Solution #5
Okay, so iterating backwards was a big help for me. I was attempting to remove an entry from a list but iterating upwards caused the loop to break because the entry no longer existed:
for (int x = myList.Count - 1; x > -1; x--)
{
myList.RemoveAt(x);
}
Answered by Mark Aven
Post is based on https://stackoverflow.com/questions/604831/collection-was-modified-enumeration-operation-may-not-execute