Parameter crossing on Threads C# ERROR

Question

Hi, I've a little problem with this code:

This is the "main" method of the app:

private Thread main_process;
private Clases.GestorTR processor;

public void begin()
{
   processor = new Clases.GestorTR();
   main_process = new Thread(new ThreadStart(processor.ExecuteP));
   main_process.Start();
}

I've created a Thread to process other "Transacction Threads" to avoid blocking the GUI. This is the method ExecuteP, on processor object:

public void ExecuteP()
{
// Readed an DataTable with BD transacction, filled with numbers
 foreach (DataRow dr in dtResults.Rows)
 {
    int Local_number = Convert.toInt32(dr["autonum"].ToString());
    ThreadStart starter;
    starter = delegate { new QueryBD.QueryCounter(Local_number); };
    new Thread(starter).Start();
 }
}

This is QueryCounter method of QueryBD class:

....
private void QueryCounter(int _counter)
    {
        logs.log("ON QUERY_PROCESS: " + _counter);
    }
...

Now, the problem. When calling the delegate, some threads are crossing parameters. For example, in the foreach method the log shows correct (1,2,3,4,5,6,7,8) but, in the QueryCounter method (called each time with the new thread, the log shows (1,1,1,4,5,6,6,8) for example. I've also tried to use locks, but the problem is the same. Also testing with the ThreadPool way with the same result. I think I'm missing something in the foreach loop, because if I debug the first run, the thread is Started, but without action in the log.

Thanks!,

Answer 1

Hello,

You should try to change some parts of your code like that:

public void ExecuteP()
{
  QueryBD facade = new QueryBD.
  foreach (DataRow dr in dtResults.Rows)
  {
    int Local_number = Convert.toInt32(dr["autonum"].ToString());
    new Thread(new ParameterizedThreadStart(facade.QueryCounter)).Start(Local_number);
  }
}

public void QueryCounter(object _counter)
{
  ...
}

Hope it works.

Btw. I've created one object called facade and I'm passing that object to various threads. It can also result in some side effects if there will be thread sensitive part of code in the facade object, so you can also consider locking there:

public void QueryCounter(object _counter)
{
   lock(this)
   {
      //
   }
}

or providing new QueryBD to each thread, but it can affect performance.

EDIT: Hey, 4 things:

1. While using ParametrizedThread, the variable passed to Start method of the thread (thread.Start(variable)) is copied at the time of call. Such copied variable is then used in the child thread. Anonymous delegate works different. It keeps the reference to the variable, so when the variable is used by the child thread, it can be changed by the time in your parent thread. That is why you had unpredicted behaviour.

2. Better explanation you can find here: http://stackoverflow.com/questions/1923577/differing-behavior-when-starting-a-thread-parameterizedthreadstart-vs-anonymous.

3. The performance depends. If creation of your object is heavy (ex. it creates new connection to DB each time it is created) performance can be seriously affected by creation of many such objects - it is where lock is better. If creation of the object is light, you can create as many objects as you want. It depends.

4. If you want your code to be run in defined order, you shouldn't use threads at all. If you want to preserve execution order, sequential invoking is the right way - see Hans Passant explanation.