Null reference in async loop, but object has no nulls

Question

First, this is not answered by this SO post, as that describes simple/basic object reference problems. What I'm experiencing is related to multithreaded async processing, which the other post doesn't solve.

I have a multithreaded .NET winforms app and I'm doing this:

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

paramList is an OrderedDictionary.

I sporadically get this error on the foreach line:

Object reference not set to an instance of an object.

As you can see, param.Key is null and param.Value is null. But this makes no sense because there are no nulls in paramList, as you can see here:

In the screenshot you can see only index 2, but I examined index 0 and 1 also, same thing, valid data, no nulls.

I'm not experienced with multithreaded apps, but I put that block in a lock() due to responses in this SO post. Before putting in the lock() I was sporadically getting the error Collection was modified; enumeration operation may not execute. After putting in the lock, that error went away, but now I'm getting the object reference as shown above.

What can I do to solve this?

EDIT

Taking the advice of a few posters, I did this:

private static object syncLock = new object();

and then down in the usage:

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

That seems to have solved the object reference error (thanks everyone), but now I sporadically get:

Collection was modified; enumeration operation may not execute.

Because I got a completely different error after trying this new approach, I created a new SO question. I'm not sure if that was the right thing to do because now it seems these problems are related and I'm just seeing different symptoms of the same core problem.

Still looking for a solution if anyone has ideas.

Answer 1

(Preferred) Change your paramList so you don't need to lock it.

If you need to share then:

1. Lock on a object that doesn't change e.g. private readonly object l = new object();, or private static readonly, if needed.
2. Check that the code that fills paramList is also locked.

Answer 2

Your code will not work. What if it was not null during the null check, but is null when you come to the lock?

There is the standing advise to always have a dedicated thing to lock onto. A object _mutex = new object(); has become customary.

Edit: Private is implied, but you propably want to make it readonly as Charles suggested.

If you lock on variables/fields, you might run into this issues. Or boxing of primitives (wich does not do anything, because primitves are always put into different boxes). Or somebody else trying to lock it up the call stack, causing a deadlock.

Example code:

//_mutex is private, readonly and exist exclusively for this operation
lock(_mutex){
  //You only do null checks after you got a lock
  if ( paramList != null ){
    foreach ( DictionaryEntry param in paramList ) {
      command.Parameters.AddWithValue(param.Key.ToString(), param.Value);
    }
  }
}

All other code reading, writing or processing the variable paramList, any of it's Dictionary entires and their fields - including any copies* - also need to be in a lock(_mutex). So they can not collide with one another.

*copies of primitive types and string are the exception. Builtin Primitive types are inmutable. Also do not usually use reference mechanics. And the String class was also made inmutable, to allow it to work this way

Answer 3

This is the solution that solved all my problems:

var connection = getConnectionFromPool(sourceOrDC);

try {
   lock ( connection ) {
      using ( SqlCommand command = new SqlCommand(sql, connection) ) {
         command.CommandType = CommandType.Text;

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

         command.CommandTimeout = 0;
         command.ExecuteNonQuery();
      }
   }

   return true;
}

I had to enclose more of the code in lock(), and lock the connection.