Koleksi telah dimodifikasi; operasi penghitungan tidak dapat dijalankan

911

Saya tidak bisa sampai ke bagian bawah kesalahan ini, karena ketika debugger terpasang, sepertinya tidak terjadi. Di bawah ini adalah kode.

Ini adalah server WCF dalam layanan Windows. Metode NotifySubscribers dipanggil oleh layanan setiap kali ada acara data (secara acak, tetapi tidak terlalu sering - sekitar 800 kali per hari).

Ketika klien Windows Forms berlangganan, ID pelanggan ditambahkan ke kamus pelanggan, dan ketika klien berhenti berlangganan, itu dihapus dari kamus. Kesalahan terjadi ketika (atau setelah) klien berhenti berlangganan. Tampaknya kali berikutnya metode NotifySubscribers () dipanggil, loop foreach () gagal dengan kesalahan di baris subjek. Metode menulis kesalahan ke dalam log aplikasi seperti yang ditunjukkan dalam kode di bawah ini. Ketika debugger dilampirkan dan klien berhenti berlangganan, kode dieksekusi dengan baik.

Apakah Anda melihat masalah dengan kode ini? Apakah saya perlu membuat kamus 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);
        }
    }
}
pendiam
sumber
dalam kasus saya itu adalah efek jaminan karena saya menggunakan beberapa. Termasuk ("tabel") yang dimodifikasi selama proses - tidak terlalu jelas ketika membaca kode. Namun saya beruntung bahwa Termasuk itu tidak diperlukan (ya! kode lama, tidak terawat) dan saya menyelesaikan masalah saya dengan hanya menghapusnya
Adi

Jawaban:

1631

Apa yang mungkin terjadi adalah bahwa SignalData secara tidak langsung mengubah kamus pelanggan di bawah tenda selama loop dan mengarah ke pesan itu. Anda dapat memverifikasi ini dengan mengubah

foreach(Subscriber s in subscribers.Values)

Untuk

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

Jika saya benar, masalahnya akan hilang

Memanggil subscribers.Values.ToList()menyalin nilai-nilai subscribers.Valueske daftar terpisah di awal foreach. Tidak ada lagi yang memiliki akses ke daftar ini (bahkan tidak memiliki nama variabel!), Jadi tidak ada yang dapat memodifikasinya di dalam loop.

JaredPar
sumber
14
BTW .ToList () hadir di System.Core dll yang tidak kompatibel dengan aplikasi .NET 2.0. Jadi, Anda mungkin perlu mengubah aplikasi target menjadi .Net 3.5
mishal153
60
Saya tidak mengerti mengapa Anda melakukan ToList dan mengapa itu memperbaiki semuanya
PositiveGuy
204
@CoffeeAddict: Masalahnya subscribers.Valuesadalah sedang dimodifikasi di dalam foreachloop. Memanggil subscribers.Values.ToList()menyalin nilai-nilai subscribers.Valueske daftar terpisah di awal foreach. Tidak ada lagi yang memiliki akses ke daftar ini (bahkan tidak memiliki nama variabel!) , Jadi tidak ada yang dapat memodifikasinya di dalam loop.
BlueRaja - Danny Pflughoeft
31
Catatan yang ToListjuga bisa melempar jika koleksi dimodifikasi saat ToListsedang dieksekusi.
Sriram Sakthivel
16
Saya cukup yakin berpikir tidak memperbaiki masalah, tetapi hanya membuatnya lebih sulit untuk mereproduksi. ToListbukan operasi atom. Apa yang lebih lucu, pada ToListdasarnya melakukan sendiri secara foreachinternal untuk menyalin item ke instance daftar baru, yang berarti Anda memperbaiki foreachmasalah dengan menambahkan foreachiterasi tambahan (meskipun lebih cepat) .
Groo
113

Ketika pelanggan berhenti berlangganan, Anda mengubah konten koleksi Pelanggan selama penghitungan.

Ada beberapa cara untuk memperbaikinya, salah satunya adalah mengubah for for menggunakan eksplisit .ToList():

public void NotifySubscribers(DataRecord sr)  
{
    foreach(Subscriber s in subscribers.Values.ToList())
    {
                                              ^^^^^^^^^  
        ...
Mitch Wheat
sumber
64

Cara yang lebih efisien, menurut saya, adalah memiliki daftar lain yang Anda nyatakan bahwa Anda memasukkan apa pun yang "harus dihapus". Kemudian setelah Anda menyelesaikan loop utama Anda (tanpa .ToList ()), Anda melakukan loop lain pada daftar "yang akan dihapus", menghapus setiap entri saat itu terjadi. Jadi di kelas Anda, Anda menambahkan:

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

Kemudian Anda mengubahnya menjadi:

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

Ini tidak hanya akan menyelesaikan masalah Anda, itu akan mencegah Anda dari harus terus membuat daftar dari kamus Anda, yang mahal jika ada banyak pelanggan di sana. Dengan asumsi daftar pelanggan yang akan dihapus pada setiap iterasi yang diberikan lebih rendah dari jumlah total dalam daftar, ini harus lebih cepat. Tetapi tentu saja merasa bebas untuk membuat profil untuk memastikan bahwa itu masalahnya jika ada keraguan dalam situasi penggunaan khusus Anda.

x4000
sumber
9
Saya berharap ini layak dipertimbangkan jika Anda memiliki Anda bekerja dengan koleksi yang lebih besar. Jika kecil saya mungkin hanya ToList dan pindah.
Karl Kieninger
42

Anda juga dapat mengunci kamus pelanggan Anda untuk mencegahnya diubah setiap kali di-looped:

 lock (subscribers)
 {
         foreach (var subscriber in subscribers)
         {
               //do something
         }
 }
Mohammad Sepahvand
sumber
2
Apakah itu contoh lengkap? Saya memiliki kelas (objek _dictionary di bawah) yang berisi Kamus generik <string, int> bernama MarkerFrequencies, tetapi melakukan ini tidak langsung menyelesaikan crash: lock (_dictionary.MarkerFrequencies) {foreach (KeyValuePair <string, int> memasangkan di _dictionary.MarkerFrequencies) {...}}
Jon Coombs
4
@JCoombs mungkin Anda memodifikasi, mungkin menetapkan ulang MarkerFrequencieskamus di dalam kunci itu sendiri, yang berarti bahwa instance asli tidak lagi terkunci. Coba juga menggunakan forbukan foreach, Lihat ini dan ini . Beri tahu saya jika itu menyelesaikannya.
Mohammad Sepahvand
1
Ya, saya akhirnya memperbaiki bug ini, dan tips ini sangat membantu - terima kasih! Dataset saya kecil, dan operasi ini hanya pembaruan tampilan UI, jadi iterasi pada salinan sepertinya yang terbaik. (Saya juga bereksperimen dengan menggunakan alih-alih foreach setelah mengunci MarkerFrequencies di kedua utas. Ini mencegah crash tetapi tampaknya memerlukan pekerjaan debug lebih lanjut. Dan itu memperkenalkan lebih banyak kerumitan. Satu-satunya alasan saya memiliki dua utas di sini adalah untuk memungkinkan pengguna membatalkan Omong-omong, sebuah operasi, UI tidak secara langsung mengubah data itu.)
Jon Coombs
9
Masalahnya adalah, untuk aplikasi besar, kunci bisa menjadi hit kinerja utama - lebih baik menggunakan koleksi di System.Collections.Concurrentnamespace.
BrainSlugs83
28

Kenapa kesalahan ini?

Secara umum koleksi .Net tidak mendukung yang disebutkan dan dimodifikasi pada saat yang sama. Jika Anda mencoba mengubah daftar koleksi selama enumerasi, itu menimbulkan pengecualian. Jadi masalah di balik kesalahan ini adalah, kita tidak bisa mengubah daftar / kamus sementara kita mengulanginya.

Salah satu solusinya

Jika kita mengulang kamus menggunakan daftar kuncinya, secara paralel kita dapat memodifikasi objek kamus, karena kita mengulangi melalui koleksi kunci dan bukan kamus (dan mengulangi koleksi kuncinya).

Contoh

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

Berikut adalah posting blog tentang solusi ini.

Dan untuk menyelam lebih dalam di StackOverflow: Mengapa kesalahan ini terjadi?

terbuka dan gratis
sumber
Terima kasih! Penjelasan yang jelas tentang mengapa itu terjadi dan segera memberi saya cara untuk menyelesaikan ini dalam aplikasi saya.
Kim Crosser
5

Sebenarnya masalahnya bagi saya bahwa Anda menghapus elemen dari daftar dan berharap untuk terus membaca daftar seolah-olah tidak ada yang terjadi.

Apa yang benar-benar perlu Anda lakukan adalah mulai dari akhir dan kembali ke awal. Bahkan jika Anda menghapus elemen dari daftar, Anda akan dapat terus membacanya.

luc.rg.roy
sumber
Saya tidak melihat bagaimana itu akan membuat perbedaan? Jika Anda menghapus elemen di tengah daftar itu masih akan menimbulkan kesalahan karena item yang coba diakses tidak dapat ditemukan?
Zapnologica
4
@ Zapnologica perbedaannya adalah - Anda tidak akan menyebutkan daftar - alih-alih melakukan untuk / masing-masing, Anda akan melakukan untuk / selanjutnya dan mengaksesnya dengan integer - Anda pasti dapat memodifikasi daftar dalam untuk / loop berikutnya, tetapi tidak pernah dalam untuk / setiap loop (karena untuk / setiap enumerasi ) - Anda juga dapat melakukannya ke depan dalam for / next, asalkan Anda memiliki logika tambahan untuk menyesuaikan penghitung Anda, dll.
BrainSlugs83
4

InvalidOperationException- Sebuah InvalidOperationException telah terjadi. Ini melaporkan "koleksi telah diubah" dalam foreach-loop

Gunakan pernyataan istirahat, Setelah objek dihapus.

ex:

ArrayList list = new ArrayList(); 

foreach (var item in list)
{
    if(condition)
    {
        list.remove(item);
        break;
    }
}
vivek
sumber
Solusi yang bagus dan sederhana jika Anda tahu daftar Anda memiliki paling banyak satu item yang perlu dihapus.
Tawab Wakil
3

Saya memiliki masalah yang sama, dan itu diselesaikan ketika saya menggunakan forloop, bukan 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);
}
Daniel Moreshet
sumber
11
Kode ini salah dan akan melewati beberapa item dalam koleksi jika ada elemen yang akan dihapus. Misalnya: Anda memiliki var arr = ["a", "b", "c"] dan dalam iterasi pertama (i = 0) Anda menghapus elemen pada posisi 0 (elemen "a"). Setelah ini semua elemen array akan bergerak satu posisi ke atas dan array akan menjadi ["b", "c"]. Jadi, dalam iterasi berikutnya (i = 1) Anda akan memeriksa elemen pada posisi 1 yang akan menjadi "c" bukan "b". Ini salah. Untuk memperbaikinya, Anda harus pindah dari bawah ke atas
Kaspars Ozols
3

Oke jadi yang membantu saya adalah beralih ke belakang. Saya mencoba untuk menghapus entri dari daftar tetapi berulang ke atas dan mengacaukan loop karena entri tidak ada lagi:

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

                            myList.RemoveAt(x);

                        }
Tandai Aven
sumber
2

Saya telah melihat banyak opsi untuk ini tetapi bagi saya yang satu ini adalah yang terbaik.

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

Maka cukup loop melalui koleksi.

Perlu diketahui bahwa ListItemCollection dapat berisi duplikat. Secara default tidak ada yang mencegah duplikat ditambahkan ke koleksi. Untuk menghindari duplikat Anda dapat melakukan ini:

ListItemCollection collection = new ListItemCollection();
            foreach (ListItem item in ListBox1.Items)
            {
                if (item.Selected && !collection.Contains(item))
                    collection.Add(item);
            }
Mike
sumber
Bagaimana kode ini mencegah duplikat dimasukkan ke dalam database. Saya telah menulis sesuatu yang serupa dan ketika saya menambahkan pengguna baru dari listbox, jika saya secara tidak sengaja menyimpan satu yang sudah ada dalam daftar, itu akan membuat entri duplikat. Apakah Anda punya saran @ Mike?
Jamie
1

Jawaban yang diterima tidak tepat dan salah dalam kasus terburuk. Jika perubahan dilakukan selamaToList() , Anda masih bisa berakhir dengan kesalahan. Selain itu lock, kinerja dan keamanan utas mana yang perlu dipertimbangkan jika Anda memiliki anggota publik, solusi yang tepat dapat menggunakan tipe yang tidak dapat diubah .

Secara umum, jenis yang tidak dapat diubah berarti bahwa Anda tidak dapat mengubah statusnya setelah dibuat. Jadi kode Anda akan terlihat seperti:

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

Ini bisa sangat berguna jika Anda memiliki anggota publik. Kelas lain selalu bisa foreachpada tipe yang tidak berubah tanpa khawatir tentang koleksi yang sedang dimodifikasi.

joe
sumber
1

Berikut ini adalah skenario khusus yang memerlukan pendekatan khusus:

  1. The Dictionarysering disebutkan.
  2. The Dictionarydimodifikasi jarang.

Dalam skenario ini membuat salinan dari Dictionary(atau Dictionary.Values) sebelum setiap penghitungan bisa sangat mahal. Gagasan saya tentang mengatasi masalah ini adalah menggunakan kembali salinan cache yang sama dalam beberapa enumerasi, dan menonton yang IEnumeratorasli Dictionarysebagai pengecualian. Enumerator akan di-cache bersama dengan data yang disalin, dan diinterogasi sebelum memulai enumerasi baru. Dalam kasus pengecualian, salinan yang di-cache akan dibuang, dan yang baru akan dibuat. Inilah implementasi dari ide ini:

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

Contoh penggunaan:

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)
{
    //...
}

Sayangnya ide ini tidak dapat digunakan saat ini dengan kelas Dictionarydi .NET Core 3.0, karena kelas ini tidak membuang Koleksi dimodifikasi pengecualian ketika disebutkan dan metode Removedan Cleardipanggil. Semua wadah lain yang saya periksa berperilaku konsisten. Aku memeriksa sistematis kelas ini: List<T>, Collection<T>, ObservableCollection<T>, HashSet<T>, SortedSet<T>, Dictionary<T,V>dan SortedDictionary<T,V>. Hanya dua metode yang disebutkan di Dictionarykelas di .NET Core tidak membatalkan enumerasi.


Pembaruan: Saya memperbaiki masalah di atas dengan membandingkan juga panjang cache dan koleksi asli. Perbaikan ini mengasumsikan bahwa kamus akan diteruskan langsung sebagai argumen ke EnumerableSnapshotkonstruktor, dan identitasnya tidak akan disembunyikan oleh (misalnya) proyeksi seperti: dictionary.Select(e => e).ΤοEnumerableSnapshot().


Penting: Kelas di atas tidak aman untuk thread. Ini dimaksudkan untuk digunakan dari kode yang berjalan secara eksklusif dalam satu utas.

Theodor Zoulias
sumber
0

Anda bisa menyalin objek kamus pelanggan ke jenis yang sama dari objek kamus sementara dan kemudian iterate objek kamus sementara menggunakan foreach loop.

Rezoan
sumber
(Posting ini sepertinya tidak memberikan jawaban yang berkualitas untuk pertanyaan. Harap edit jawaban Anda dan, atau cukup kirimkan sebagai komentar untuk pertanyaan).
sɐunıɔ ןɐ qɐp
0

Jadi cara yang berbeda untuk menyelesaikan masalah ini adalah alih-alih menghapus elemen, buat kamus baru dan hanya tambahkan elemen yang tidak ingin Anda hapus lalu ganti kamus asli dengan yang baru. Saya tidak berpikir ini terlalu banyak masalah efisiensi karena tidak meningkatkan berapa kali Anda beralih pada struktur.

ford prefek
sumber
0

Ada satu tautan di mana itu diuraikan dengan sangat baik & solusi juga diberikan. Cobalah jika Anda mendapat solusi yang tepat, silakan kirim di sini sehingga orang lain dapat mengerti. Solusi yang diberikan ok maka seperti posting sehingga orang lain dapat mencoba solusi ini.

untuk Anda referensi tautan asli: - https://bensonxion.wordpress.com/2012/05/07/serializing-an-ienumerable-produces-collection-was-modified-enumeration-operation-may-not-execute/

Ketika kami menggunakan. Serialisasi kelas Net untuk membuat serial objek di mana definisinya berisi tipe Enumerable, yaitu koleksi, Anda akan dengan mudah mendapatkan InvalidOperationException yang mengatakan "Koleksi telah diubah; operasi penghitungan mungkin tidak dijalankan" di mana pengkodean Anda berada di bawah skenario multi-utas. Penyebab utamanya adalah bahwa kelas serialisasi akan beralih melalui pengumpulan melalui enumerator, sehingga masalah untuk mencoba beralih melalui koleksi sambil memodifikasinya.

Solusi pertama, kita cukup menggunakan kunci sebagai solusi sinkronisasi untuk memastikan bahwa operasi ke objek Daftar hanya dapat dijalankan dari satu utas pada satu waktu. Jelas, Anda akan mendapatkan penalti kinerja jika Anda ingin membuat serial koleksi objek itu, maka untuk masing-masing, kunci akan diterapkan.

Nah, .Net 4.0 yang membuat berurusan dengan skenario multi-threading berguna. untuk masalah bidang Pengumpulan serialisasi ini, saya menemukan kita bisa mengambil manfaat dari kelas ConcurrentQueue (Periksa MSDN), yang merupakan koleksi aman-benang dan FIFO dan membuat kode bebas dari kunci.

Menggunakan kelas ini, dalam kesederhanaannya, hal-hal yang perlu Anda modifikasi untuk kode Anda menggantikan tipe Koleksi dengannya, gunakan Enqueue untuk menambahkan elemen ke akhir ConcurrentQueue, hapus kode kunci itu. Atau, jika skenario yang sedang Anda kerjakan memang membutuhkan barang koleksi seperti Daftar, Anda akan memerlukan beberapa kode lagi untuk mengadaptasi ConcurrentQueue ke dalam bidang Anda.

BTW, ConcurrentQueue tidak memiliki metode yang jelas karena algoritma yang mendasari yang tidak mengizinkan pembersihan koleksi secara atom. jadi Anda harus melakukannya sendiri, cara tercepat adalah membuat kembali ConcurrentQueue kosong baru untuk pengganti.

pengguna8851697
sumber
-1

Saya pribadi menjalankan ini baru-baru ini dalam kode asing di mana itu dalam dbcontext terbuka dengan Linq's .Remove (item). Saya mengalami kesulitan menemukan debugging kesalahan karena item dihapus pertama kali iterasi, dan jika saya mencoba untuk mundur dan melangkah kembali, koleksi itu kosong, karena saya berada dalam konteks yang sama!

Michael Sefranek
sumber