Signup/Sign In
Ask Question
Not satisfied by the Answer? Still looking for a better solution?

Collection was modified; enumeration operation may not execute

When a Windows Forms client subscribes, the subscriber ID is added to the subscribers dictionary, and when the client unsubscribes, it is deleted from the dictionary. The error happens when (or after) a client unsubscribes. It appears that the next time the NotifySubscribers() method is called, the foreach() loop fails with the error in the subject line. The method writes the error into the application log as shown in the code below. When a debugger is attached and a client unsubscribes, the code executes fine.

Do you see a problem with this code? Do I need to make the dictionary 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);
}
}
}
by

3 Answers

rahul07
What's 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 I'm 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 doesn't even have a variable name!), so nothing can modify it inside the loop.
sandhya6gczb
When a subscriber unsubscribes you are changing the 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())
{
^^^^^^^^^
...
RoliMishra
You can also lock your subscribers dictionary to prevent it from being modified whenever its being looped:

lock (subscribers)
{
foreach (var subscriber in subscribers)
{
//do something
}
}

Login / Signup to Answer the Question.