Collection Was Modified During Enumeration in C#: Why It Happens and How to Fix It

By the end of this page, you will understand why C# throws Collection was modified; enumeration operation may not execute, why it commonly appears with foreach and Dictionary<TKey, TValue>, and how to fix it safely. You will also learn the difference between modifying a collection in the same loop and modifying it from another thread, which is especially important in server code such as WCF services.

In C#, a foreach loop uses an enumerator behind the scenes. That enumerator expects the collection to stay structurally unchanged while it is being read.

A structural change means operations like:

• Add
• Remove
• Clear

When a collection changes during enumeration, the enumerator becomes invalid. To protect you from unpredictable behavior, .NET throws this exception:

Collection was modified; enumeration operation may not execute

In your code, this can happen in two different ways:

1. You remove an item during the foreach loop

   • Inside NotifySubscribers(), if SignalData(sr) throws, you call UnsubscribeEvent(s.ClientId).
   • That removes an entry from subscribers while foreach (Subscriber s in subscribers.Values) is still iterating.

2. Another thread modifies the dictionary at the same time

   • A client may subscribe or unsubscribe while NotifySubscribers() is looping.
   • Dictionary<TKey, TValue> is not thread-safe for concurrent reads/writes.

The debugger changes timing. When you attach a debugger, threads often run more slowly or in a different order, so the race condition may disappear temporarily. This is very common with concurrency bugs.

Why this matters in real programming:

• Server applications often have shared collections of clients, sessions, tasks, or cached objects.
• If these collections are updated from multiple methods or threads, iteration must be handled carefully.
• Ignoring this can cause random production errors that are hard to reproduce locally.

Think of foreach like reading names from a printed attendance sheet.

While you are moving down the list, someone else should not:

• erase a name,
• add a new name,
• reorder the list.

If that happens, your place on the sheet may no longer make sense.

A Dictionary enumerator works the same way. It expects the collection to remain stable until the loop finishes.

If you need to remove people while processing them, use one of these approaches:

• make a copy of the list first, then iterate the copy,
• collect items to remove in a separate list, then remove them after the loop,
• use a thread-safe collection when multiple threads are involved.

A useful mental shortcut:

• Read while looping: safe
• Change while looping: dangerous unless you control how it happens

The core rule is simple: do not structurally modify a collection while iterating it with foreach.

Problematic pattern

var numbers = new List<int> { 1, 2, 3, 4 };

foreach (var n in numbers)
{
    if (n % 2 == 0)
    {
        numbers.Remove(n); // Throws at runtime
    }
}

The loop is reading numbers, but Remove changes the collection.

Safer pattern: remove after enumeration

var numbers = new List<int> { 1, 2, 3, 4 };
var toRemove = new List<int>();

foreach (var n in numbers)
{
    if (n % 2 == 0)
    {
        toRemove.Add(n);
    }
}

foreach (var n in toRemove)
{
    numbers.Remove(n);
}

This works because the original loop only reads from numbers.

Safer pattern: iterate over a snapshot

For your scenario, a common solution is to iterate over a copy:

Subscriber[] snapshot;

lock (_sync)
{
    snapshot = subscribers.Values.ToArray();
}

foreach (var s in snapshot)
{
    try
    {
        s.Callback.SignalData(sr);
    }
    catch
    {
        lock (_sync)
        {
            subscribers.Remove(s.ClientId);
        }
    }
}

Why this helps

• snapshot is a separate array.
• Even if the dictionary changes later, the loop is walking the array, not the live dictionary.
• The lock protects shared access to the dictionary.

A stronger version for your service

[ServiceBehavior(InstanceContextMode = InstanceContextMode.Single)]
public class SubscriptionServer : ISubscriptionServer
{
    private static readonly Dictionary<Guid, Subscriber> subscribers = new();
    private static readonly object _sync = new object();

    public void NotifySubscribers(DataRecord sr)
    {
        Subscriber[] snapshot;

        lock (_sync)
        {
            snapshot = subscribers.Values.ToArray();
        }

        foreach (var s in snapshot)
        {
            try
            {
                s.Callback.SignalData(sr);
            }
            catch (Exception e)
            {
                DCS.WriteToApplicationLog(
                    e.Message,
                    System.Diagnostics.EventLogEntryType.Error);

                lock (_sync)
                {
                    subscribers.Remove(s.ClientId);
                }
            }
        }
    }

    public Guid SubscribeEvent(string clientDescription)
    {
        var subscriber = new Subscriber();
        subscriber.Callback = OperationContext.Current
            .GetCallbackChannel<IDCSCallback>();

        lock (_sync)
        {
            subscribers.Add(subscriber.ClientId, subscriber);
        }

        return subscriber.ClientId;
    }

    public void UnsubscribeEvent(Guid clientId)
    {
        lock (_sync)
        {
            subscribers.Remove(clientId);
        }
    }
}

This version avoids modifying the dictionary during direct enumeration and also protects it from concurrent access.

Consider this smaller example:

var words = new List<string> { "A", "B", "C" };

foreach (var word in words)
{
    Console.WriteLine(word);

    if (word == "B")
    {
        words.Remove("B");
    }
}

What happens step by step

1. foreach creates an enumerator for words.
2. The enumerator starts with the collection in a valid state.
3. First iteration:
   • word is "A"
   • Console.WriteLine("A")
4. Second iteration:
   • word is "B"
   • Console.WriteLine("B")
5. Inside the if, words.Remove("B") changes the list structure.
6. The enumerator is now invalid because the collection it was tracking has changed.
7. On the next attempt to continue iteration, .NET throws:

Collection was modified; enumeration operation may not execute

The same idea in your code

Your method does this conceptually:

foreach (Subscriber s in subscribers.Values)
{
    try
    {
        s.Callback.SignalData(sr);
    }
    catch
    {
        UnsubscribeEvent(s.ClientId); // modifies subscribers
    }
}

That means:

1. Start enumerating subscribers.Values
2. A callback fails
3. UnsubscribeEvent removes from subscribers
4. The foreach tries to continue
5. Enumeration fails because the dictionary changed

If a client unsubscribes from another thread during the same time window, the result is the same: the enumerator sees the collection changed and throws.

This issue appears often in real applications whenever code keeps a shared in-memory collection.

Subscriber or client registries

Like your WCF example:

• connected clients
• active WebSocket sessions
• SignalR connections
• event listeners

Job and task processing

A server may loop through active jobs while another thread adds or removes jobs.

Caches and lookup tables

A dictionary may store:

• user sessions
• configuration values
• object instances
• API tokens

If one part of the app reads while another writes, enumeration errors can occur.

UI applications

A WinForms or WPF app may loop through controls or models while an event handler removes one.

Data pipelines

A script may process a collection of files, records, or messages while filtering or deleting them at the wrong time.

The general lesson: if a collection is shared, think carefully about when it is read and when it is changed.

In production code, developers usually solve this with a small set of common patterns.

1. Snapshot before iterating

Very common when broadcasting to clients:

Subscriber[] snapshot;
lock (_sync)
{
    snapshot = subscribers.Values.ToArray();
}

This reduces the time spent holding a lock and makes the iteration stable.

2. Remove failed items after processing

Instead of removing during enumeration, collect IDs first:

var failedIds = new List<Guid>();

foreach (var s in snapshot)
{
    try
    {
        s.Callback.SignalData(sr);
    }
    catch
    {
        failedIds.Add(s.ClientId);
    }
}

lock (_sync)
{
    foreach (var id in failedIds)
    {
        subscribers.Remove(id);
    }
}

This pattern is clean and easy to reason about.

3. Guard shared state with lock

For ordinary dictionaries, a private lock object is standard:

private static readonly object _sync = new object();

Use the same lock consistently for:

• add
• remove
• snapshot creation

4. Use concurrent collections when appropriate

If many threads access the collection frequently, developers may choose ConcurrentDictionary<TKey, TValue>.

But even then, you still need to think about iteration semantics and consistency. A concurrent collection helps with thread-safe access, but it does not automatically make every operation behave like a fixed snapshot unless you explicitly design for that.

5. Early cleanup of dead connections

Broadcast systems often treat callback failures as a signal to unregister the client. Your code already follows this idea; it just needs a safer removal strategy.

6. Keep lock scope small

Developers usually avoid holding locks while making network calls or callbacks:

• bad: lock dictionary and call SignalData inside the lock
• better: copy subscribers under lock, then call callbacks outside the lock

This avoids blocking other operations for too long.

1. Removing from the same collection inside foreach

Broken example:

foreach (var item in items)
{
    if (ShouldRemove(item))
    {
        items.Remove(item);
    }
}

Why it fails:

• foreach expects the collection to stay unchanged.

How to avoid it:

• use a snapshot
• collect items to remove later
• use a loop structure that supports safe indexed removal when appropriate

2. Assuming the debugger proves the code is safe

Beginners often think:

• “It works in debug mode, so the logic must be fine.”

But concurrency bugs depend on timing. The debugger changes timing, so the bug may hide.

How to avoid it:

• trust the exception and review shared-state access
• test without breakpoints
• look for race conditions in server code

3. Using Dictionary from multiple threads without synchronization

Broken example:

private static Dictionary<Guid, Subscriber> subscribers = new();

public void Add(Subscriber s)
{
    subscribers.Add(s.ClientId, s);
}

public void Broadcast(DataRecord sr)
{
    foreach (var sub in subscribers.Values)
    {
        sub.Callback.SignalData(sr);
    }
}

Why it is risky:

• one thread may add or remove while another enumerates

How to avoid it:

• protect access with lock
• or use a concurrent collection when it fits

4. Locking around callback execution

Problematic pattern:

lock (_sync)
{
    foreach (var sub in subscribers.Values)
    {
        sub.Callback.SignalData(sr);
    }
}

Why this is dangerous:

• callbacks may be slow
• callbacks may re-enter your service
• other threads get blocked waiting for the lock

How to avoid it:

• copy first under lock
• perform callback calls outside the lock

5. Reinitializing a static collection in the constructor

Your code does this:

private static IDictionary<Guid, Subscriber> subscribers;

public SubscriptionServer()
{
    subscribers = new Dictionary<Guid, Subscriber>();
}

Why this is risky:

• static data belongs to the type, not an instance
• constructors can run more than once in some hosting scenarios
• reassigning can wipe existing subscribers

Safer version:

private static readonly Dictionary<Guid, Subscriber> subscribers = new();

6. Catching broad exceptions without a clear action

Broad catch (Exception) can be useful for logging failed clients, but make sure the code path is intentional.

How to avoid trouble:

• log enough context
• remove only the failing client
• avoid swallowing errors silently

Dictionary vs ConcurrentDictionary

Snapshot vs live enumeration

// Unsafe: modifying a collection during foreach
foreach (var item in collection)
{
    collection.Remove(item); // don't do this
}

// Safe pattern: snapshot + lock
private static readonly Dictionary<Guid, Subscriber> subscribers = new();
private static readonly object _sync = new object();

Subscriber[] snapshot;
lock (_sync)
{
    snapshot = subscribers.Values.ToArray();
}

foreach (var s in snapshot)
{
    // work with s
}

// Safe pattern: collect removals, then remove after loop
var failedIds = new List<Guid>();

foreach (var s in snapshot)
{
    try
    {
        s.Callback.SignalData(sr);
    }
    catch
    {
        failedIds.Add(s.ClientId);
    }
}

lock (_sync)
{
    foreach (var id in failedIds)
    {
        subscribers.Remove(id);
    }
}

Rules to remember

• foreach does not allow structural collection changes during enumeration.
• Dictionary<TKey, TValue> is not thread-safe for concurrent modification.
• Debuggers can hide race conditions by changing timing.
• In server code, prefer:
  • lock for shared access
  • snapshot before broadcasting
  • removal after iteration
• Do not initialize a static collection inside the instance constructor.

Good default fix for this case

1. Make the dictionary static readonly.
2. Add a private lock object.
3. Copy subscribers.Values to an array inside the lock.
4. Iterate the array outside the lock.
5. Remove failed clients inside a lock after the callback fails.

Why does foreach throw when a collection changes?

Because foreach uses an enumerator that expects the collection structure to remain unchanged while iterating.

Is removing the current item inside foreach ever safe in C#?

Not for normal collections like List<T> or Dictionary<TKey, TValue>. Use a snapshot, a separate removal list, or a different loop strategy.

Does this mean Dictionary is broken?

No. Dictionary<TKey, TValue> works correctly, but it is not designed for concurrent modification during enumeration.

Why does the error disappear when debugging?

Because the debugger changes execution timing. Race conditions often become less frequent or move to different places when code runs more slowly.

Should I use ConcurrentDictionary here?

Maybe, but not automatically. For a subscriber list, a lock plus snapshot iteration is often simpler and easier to reason about.

Should I lock the whole NotifySubscribers() method?

Usually no. Avoid holding a lock while calling client callbacks, because callbacks may be slow or re-entrant. Copy first, then notify outside the lock.

Why is initializing a static dictionary in the constructor a problem?

Because a static field belongs to the class, not each instance. Reassigning it in the constructor can reset shared state unexpectedly.

What is the simplest fix for my code?

Create a snapshot of subscribers before the loop, iterate the snapshot, and synchronize add/remove operations with a lock.

• foreach loop in C# — Important because the exception happens during enumeration.
• Dictionary<TKey, TValue> — The collection type involved in the error.
• Enumerators in C# — Explains why collection changes invalidate iteration.
• Thread safety — Critical when multiple threads access shared state.
• lock statement in C# — A common way to protect shared collections.
• ConcurrentDictionary<TKey, TValue> — A thread-safe alternative for concurrent access.
• Race conditions — Helps explain why the bug appears randomly and hides during debugging.
• Exception handling in C# — Relevant because callback failures trigger unsubscribe logic.