Коллекция была изменена; операция перечисления может не выполняться

911

Я не могу докопаться до этой ошибки, потому что, когда отладчик подключен, кажется, что это не происходит. Ниже приведен код.

Это сервер WCF в службе Windows. Метод NotifySubscribeers вызывается службой всякий раз, когда происходит событие данных (через случайные интервалы, но не очень часто - около 800 раз в день).

Когда клиент Windows Forms подписывается, идентификатор подписчика добавляется в словарь подписчиков, а когда клиент отписывается, он удаляется из словаря. Ошибка происходит, когда (или после) клиент отписывается. Похоже, что при следующем вызове метода NotifySubscribeers () цикл foreach () завершится с ошибкой в ​​строке темы. Метод записывает ошибку в журнал приложения, как показано в коде ниже. Когда отладчик подключен и клиент отписывается, код выполняется нормально.

Вы видите проблему с этим кодом? Нужно ли сделать словарь потокобезопасным?

[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);
        }
    }
}
cdonner
источник
в моем случае это был побочный эффект, потому что я использовал несколько .Include («таблица»), которые были изменены во время процесса - не очень очевидно при чтении кода. однако мне повезло, что эти Включения не были нужны (да! старый, не поддерживаемый код), и я решил свою проблему, просто удалив их
Ади

Ответы:

1632

Вероятно, происходит то, что SignalData косвенно изменяет словарь подписчиков под капотом во время цикла и приводит к этому сообщению. Вы можете проверить это, изменив

foreach(Subscriber s in subscribers.Values)

к

foreach(Subscriber s in subscribers.Values.ToList())

Если я прав, проблема исчезнет

Вызов subscribers.Values.ToList()копирует значения subscribers.Valuesв отдельный список в начале foreach. Ничто другое не имеет доступа к этому списку (у него даже нет имени переменной!), Поэтому ничто не может изменить его внутри цикла.

JaredPar
источник
14
Кстати .ToList () присутствует в dll System.Core, который не совместим с приложениями .NET 2.0. Поэтому вам может потребоваться изменить целевое приложение на .Net 3.5
mishal153
60
Я не понимаю, почему вы сделали ToList и почему это все исправляет
PositiveGuy
204
@CoffeeAddict: Проблема в том subscribers.Values, что в foreachцикле изменяются . Вызов subscribers.Values.ToList()копирует значения subscribers.Valuesв отдельный список в начале foreach. Ничто другое не имеет доступа к этому списку (у него даже нет имени переменной!) , Поэтому ничто не может изменить его внутри цикла.
BlueRaja - Дэнни Пфлугхофт
31
Обратите внимание, что ToListтакже может выдать, если коллекция была изменена во время ToListвыполнения.
Шрирам Шактивель
16
Я уверен, что думаю, не решит проблему, а только затруднит ее воспроизведение. ToListне атомная операция. Что еще смешнее, в ToListосновном он сам выполняет свои функции foreachдля копирования элементов в новый экземпляр списка, что означает, что вы устранили foreachпроблему, добавив дополнительную (хотя и более быструю) foreachитерацию.
Гро
113

Когда подписчик отменяет подписку, вы меняете содержимое коллекции подписчиков во время перечисления.

Есть несколько способов исправить это, один из которых заключается в изменении цикла for для использования явного .ToList():

public void NotifySubscribers(DataRecord sr)  
{
    foreach(Subscriber s in subscribers.Values.ToList())
    {
                                              ^^^^^^^^^  
        ...
Митч Пшеничный
источник
64

Более эффективный способ, на мой взгляд, состоит в том, чтобы иметь другой список, в который вы заявляете, что вы помещаете в него все, что «должно быть удалено». Затем после завершения основного цикла (без .ToList ()) вы делаете еще один цикл над списком «подлежащих удалению», удаляя каждую запись, как это происходит. Итак, в вашем классе вы добавляете:

private List<Guid> toBeRemoved = new List<Guid>();

Затем вы измените его на:

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 );
}

Это не только решит вашу проблему, но и избавит вас от необходимости создавать список из вашего словаря, что дорого, если там много подписчиков. Предполагая, что список подписчиков, которые будут удалены на любой итерации, меньше, чем общее число в списке, это должно быть быстрее. Но, конечно, не стесняйтесь профилировать его, чтобы быть уверенным, что это так, если есть какие-либо сомнения в вашей конкретной ситуации использования.

х 4000
источник
9
Я ожидаю, что это стоит рассмотреть, если у вас есть, вы работаете с большими коллекциями. Если бы он был маленьким, я бы, вероятно, просто ToList и двигаться дальше.
Карл Кенингер
42

Вы также можете заблокировать словарь подписчиков, чтобы предотвратить его изменение при зацикливании:

 lock (subscribers)
 {
         foreach (var subscriber in subscribers)
         {
               //do something
         }
 }
Мухаммед Сепахванд
источник
2
Это полный пример? У меня есть класс (_dictionary obj ниже), который содержит общий словарь <string, int> с именем MarkerFrequencies, но это не сразу решило проблему сбоя: lock (_dictionary.MarkerFrequencies) {foreach (KeyValuePair <string, int> пара в _dictionary.MarkerFrequencies) {...}}
Джон Кумбс
4
@JCoombs возможно, что вы модифицируете, возможно, переназначаете MarkerFrequenciesсловарь внутри самой блокировки, а это означает, что исходный экземпляр больше не заблокирован. Также попробуйте использовать forвместо foreach, см. Это и это . Дайте мне знать, если это решит это.
Мохаммед Сепахванд
1
Ну, я наконец-то нашел время для исправления этой ошибки, и эти советы были полезны - спасибо! Мой набор данных был небольшим, и эта операция была просто обновлением отображения пользовательского интерфейса, поэтому итерации по копии казались наилучшими. (Я также экспериментировал с использованием for вместо foreach после блокировки MarkerFrequencies в обоих потоках. Это предотвратило сбой, но, по-видимому, потребовало дальнейшей работы по отладке. И это привело к большей сложности. Моя единственная причина наличия двух потоков здесь - дать пользователю возможность отменить кстати, операция. Пользовательский интерфейс напрямую не изменяет эти данные.)
Джон Кумбс
9
Проблема в том, что для больших приложений блокировки могут быть основным ударом по производительности - лучше использовать коллекцию в System.Collections.Concurrentпространстве имен.
BrainSlugs83
28

Почему эта ошибка?

В целом коллекции .Net не поддерживают одновременное перечисление и изменение. Если вы попытаетесь изменить список коллекции во время перечисления, это вызовет исключение. Таким образом, проблема, стоящая за этой ошибкой, заключается в том, что мы не можем изменить список / словарь, пока мы повторяем его.

Одно из решений

Если мы выполняем итерацию по словарю, используя список его ключей, параллельно мы можем модифицировать объект словаря, поскольку мы выполняем итерацию по коллекции ключей, а не по словарю (и итерируем коллекцию ключей).

пример

//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;
}

Вот сообщение в блоге об этом решении.

И для глубокого погружения в StackOverflow: почему возникает эта ошибка?

открытый и бесплатный
источник
Спасибо! Четкое объяснение того, почему это происходит, и сразу же дало мне возможность решить эту проблему в моем приложении.
Ким
5

На самом деле мне кажется, что проблема заключается в том, что вы удаляете элементы из списка и ожидаете продолжить чтение списка, как будто ничего не произошло.

Что вам действительно нужно сделать, это начать с конца и вернуться к началу. Даже если вы удалите элементы из списка, вы сможете продолжить его чтение.

luc.rg.roy
источник
Я не понимаю, как это будет иметь значение? Если вы удалите элемент в середине списка, он все равно выдаст ошибку, так как элемент, к которому он пытается получить доступ, не найден?
Запнологика
4
@Zapnologica разница в том, что вы не будете перечислять список - вместо того, чтобы делать для / каждого, вы будете делать для / следующего и получить к нему доступ с помощью целого числа - вы определенно можете изменить список а в цикл for / next, но никогда не в цикле for / each (потому что для перечисления for / each ) - вы также можете делать это вперед в цикле for / next, если у вас есть дополнительная логика для настройки счетчиков и т. д.
BrainSlugs83
4

InvalidOperationException - произошло InvalidOperationException. Он сообщает, что «коллекция была изменена» в цикле foreach

Используйте оператор break, как только объект будет удален.

например:

ArrayList list = new ArrayList(); 

foreach (var item in list)
{
    if(condition)
    {
        list.remove(item);
        break;
    }
}
Вивек
источник
Хорошее и простое решение, если вы знаете, что в вашем списке есть не более одного элемента, который нужно удалить.
Таваб Вакиль
3

У меня была та же проблема, и она была решена, когда я использовал forцикл вместо foreach.

// foreach (var item in itemsToBeLast)
for (int i = 0; i < itemsToBeLast.Count; i++)
{
    var matchingItem = itemsToBeLast.FirstOrDefault(item => item.Detach);

   if (matchingItem != null)
   {
      itemsToBeLast.Remove(matchingItem);
      continue;
   }
   allItems.Add(itemsToBeLast[i]);// (attachDetachItem);
}
Даниэль Морешет
источник
11
Этот код неверен и пропустит некоторые элементы в коллекции, если какой-либо элемент будет удален. Например: у вас есть var arr = ["a", "b", "c"] и в первой итерации (i = 0) вы удаляете элемент в позиции 0 (элемент "a"). После этого все элементы массива переместятся на одну позицию вверх, и массив будет ["b", "c"]. Итак, на следующей итерации (i = 1) вы проверите элемент в позиции 1, который будет «c», а не «b». Это не верно. Чтобы это исправить, вы должны двигаться снизу вверх
Kaspars Ozols
3

Итак, что помогло мне, итерация назад. Я пытался удалить запись из списка, но делал итерации вверх, и это запутало цикл, потому что запись больше не существует:

for (int x = myList.Count - 1; x > -1; x--)
                        {

                            myList.RemoveAt(x);

                        }
Марк Авен
источник
2

Я видел много вариантов для этого, но для меня этот был лучшим.

ListItemCollection collection = new ListItemCollection();
        foreach (ListItem item in ListBox1.Items)
        {
            if (item.Selected)
                collection.Add(item);
        }

Тогда просто переберите коллекцию.

Имейте в виду, что ListItemCollection может содержать дубликаты. По умолчанию ничто не препятствует добавлению дубликатов в коллекцию. Чтобы избежать дубликатов, вы можете сделать это:

ListItemCollection collection = new ListItemCollection();
            foreach (ListItem item in ListBox1.Items)
            {
                if (item.Selected && !collection.Contains(item))
                    collection.Add(item);
            }
Майк
источник
Как этот код предотвратит ввод дубликатов в базу данных? Я написал что-то похожее, и когда я добавляю новых пользователей из списка, если я случайно оставлю одного выбранного, который уже есть в списке, он создаст дублирующую запись. У вас есть предложения @Mike?
Джейми
1

Принятый ответ неточен и неверен в худшем случае. Если изменения будут сделаны во времяToList() , вы все равно можете получить ошибку. Помимоlock , какую производительность и безопасность потоков необходимо учитывать, если у вас есть открытый член, правильное решение может использовать неизменяемые типы .

В общем случае неизменный тип означает, что вы не можете изменить его состояние после его создания. Итак, ваш код должен выглядеть так:

public class SubscriptionServer : ISubscriptionServer
{
    private static ImmutableDictionary<Guid, Subscriber> subscribers = ImmutableDictionary<Guid, Subscriber>.Empty;
    public void SubscribeEvent(string id)
    {
        subscribers = subscribers.Add(Guid.NewGuid(), new Subscriber());
    }
    public void NotifyEvent()
    {
        foreach(var sub in subscribers.Values)
        {
            //.....This is always safe
        }
    }
    //.........
}

Это может быть особенно полезно, если у вас есть публичный член. Другие классы всегда могут foreachиспользовать неизменяемые типы, не беспокоясь об изменении коллекции.

Джо
источник
1

Вот конкретный сценарий, который требует специализированного подхода:

  1. DictionaryЧасто перечислены.
  2. DictionaryИзменяется нечасто.

В этом сценарии создание копии Dictionary(или Dictionary.Values) перед каждым перечислением может быть довольно дорогостоящим. Моя идея решить эту проблему - повторно использовать одну и ту же кэшированную копию в нескольких перечислениях и просматривать IEnumeratorоригинал на Dictionaryпредмет исключений. Перечислитель будет кеширован вместе с скопированными данными и опрошен перед началом нового перечисления. В случае исключения кэшированная копия будет отброшена, и будет создана новая. Вот моя реализация этой идеи:

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;

public class EnumerableSnapshot<T> : IEnumerable<T>, IDisposable
{
    private IEnumerable<T> _source;
    private IEnumerator<T> _enumerator;
    private ReadOnlyCollection<T> _cached;

    public EnumerableSnapshot(IEnumerable<T> source)
    {
        _source = source ?? throw new ArgumentNullException(nameof(source));
    }

    public IEnumerator<T> GetEnumerator()
    {
        if (_source == null) throw new ObjectDisposedException(this.GetType().Name);
        if (_enumerator == null)
        {
            _enumerator = _source.GetEnumerator();
            _cached = new ReadOnlyCollection<T>(_source.ToArray());
        }
        else
        {
            var modified = false;
            if (_source is ICollection collection) // C# 7 syntax
            {
                modified = _cached.Count != collection.Count;
            }
            if (!modified)
            {
                try
                {
                    _enumerator.MoveNext();
                }
                catch (InvalidOperationException)
                {
                    modified = true;
                }
            }
            if (modified)
            {
                _enumerator.Dispose();
                _enumerator = _source.GetEnumerator();
                _cached = new ReadOnlyCollection<T>(_source.ToArray());
            }
        }
        return _cached.GetEnumerator();
    }

    public void Dispose()
    {
        _enumerator?.Dispose();
        _enumerator = null;
        _cached = null;
        _source = null;
    }

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

public static class EnumerableSnapshotExtensions
{
    public static EnumerableSnapshot<T> ToEnumerableSnapshot<T>(
        this IEnumerable<T> source) => new EnumerableSnapshot<T>(source);
}

Пример использования:

private static IDictionary<Guid, Subscriber> _subscribers;
private static EnumerableSnapshot<Subscriber> _subscribersSnapshot;

//...(in the constructor)
_subscribers = new Dictionary<Guid, Subscriber>();
_subscribersSnapshot = _subscribers.Values.ToEnumerableSnapshot();

// ...(elsewere)
foreach (var subscriber in _subscribersSnapshot)
{
    //...
}

К сожалению, эта идея в настоящее время не может использоваться с классом Dictionaryв .NET Core 3.0, потому что этот класс не генерирует исключение измененной коллекции, когда перечисляются и методы Removeи Clearвызываются. Все остальные контейнеры, которые я проверял, ведут себя последовательно. Я проверил систематически эти классы: List<T>, Collection<T>, ObservableCollection<T>, HashSet<T>, SortedSet<T>,Dictionary<T,V> иSortedDictionary<T,V> . Только два вышеупомянутых метода Dictionaryкласса в .NET Core не лишают законной силы перечисление.


Обновление: я исправил вышеуказанную проблему, сравнив также длину кэшированной и исходной коллекции. Это исправление предполагает, что словарь будет передан непосредственно в качестве аргументаEnumerableSnapshot конструктору, и его идентичность не будет скрыта (например, проекцией):dictionary.Select(e => e).ΤοEnumerableSnapshot() .


Важный: вышеупомянутый класс не потокобезопасен. Он предназначен для использования из кода, работающего исключительно в одном потоке.

Теодор Зулиас
источник
0

Вы можете скопировать объект словаря подписчиков во временный объект словаря того же типа, а затем выполнить итерацию объекта временного словаря, используя цикл foreach.

Rezoan
источник
(Похоже, что это сообщение не дает качественного ответа на вопрос. Пожалуйста, либо отредактируйте свой ответ и, либо просто опубликуйте его как комментарий к вопросу).
sɐunıɔ ןɐ qɐp
0

Поэтому другим способом решения этой проблемы было бы вместо удаления элементов создать новый словарь и добавить только те элементы, которые вы не хотите удалять, а затем заменить исходный словарь новым. Я не думаю, что это слишком большая проблема эффективности, потому что это не увеличивает количество итераций по структуре.

Форд префект
источник
0

Есть одна ссылка, где она разработана очень хорошо, и решение также дается. Попробуйте, если у вас есть правильное решение, пожалуйста, напишите здесь, чтобы другие могли понять. Данное решение в порядке, тогда как пост, так что другие могут попробовать это решение.

для вас ссылка оригинальная ссылка: - https://bensonxion.wordpress.com/2012/05/07/serializing-an-ienumerable-produces-collection-was-modified-enumeration-operation-may-not-execute/

Когда мы используем классы .Net Serialization для сериализации объекта, где его определение содержит тип Enumerable, то есть коллекцию, вы легко получите InvalidOperationException, говорящее «Коллекция была изменена; операция перечисления может не выполняться», если кодирование выполняется в многопоточных сценариях. Основная причина в том, что классы сериализации будут перебирать коллекцию через перечислитель, поэтому проблема заключается в попытке перебрать коллекцию при ее изменении.

Первое решение, мы можем просто использовать блокировку в качестве решения для синхронизации, чтобы гарантировать, что операция с объектом List может выполняться только из одного потока за раз. Очевидно, вы получите снижение производительности, что если вы хотите сериализовать коллекцию этого объекта, то для каждого из них будет применена блокировка.

Ну, .Net 4.0, который делает работу с многопоточными сценариями удобной. для этой проблемы с сериализацией в поле «Коллекция» я обнаружил, что мы можем просто воспользоваться классом ConcurrentQueue (Check MSDN), который является поточно-ориентированным и FIFO-коллекцией и делает код свободным от блокировки.

Используя этот класс, в его простоте материал, который вам нужно изменить для своего кода, заменяет им тип Collection, используйте Enqueue, чтобы добавить элемент в конец ConcurrentQueue, удалите код блокировки. Или, если сценарий, над которым вы работаете, требует таких сборщиков, как List, вам потребуется еще немного кода для адаптации ConcurrentQueue к вашим полям.

Кстати, ConcurrentQueue не имеет метода Clear из-за базового алгоритма, который не допускает атомарной очистки коллекции. так что вы должны сделать это сами, самый быстрый способ - создать новое пустое ConcurrentQueue для замены.

user8851697
источник
-1

Я лично столкнулся с этим недавно в незнакомом коде, где он был в открытом dbcontext с .Remove (item) Линка. У меня было чертовски много времени на обнаружение ошибок, потому что элементы были удалены при первой итерации, и если я попытался сделать шаг назад и повторить шаг назад, коллекция оказалась пустой, поскольку я находился в том же контексте!

Майкл Сефранек
источник