Using parametrized thread

I'm trying to create some number of threads which would be equal to nOfplayers. I'm doing it like this:

for (int i = 0; i < nOfPlayers; i++)
{
    thr[i] = new Thread(
        () => playerComm[i].Communicate(handler[i], ref game, ref table));
    thr[i].Start();
}

There is only one time I use this cycle. But somehow, I don't know why, new Thread executes more than nOfPlayers-1 times. So I've got the index out of the range.

Does anyone know why it does it happen and how to do it right?

Jon Skeet
people
quotationmark

No, it's executing the right number of times - but you're capturing i, which is incrementing before the thread gets to use it.

Create a copy of i in your loop:

for (int i = 0; i < nOfPlayers; i++)
{
    int player = i;
    thr[i] = new Thread(
        () => playerComm[player].Communicate(handler[player], ref game, ref table));
    thr[i].Start();
}

See Eric Lippert's "Closing over the loop variable considered harmful" blog post for more info.

Also, using ref in multiple threads like this feels wrong to me - are you sure you need ref parameters at all? See my article on parameter passing in C# for details about what it means.

Finally, given that you appear to have two collections here - one for handlers and one for communicators, consider having a single collection instead, where each element of the collection knows about both parts.

Then in C# 5 you could get away with:

// Only correct in C# 5 and later!
var threads = new List<Thread>();
foreach (var player in players)
{
    // HandleCommunications would call comm.Communicate(handler, game, table)
    var thread = new Thread(() => player.HandleCommunications(game, table));
    thread.Start();
    threads.Add(thread);
}

Note that in C# 3 and 4 you'd still need an extra local variable within the loop, but the behaviour of foreach with respect to variable capture was fixed to be rather more sensible in C# 5.

people

See more on this question at Stackoverflow