views:

123

answers:

2

I have a loop that could contain 1-10 records....

Inside that loop I am calling :

 ThreadStart processTaskThread = delegate { ProcessTasks(myParam ); };
 new Thread(processTaskThread).Start();

My question is: Will this work, or do I somehow need to write the code, so that the threads are declared using unique variable names?

+7  A: 

You don't need to give them unique names. You do need to make sure they are cleaned up when they finish, and you do need to make sure that if something changes to send you 10,000 records instead of just 10 that you don't try to create 10,000 threads. One way to do both is via the QueueUserWorkItem() function.

Joel Coehoorn
You don't need to clean up threads when they finish - at least not unless you're particularly doing something that requires cleanup.
Jon Skeet
I will research using the Queue instead, thanks
JL
But if you're doing this in a loop, you'll still need to use a copy of the variable to avoid the capture issue...
Jon Skeet
+5  A: 

You haven't shown the loop, and that's crucial. For instance, this is broken:

foreach (Foo myParam in parameters)
{
    ThreadStart processTaskThread = delegate { ProcessTasks(myParam ); };
    new Thread(processTaskThread).Start();
}

But this is okay:broken:

foreach (Foo myParam in parameters)
{
    Foo copy = myParam; // This line is crucial

    // Then we use the new variable in the anonymous method
    ThreadStart processTaskThread = delegate { ProcessTasks(copy); };
    new Thread(processTaskThread).Start();
}

In the first version, the myParam variable is being captured, and there's only one variable. In the second version, there's a new "instance" of the variable for each iteration of the loop. See my article on closures for more information.

Note that this doesn't require threads to demonstrate the problem. Here's an example without using any thread:

using System;
using System.Collections.Generic;

class Test
{
    static void Main()
    {
        List<Action> actions = new List<Action>();
        for (int i=0; i < 10; i++)
        {
            actions.Add(delegate { Console.WriteLine(i); });
        }

        foreach (Action action in actions)
        {
            action();
        }
    }
}

That prints "10" ten times. Here's the fixed version:

using System;
using System.Collections.Generic;

class Test
{
    static void Main()
    {
        List<Action> actions = new List<Action>();
        for (int i=0; i < 10; i++)
        {
            int copy = i;
            actions.Add(delegate { Console.WriteLine(copy); });
        }

        foreach (Action action in actions)
        {
            action();
        }
    }
}
Jon Skeet
+1: captured variables are a bit like roses; they are beautiful, but you need to watch out for the thorns.
Fredrik Mörk
Thank you Jon, as it turns out I was using the "broken" version. I have decided to go with the queue version instead.
JL