1

I have a dictionary that is being used in two threads, one is sending out UDP packets, one is receiving them. Both are keeping a common collection to see count outgoing and returning packets and hopefully keeping them at 0 difference :)

Now I'm iterating through the dictionary to update values and after iteration it errors. I do have a lock object in place, how could I solve this?

First Thread:

            lock (retryListLock)
            {
                // loop through all known devices in the device list to build a counter if the device still lives
                foreach(string key in retryList.Keys)
                {
                    retryList[key] += 1;
                    if (retryList[key] > Retries)
                    {
                        DiscoveredDevice device = Devices.Find(d => d.SerialNo == key);

                        if (device != null)
                        {
                            OnDeviceRemoved(device);
                            Devices.Remove(device);
                            retryList.Remove(key);
                        }
                    }
                }
            }

Second Thread:

lock (retryListLock)
{
     if (retryList.ContainsKey(frame.SerialNo))
           retryList[frame.SerialNo] = 0;
     else
           retryList.Add(frame.SerialNo, 0);
}

I'm only getting the error after the first thread adds +1 to the value of that item, in the second iteration it errors out:

the collection has changed. enumeration operation may not execute (translated from Dutch)

How can I solve this? Obviously the Dictionary is the easiest to use for me in this case.

Martin
  • 397
  • 3
  • 16
  • "It errors out" -- pure textual noise. And without it there's no question, so flagging this to be closed. – Blindy Aug 05 '15 at 20:25
  • @Martin If you read the error message it will explain what the problem is. If you want more information, you can simply do a web search on that error message to find out more. Clearly you have done no research at all on this very common problem. – Servy Aug 05 '15 at 20:27
  • @crush Given how he's using it he'd still have to use a `lock` when accessing it, as he's performing multiple operations that need to be observably atomic with respect to others using the dictionary. – Servy Aug 05 '15 at 20:28
  • @Servy Right, I didn't actually read the question until now - I shouldn't have commented so soon. – crush Aug 05 '15 at 20:28
  • I did read into this, but I then suppose I have to move away from the Dictionary, which I was trying not too, to my understanding with the current problem I'm not changing the enumerator of the Dictionary but only the contents. The Remove will later on cause a problem however. – Martin Aug 05 '15 at 20:31

4 Answers4

1

The problem is that you cannot change the dictionary in an iterator/foreach

        foreach(string key in retryList.Keys)
        {
            retryList[key] += 1; // <-- The error happens here ! Do not alter the Dictionary during an iteration
            if (retryList[key] > Retries)
            {
                DiscoveredDevice device = Devices.Find(d => d.SerialNo == key);

                if (device != null)
                {
                    OnDeviceRemoved(device);
                    Devices.Remove(device);
                    retryList.Remove(key); // <-- The error could also happen here ! Do not alter the Dictionary during an iteration
                }
            }
        }

I found this question on stackoverflow which might help you

How to iterate through Dictionary and change values?

Here we find a statement from MSDN

The foreach statement is a wrapper around the enumerator, which allows only reading from the collection, not writing to it.

Community
  • 1
  • 1
Bongo
  • 2,933
  • 5
  • 36
  • 67
  • So you can't change the Dictionary at all in a loop? – Martin Aug 05 '15 at 20:31
  • @Bongo The += 1 is allowed because it is not modifying the collection, just an item it contains. The error will still happen there though because its accessing the collection *after* its been modified, and thats the first access point after modification in the previous iteration. – Ron Beyer Aug 05 '15 at 20:32
  • @RonBeyer maybe I misunderstood you but I think the error happens in this statement when accessing it a second time: foreach(string key in retryList.Keys) which is due to alteration of the dictionary – Bongo Aug 05 '15 at 20:49
  • By taking the keys into a second list it was solved: List Keys = new List(retryList.Keys); Thanks Bongo. – Martin Aug 05 '15 at 20:50
1

With thanks to Bongo, taking the keys into a second list for iteration solved it:

List<string> Keys = new List<string>(retryList.Keys);
                foreach(string key in Keys)
Martin
  • 397
  • 3
  • 16
0

The error has nothing to do with the locks and multithreading. Your enumerator (what foreach are using) is invalidated when you modify the very same data structure (the dictionary) on what the enumerator enumerates in the loop.

Solution:

You must first loop say with foreach, and remember practically in a list what you should remove. Then in a separate loop on the remembered keys, remove they from the dictionary.

g.pickardou
  • 32,346
  • 36
  • 123
  • 268
0

This might be useful to you. Concurrent Dictionary It's thread safe

Also change the for each to a reverse for loop Something like this.

for (int x = max; x>=1; x--)
{
}
Steven Yates
  • 2,400
  • 3
  • 30
  • 58
  • How do you get a value from a Dictionary based on index? This is actually the road I initially took. – Martin Aug 05 '15 at 20:52
  • You can use https://msdn.microsoft.com/en-us/library/bb299233(v=vs.110).aspx but dictionarys are not sorted so if you need it to be sorted you might want to consider switching to a sortedlist and do away with the dictionary if you don't really need the key aspect and can manage it differently. See extention methods on https://msdn.microsoft.com/en-us/library/1e8ecdh9(v=vs.110).aspx – Steven Yates Aug 05 '15 at 21:03