Collection was modified in async application?

Question

I have the following code in my .NET 4.6.2 winforms app:

paramList is an OrderedDictionary.

if ( paramList != null ) {
     foreach ( DictionaryEntry param in paramList ) {
         command.Parameters.AddWithValue(param.Key.ToString(), param.Value);
     }
}

It always worked great until I started implementing more async/multithreaded processing in the application. Now, sporadically, I get the following error when the above code runs:

System.InvalidOperationException: 'Collection was modified; enumeration operation may not execute.'

It doesn't seem like I'm modifying the collection at all, so I don't know why it's complaining.

I understand the discussion/solutions in these posts, but it doesn't seem to apply to mine:

Collection was modified exception

System.InvalidOperationException: Collection was modified

What is the best way to make this thread safe?

EDIT #1

I have this now:

private static object syncLock = new object();

[...]

lock ( syncLock ) {
   if ( paramList != null ) {
      foreach ( DictionaryEntry param in paramList ) {
          command.Parameters.AddWithValue(param.Key.ToString(), param.Value);
      }
   }
}

I ran it a few times and then the problem came back again:

How can this still be happening, even inside a lock()? What can I do?

EDIT #2

paramList is always created by the caller and passed into a function, for example:

var paramList = new OrderedDictionary();
paramList.Add("RuleID", btRule.RuleID);
paramList.Add("TypeSeq", 0);
paramList.Add("Type", btRule.Action);

bool result = await ExecuteQueryAsync(sql, paramList, connection);

public static async Task<bool> ExecuteQueryAsync(string sql, OrderedDictionary paramList, SqlConnection connection) {
     try {
        using ( SqlCommand command = new SqlCommand(sql, connection) ) {
           command.CommandType = CommandType.Text;

           lock ( syncLock ) {
              if ( paramList != null ) {
                 foreach ( DictionaryEntry param in paramList ) {
                    command.Parameters.AddWithValue(param.Key.ToString(), param.Value);
                 }
              }
           }

           command.CommandTimeout = 0;
           await command.ExecuteNonQueryAsync();
        }
        return true;
     }
     catch ( Exception ex ) {
        [...]
     }
}

any modification of paramList on any thread will cause this problem, you most likely have a thread safety issue somewhere — TheGeneral, Nov 09 '19 at 00:35
You could try [locking](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement) paramList wherever it's accessed in an async method — RyanHx, Nov 09 '19 at 01:15
what's `paramList`? where does it come from? what's it's type? — zaitsman, Nov 09 '19 at 01:19
You really need to provide us with a [mcve] to diagnose this issue. But, as a temporary fix, try adding `.ToList()` after the `paramList` in the `foreach` and see if the problem (mostly) disappears. — Enigmativity, Nov 09 '19 at 03:17
I edited the question to give the type of paramList (`OrderedDictionary`), which doesn't have a `ToList()`. — HerrimanCoder, Nov 09 '19 at 14:48
Using `lock()` seems to solve it, but now I get object reference errors as shown here: https://stackoverflow.com/questions/58784691/null-reference-in-async-loop-but-object-has-no-nulls — HerrimanCoder, Nov 10 '19 at 00:09
If you have to protect a shared resource with a `lock`, you must protect it everywhere, using the same locker object. Every read **and** write access to the resource must be protected. I don't see the `paramList` to be protected at the time it is populated with elements. Also I would like to see how you implement the multithreading part. Do you start threads manually, use the `Parallel` class, use `Task.WhenAll`, something else? — Theodor Zoulias, Nov 10 '19 at 15:36
1- It is obviously being accessed by more than one thread even though it might not be so easy to determine. 2-The lock only works if it is around both the write and read operations. so you have to also lock the code that populates the dictionary. 3-Just advice... finding a non-blocking pattern would be a much better approach. — Jonathan Alfaro, Nov 10 '19 at 15:59

score 1 · Answer 1 · answered Nov 09 '19 at 05:34

1

Because of if ( paramList != null ) { my guess is that paramList is reused by different actors. You will need to use a different instances per unit of work.

answered Nov 09 '19 at 05:34

tymtam

31,798
8
86
126

This solution doesn't make sense to me, I don't know what you're saying to do. Using `lock()` seems to solve it, but now I get object reference errors as shown here: https://stackoverflow.com/questions/58784691/null-reference-in-async-loop-but-object-has-no-nulls – HerrimanCoder Nov 10 '19 at 00:09
1

Each actor should have their own `paramList`, they shouldn't share it. – tymtam Nov 10 '19 at 00:12
`paramList` isn't shared - it's passed in by the caller, as a function param. – HerrimanCoder Nov 10 '19 at 02:50
The `paramList` is a key to this problem, and you are not showing us where you create it and how you use it. I would put my money that tymtam's assumption is correct. – Theodor Zoulias Nov 10 '19 at 07:14
Theodor and tymtam: please see my EDIT #2. I show exactly what `paramList` is, example usage, and how it's passed into the function. I don't think it's ever shared because every calling function has its own new instance of it. Any ideas? – HerrimanCoder Nov 10 '19 at 14:36

Collection was modified in async application?

1 Answers1

Linked